Re: [PATCH 4/5] vsp-lib: Support RPF frame cropping
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
>>
>> Pass the optional '--crop (X,Y)/WxH' parameter through reference_frame
>> allowing the input to be cropped for comparison
>>
>> Signed-off-by: Kieran Bingham
>> ---
>> 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
Re: [PATCH 4/5] vsp-lib: Support RPF frame cropping
Hi Kieran,
Thank you for the patch.
On Wednesday 08 Feb 2017 14:03:59 Kieran Bingham wrote:
> From: Kieran Bingham
>
> Pass the optional '--crop (X,Y)/WxH' parameter through reference_frame
> allowing the input to be cropped for comparison
>
> Signed-off-by: Kieran Bingham
> ---
> 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
--
Regards,
Laurent Pinchart
[PATCH 4/5] vsp-lib: Support RPF frame cropping
From: Kieran Bingham
Pass the optional '--crop (X,Y)/WxH' parameter through reference_frame
allowing the input to be cropped for comparison
Signed-off-by: Kieran Bingham
---
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"
+ ;;
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
--
git-series 0.9.1
