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@xxxxxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxxx>
>> ---
>> 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
