| Message ID | a5a3445637837e9bca24ccc6585e40f52acbe069.1486562055.git-series.kieran.bingham@ideasonboard.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Kieran, Thank you for the patch. On Wednesday 08 Feb 2017 14:03:59 Kieran Bingham wrote: > From: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Pass the optional '--crop (X,Y)/WxH' parameter through reference_frame > allowing the input to be cropped for comparison > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > scripts/vsp-lib.sh | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/scripts/vsp-lib.sh b/scripts/vsp-lib.sh > index 08bf8f36c582..42a4bb20d1be 100755 > --- a/scripts/vsp-lib.sh > +++ b/scripts/vsp-lib.sh > @@ -162,6 +162,9 @@ reference_frame() { > vflip) > [ x$value = x1 ] && options="$options --vflip" > ;; > + crop) > + options="$options --crop $value" > + ;; Could you please keep the options sorted alphabetically ? > esac > done > > @@ -717,6 +720,40 @@ format_rpf_wpf() { > __vsp_wpf_format=$5 > } > > +format_crop_rpf_wpf() { > + local rpf=$1 > + local wpf=$2 > + local infmt=$(format_v4l2_to_mbus $3) > + local size=$4 > + local outfmt=$(format_v4l2_to_mbus $5) > + local rpfcrop=$6 > + local wpfcrop=$7 > + local rpfoutsize= > + local outsize= > + > + if [ x$rpfcrop != 'x' ] ; then I don't think this will work properly. You want to test for the presence of an RPF crop argument. If it's absent, and the WPF crop argument is specified, WPF crop will be stored in $6, and will thus be assigned to $rpfcrop. I see two possible solutions. One of them is to make the RPF crop argument mandatory. After all, given the function name, one can expect RPF crop to be specified, otherwise the caller should use rpf-wpf, not crop-rpf-wpf. Another more versatile option would be to add RPF crop support to the existing format_rpf_wpf() crop function, and make both RPF and WPF crop optional. You could use named option for that (rcrop=... wcrop=...) or use a special value to indicate that an option should be skipped (for instance "- (0,0)/640x480" to set the WPF crop rectangle without setting the RPF crop rectangle). > + rpfcrop="crop:$rpfcrop" > + rpfoutsize=$(echo $rpfcrop | sed 's/.*\///') > + else > + rpfoutsize=$size > + fi > + > + if [ x$wpfcrop != 'x' ] ; then > + wpfcrop="crop:$wpfcrop" > + outsize=$(echo $wpfcrop | sed 's/.*\///') > + else > + outsize=$rpfoutsize > + fi > + > + $mediactl -d $mdev -V "'$dev rpf.$rpf':0 [fmt:$infmt/$size $rpfcrop]" > + $mediactl -d $mdev -V "'$dev rpf.$rpf':1 [fmt:$infmt/$rpfoutsize]" > + $mediactl -d $mdev -V "'$dev wpf.$wpf':0 [fmt:$infmt/$rpfoutsize > $wpfcrop]" > + $mediactl -d $mdev -V "'$dev wpf.$wpf':1 [fmt:$outfmt/$outsize]" > + > + __vsp_rpf_format=$3 > + __vsp_wpf_format=$5 > +} > + > format_wpf() { > local format=$(format_v4l2_to_mbus $1) > local size=$2
Ooops, I left this on my screen, and have already sent the V2 anyway. Hitting send for posterity. -- Kieran On 10/02/17 09:20, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Wednesday 08 Feb 2017 14:03:59 Kieran Bingham wrote: >> From: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >> Pass the optional '--crop (X,Y)/WxH' parameter through reference_frame >> allowing the input to be cropped for comparison >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> scripts/vsp-lib.sh | 37 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 37 insertions(+) >> >> diff --git a/scripts/vsp-lib.sh b/scripts/vsp-lib.sh >> index 08bf8f36c582..42a4bb20d1be 100755 >> --- a/scripts/vsp-lib.sh >> +++ b/scripts/vsp-lib.sh >> @@ -162,6 +162,9 @@ reference_frame() { >> vflip) >> [ x$value = x1 ] && options="$options --vflip" >> ;; >> + crop) >> + options="$options --crop $value" >> + ;; > > Could you please keep the options sorted alphabetically ? Fixed, > >> esac >> done >> >> @@ -717,6 +720,40 @@ format_rpf_wpf() { >> __vsp_wpf_format=$5 >> } >> >> +format_crop_rpf_wpf() { >> + local rpf=$1 >> + local wpf=$2 >> + local infmt=$(format_v4l2_to_mbus $3) >> + local size=$4 >> + local outfmt=$(format_v4l2_to_mbus $5) >> + local rpfcrop=$6 >> + local wpfcrop=$7 >> + local rpfoutsize= >> + local outsize= >> + >> + if [ x$rpfcrop != 'x' ] ; then > > I don't think this will work properly. You want to test for the presence of an > RPF crop argument. If it's absent, and the WPF crop argument is specified, WPF > crop will be stored in $6, and will thus be assigned to $rpfcrop. Yes, you're correct. > > I see two possible solutions. One of them is to make the RPF crop argument > mandatory. After all, given the function name, one can expect RPF crop to be > specified, otherwise the caller should use rpf-wpf, not crop-rpf-wpf. Another > more versatile option would be to add RPF crop support to the existing > format_rpf_wpf() crop function, and make both RPF and WPF crop optional. You > could use named option for that (rcrop=... wcrop=...) or use a special value > to indicate that an option should be skipped (for instance "- (0,0)/640x480" > to set the WPF crop rectangle without setting the RPF crop rectangle). I think prefer named arguments over positional arguments in non-type-checked code such as this... Although I think I went down the 'fast' route of duplicating rpf-crop over adding named arguments. That's not a good enough reason not to do it though ... As there are no current users of the 'wpf' crop parameter, I've changed this to support --rpfcrop=... --wpfcrop=... > >> + rpfcrop="crop:$rpfcrop" >> + rpfoutsize=$(echo $rpfcrop | sed 's/.*\///') >> + else >> + rpfoutsize=$size >> + fi >> + >> + if [ x$wpfcrop != 'x' ] ; then >> + wpfcrop="crop:$wpfcrop" >> + outsize=$(echo $wpfcrop | sed 's/.*\///') >> + else >> + outsize=$rpfoutsize >> + fi >> + >> + $mediactl -d $mdev -V "'$dev rpf.$rpf':0 [fmt:$infmt/$size $rpfcrop]" >> + $mediactl -d $mdev -V "'$dev rpf.$rpf':1 [fmt:$infmt/$rpfoutsize]" >> + $mediactl -d $mdev -V "'$dev wpf.$wpf':0 [fmt:$infmt/$rpfoutsize >> $wpfcrop]" >> + $mediactl -d $mdev -V "'$dev wpf.$wpf':1 [fmt:$outfmt/$outsize]" >> + >> + __vsp_rpf_format=$3 >> + __vsp_wpf_format=$5 >> +} >> + >> format_wpf() { >> local format=$(format_v4l2_to_mbus $1) >> local size=$2 > -- Kieran
diff --git a/scripts/vsp-lib.sh b/scripts/vsp-lib.sh index 08bf8f36c582..42a4bb20d1be 100755 --- a/scripts/vsp-lib.sh +++ b/scripts/vsp-lib.sh @@ -162,6 +162,9 @@ reference_frame() { vflip) [ x$value = x1 ] && options="$options --vflip" ;; + crop) + options="$options --crop $value" + ;; esac done @@ -717,6 +720,40 @@ format_rpf_wpf() { __vsp_wpf_format=$5 } +format_crop_rpf_wpf() { + local rpf=$1 + local wpf=$2 + local infmt=$(format_v4l2_to_mbus $3) + local size=$4 + local outfmt=$(format_v4l2_to_mbus $5) + local rpfcrop=$6 + local wpfcrop=$7 + local rpfoutsize= + local outsize= + + if [ x$rpfcrop != 'x' ] ; then + rpfcrop="crop:$rpfcrop" + rpfoutsize=$(echo $rpfcrop | sed 's/.*\///') + else + rpfoutsize=$size + fi + + if [ x$wpfcrop != 'x' ] ; then + wpfcrop="crop:$wpfcrop" + outsize=$(echo $wpfcrop | sed 's/.*\///') + else + outsize=$rpfoutsize + fi + + $mediactl -d $mdev -V "'$dev rpf.$rpf':0 [fmt:$infmt/$size $rpfcrop]" + $mediactl -d $mdev -V "'$dev rpf.$rpf':1 [fmt:$infmt/$rpfoutsize]" + $mediactl -d $mdev -V "'$dev wpf.$wpf':0 [fmt:$infmt/$rpfoutsize $wpfcrop]" + $mediactl -d $mdev -V "'$dev wpf.$wpf':1 [fmt:$outfmt/$outsize]" + + __vsp_rpf_format=$3 + __vsp_wpf_format=$5 +} + format_wpf() { local format=$(format_v4l2_to_mbus $1) local size=$2