diff mbox series

[2/3] gen-image: Crop input image before format conversion to YUV

Message ID 20220228112901.21289-2-laurent.pinchart@ideasonboard.com (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series [1/3] tests: Add SPDX headers to vsp-unit-test-0026.sh | expand

Commit Message

Laurent Pinchart Feb. 28, 2022, 11:29 a.m. UTC
When converting the input image to a subsampled YUV format, chroma
components are averaged horizontally to emulate the VSP internal
conversion to YUV444. If the image is subsequently cropped to emulate
the RPF cropping, the edge chroma values end up effectively taking into
account pixels outside of the crop rectangle. This doesn't match the
hardware mode of operation which crops the image when reading it through
DMA, before performing any other operation.

Fix this by cropping the image just after reading it, before format
conversion.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/gen-image.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

Comments

Kieran Bingham Feb. 28, 2022, 12:11 p.m. UTC | #1
Quoting Laurent Pinchart (2022-02-28 11:29:00)
> When converting the input image to a subsampled YUV format, chroma
> components are averaged horizontally to emulate the VSP internal
> conversion to YUV444. If the image is subsequently cropped to emulate
> the RPF cropping, the edge chroma values end up effectively taking into
> account pixels outside of the crop rectangle. This doesn't match the
> hardware mode of operation which crops the image when reading it through
> DMA, before performing any other operation.
> 
> Fix this by cropping the image just after reading it, before format
> conversion.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Seems fine. And there's only one input format so:

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
>  src/gen-image.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/src/gen-image.c b/src/gen-image.c
> index d9f92253af46..170d69490399 100644
> --- a/src/gen-image.c
> +++ b/src/gen-image.c
> @@ -1531,6 +1531,23 @@ static int process(const struct options *options)
>                 goto done;
>         }
>  
> +       /* Crop */
> +       if (options->crop) {
> +               struct image *cropped;
> +
> +               cropped = image_new(format_by_name("RGB24"),
> +                                   options->inputcrop.width,
> +                                   options->inputcrop.height);
> +               if (!cropped) {
> +                       ret = -ENOMEM;
> +                       goto done;
> +               }
> +
> +               image_crop(input, cropped, &options->inputcrop);
> +               image_delete(input);
> +               input = cropped;
> +       }
> +
>         /* Convert colorspace */
>         if (options->input_format->type == FORMAT_YUV) {
>                 struct image *yuv;
> @@ -1561,22 +1578,6 @@ static int process(const struct options *options)
>                 input = rgb;
>         }
>  
> -       /* Crop */
> -       if (options->crop) {
> -               struct image *cropped;
> -
> -               cropped = image_new(input->format, options->inputcrop.width,
> -                               options->inputcrop.height);
> -               if (!cropped) {
> -                       ret = -ENOMEM;
> -                       goto done;
> -               }
> -
> -               image_crop(input, cropped, &options->inputcrop);
> -               image_delete(input);
> -               input = cropped;
> -       }
> -
>         /* Scale */
>         if (options->output_width && options->output_height) {
>                 output_width = options->output_width;
> -- 
> Regards,
> 
> Laurent Pinchart
>
diff mbox series

Patch

diff --git a/src/gen-image.c b/src/gen-image.c
index d9f92253af46..170d69490399 100644
--- a/src/gen-image.c
+++ b/src/gen-image.c
@@ -1531,6 +1531,23 @@  static int process(const struct options *options)
 		goto done;
 	}
 
+	/* Crop */
+	if (options->crop) {
+		struct image *cropped;
+
+		cropped = image_new(format_by_name("RGB24"),
+				    options->inputcrop.width,
+				    options->inputcrop.height);
+		if (!cropped) {
+			ret = -ENOMEM;
+			goto done;
+		}
+
+		image_crop(input, cropped, &options->inputcrop);
+		image_delete(input);
+		input = cropped;
+	}
+
 	/* Convert colorspace */
 	if (options->input_format->type == FORMAT_YUV) {
 		struct image *yuv;
@@ -1561,22 +1578,6 @@  static int process(const struct options *options)
 		input = rgb;
 	}
 
-	/* Crop */
-	if (options->crop) {
-		struct image *cropped;
-
-		cropped = image_new(input->format, options->inputcrop.width,
-				options->inputcrop.height);
-		if (!cropped) {
-			ret = -ENOMEM;
-			goto done;
-		}
-
-		image_crop(input, cropped, &options->inputcrop);
-		image_delete(input);
-		input = cropped;
-	}
-
 	/* Scale */
 	if (options->output_width && options->output_height) {
 		output_width = options->output_width;