diff mbox series

[2/3] media: staging: rkisp1: rsz: change (hv)div only if capture format is YUV

Message ID 20200328105606.13660-3-dafna.hirschfeld@collabora.com (mailing list archive)
State New, archived
Headers show
Series media: staging: rkisp1: add support for RGB formats | expand

Commit Message

Dafna Hirschfeld March 28, 2020, 10:56 a.m. UTC
RGB formats in selfpath should receive input format as YUV422.
The resizer input format is always YUV422 and therefore
if the capture format is RGB, the resizer should not change
the YUV rations.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-resizer.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Helen Koike March 30, 2020, 8:04 p.m. UTC | #1
On 3/28/20 7:56 AM, Dafna Hirschfeld wrote:
> RGB formats in selfpath should receive input format as YUV422.
> The resizer input format is always YUV422 and therefore
> if the capture format is RGB, the resizer should not change
> the YUV rations.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  drivers/staging/media/rkisp1/rkisp1-resizer.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> index 8704267a066f..5721eee29ecb 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> @@ -389,8 +389,18 @@ static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
>  			&rsz->rkisp1->capture_devs[rsz->id];
>  		const struct v4l2_format_info *pixfmt_info = cap->pix.info;
>  
> -		hdiv = pixfmt_info->hdiv;
> -		vdiv = pixfmt_info->vdiv;
> +		/*
> +		 * The resizer always get the input as YUV422
> +		 * If the capture encoding is also YUV, then the resizer should
> +		 * change the 4:2:2 sampling to the sampling of the capture
> +		 * format (4:2:2 -> 4:2:0 for example).
> +		 * If the capture format is RGB then the memory input should
> +		 * be YUV422 so we don't change the default hdiv, vdiv
> +		 */
> +		if (v4l2_is_format_yuv(pixfmt_info)) {

Can't this be moved with && in the outer if statement block?

Regards,
Helen

> +			hdiv = pixfmt_info->hdiv;
> +			vdiv = pixfmt_info->vdiv;
> +		}
>  	}
>  	src_c.width = src_y.width / hdiv;
>  	src_c.height = src_y.height / vdiv;
>
Dafna Hirschfeld April 4, 2020, 12:09 p.m. UTC | #2
On 30.03.20 22:04, Helen Koike wrote:
> 
> 
> On 3/28/20 7:56 AM, Dafna Hirschfeld wrote:
>> RGB formats in selfpath should receive input format as YUV422.
>> The resizer input format is always YUV422 and therefore
>> if the capture format is RGB, the resizer should not change
>> the YUV rations.
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>>   drivers/staging/media/rkisp1/rkisp1-resizer.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
>> index 8704267a066f..5721eee29ecb 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
>> @@ -389,8 +389,18 @@ static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
>>   			&rsz->rkisp1->capture_devs[rsz->id];
>>   		const struct v4l2_format_info *pixfmt_info = cap->pix.info;
>>   
>> -		hdiv = pixfmt_info->hdiv;
>> -		vdiv = pixfmt_info->vdiv;
>> +		/*
>> +		 * The resizer always get the input as YUV422
>> +		 * If the capture encoding is also YUV, then the resizer should
>> +		 * change the 4:2:2 sampling to the sampling of the capture
>> +		 * format (4:2:2 -> 4:2:0 for example).
>> +		 * If the capture format is RGB then the memory input should
>> +		 * be YUV422 so we don't change the default hdiv, vdiv
>> +		 */
>> +		if (v4l2_is_format_yuv(pixfmt_info)) {
> 
> Can't this be moved with && in the outer if statement block?

Actually the outer statement is not needed at all and can be removed
since the code return if rsz->fmt_type is RKISP1_FMT_BAYER but
the resizer format is either bayer or yuv so there is no
need for the "if (rsz->fmt_type == RKISP1_FMT_YUV)"

Thanks,
Dafna

> 
> Regards,
> Helen
> 
>> +			hdiv = pixfmt_info->hdiv;
>> +			vdiv = pixfmt_info->vdiv;
>> +		}
>>   	}
>>   	src_c.width = src_y.width / hdiv;
>>   	src_c.height = src_y.height / vdiv;
>>
Laurent Pinchart April 5, 2020, 5:29 p.m. UTC | #3
Hi Dafna,

On Sat, Apr 04, 2020 at 02:09:20PM +0200, Dafna Hirschfeld wrote:
> On 30.03.20 22:04, Helen Koike wrote:
> > On 3/28/20 7:56 AM, Dafna Hirschfeld wrote:
> >> RGB formats in selfpath should receive input format as YUV422.
> >> The resizer input format is always YUV422 and therefore
> >> if the capture format is RGB, the resizer should not change
> >> the YUV rations.
> >>
> >> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> >> ---
> >>   drivers/staging/media/rkisp1/rkisp1-resizer.c | 14 ++++++++++++--
> >>   1 file changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> >> index 8704267a066f..5721eee29ecb 100644
> >> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
> >> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
> >> @@ -389,8 +389,18 @@ static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
> >>   			&rsz->rkisp1->capture_devs[rsz->id];
> >>   		const struct v4l2_format_info *pixfmt_info = cap->pix.info;
> >>   
> >> -		hdiv = pixfmt_info->hdiv;
> >> -		vdiv = pixfmt_info->vdiv;
> >> +		/*
> >> +		 * The resizer always get the input as YUV422
> >> +		 * If the capture encoding is also YUV, then the resizer should
> >> +		 * change the 4:2:2 sampling to the sampling of the capture
> >> +		 * format (4:2:2 -> 4:2:0 for example).
> >> +		 * If the capture format is RGB then the memory input should
> >> +		 * be YUV422 so we don't change the default hdiv, vdiv
> >> +		 */
> >> +		if (v4l2_is_format_yuv(pixfmt_info)) {
> > 
> > Can't this be moved with && in the outer if statement block?
> 
> Actually the outer statement is not needed at all and can be removed
> since the code return if rsz->fmt_type is RKISP1_FMT_BAYER but
> the resizer format is either bayer or yuv so there is no
> need for the "if (rsz->fmt_type == RKISP1_FMT_YUV)"

This sounds reasonable.

> >> +			hdiv = pixfmt_info->hdiv;
> >> +			vdiv = pixfmt_info->vdiv;
> >> +		}
> >>   	}
> >>   	src_c.width = src_y.width / hdiv;
> >>   	src_c.height = src_y.height / vdiv;
> >>
diff mbox series

Patch

diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index 8704267a066f..5721eee29ecb 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -389,8 +389,18 @@  static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
 			&rsz->rkisp1->capture_devs[rsz->id];
 		const struct v4l2_format_info *pixfmt_info = cap->pix.info;
 
-		hdiv = pixfmt_info->hdiv;
-		vdiv = pixfmt_info->vdiv;
+		/*
+		 * The resizer always get the input as YUV422
+		 * If the capture encoding is also YUV, then the resizer should
+		 * change the 4:2:2 sampling to the sampling of the capture
+		 * format (4:2:2 -> 4:2:0 for example).
+		 * If the capture format is RGB then the memory input should
+		 * be YUV422 so we don't change the default hdiv, vdiv
+		 */
+		if (v4l2_is_format_yuv(pixfmt_info)) {
+			hdiv = pixfmt_info->hdiv;
+			vdiv = pixfmt_info->vdiv;
+		}
 	}
 	src_c.width = src_y.width / hdiv;
 	src_c.height = src_y.height / vdiv;