diff mbox series

media: verisilicon: Do not enable G2 postproc downscale if source is narrower than destination

Message ID 20230824013935.303132-1-marex@denx.de (mailing list archive)
State New, archived
Headers show
Series media: verisilicon: Do not enable G2 postproc downscale if source is narrower than destination | expand

Commit Message

Marek Vasut Aug. 24, 2023, 1:39 a.m. UTC
In case of encoded input VP9 data width that is not multiple of macroblock
size, which is 16 (e.g. 1080x1920 frames, where 1080 is multiple of 8), the
width is padded to be a multiple of macroblock size (for 1080x1920 frames,
that is 1088x1920).

The hantro_postproc_g2_enable() checks whether the encoded data width is
equal to decoded frame width, and if not, enables down-scale mode. For a
frame where input is 1080x1920 and output is 1088x1920, this is incorrect
as no down-scale happens, the frame is only padded. Enabling the down-scale
mode in this case results in corrupted frames.

Fix this by adjusting the check to test whether encoded data width is
greater than decoded frame width, and only in that case enable the
down-scale mode.

To generate input test data to trigger this bug, use e.g.:
$ gst-launch-1.0 videotestsrc ! video/x-raw,width=272,height=256,format=I420 ! \
                 vp9enc ! matroskamux ! filesink location=/tmp/test.vp9
To trigger the bug upon decoding (note that the NV12 must be forced, as
that assures the output data would pass the G2 postproc):
$ gst-launch-1.0 filesrc location=/tmp/test.vp9 ! matroskademux ! vp9parse ! \
                 v4l2slvp9dec ! video/x-raw,format=NV12 ! videoconvert ! fbdevsink

Fixes: 79c987de8b35 ("media: hantro: Use post processor scaling capacities")
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Adam Ford <aford173@gmail.com>
Cc: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: linux-media@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
---
 drivers/media/platform/verisilicon/hantro_postproc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Benjamin Gaignard Aug. 24, 2023, 6:57 a.m. UTC | #1
Le 24/08/2023 à 03:39, Marek Vasut a écrit :
> In case of encoded input VP9 data width that is not multiple of macroblock
> size, which is 16 (e.g. 1080x1920 frames, where 1080 is multiple of 8), the
> width is padded to be a multiple of macroblock size (for 1080x1920 frames,
> that is 1088x1920).
>
> The hantro_postproc_g2_enable() checks whether the encoded data width is
> equal to decoded frame width, and if not, enables down-scale mode. For a
> frame where input is 1080x1920 and output is 1088x1920, this is incorrect
> as no down-scale happens, the frame is only padded. Enabling the down-scale
> mode in this case results in corrupted frames.
>
> Fix this by adjusting the check to test whether encoded data width is
> greater than decoded frame width, and only in that case enable the
> down-scale mode.
>
> To generate input test data to trigger this bug, use e.g.:
> $ gst-launch-1.0 videotestsrc ! video/x-raw,width=272,height=256,format=I420 ! \
>                   vp9enc ! matroskamux ! filesink location=/tmp/test.vp9
> To trigger the bug upon decoding (note that the NV12 must be forced, as
> that assures the output data would pass the G2 postproc):
> $ gst-launch-1.0 filesrc location=/tmp/test.vp9 ! matroskademux ! vp9parse ! \
>                   v4l2slvp9dec ! video/x-raw,format=NV12 ! videoconvert ! fbdevsink

I had propose the same patch in my series in July:
https://patchwork.kernel.org/project/linux-media/patch/20230705121056.37017-8-benjamin.gaignard@collabora.com/

Reviewed-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

>
> Fixes: 79c987de8b35 ("media: hantro: Use post processor scaling capacities")
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Adam Ford <aford173@gmail.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: linux-media@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> ---
>   drivers/media/platform/verisilicon/hantro_postproc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
> index 0224ff68ab3fc..64d6fb852ae9b 100644
> --- a/drivers/media/platform/verisilicon/hantro_postproc.c
> +++ b/drivers/media/platform/verisilicon/hantro_postproc.c
> @@ -107,7 +107,7 @@ static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
>   
>   static int down_scale_factor(struct hantro_ctx *ctx)
>   {
> -	if (ctx->src_fmt.width == ctx->dst_fmt.width)
> +	if (ctx->src_fmt.width <= ctx->dst_fmt.width)
>   		return 0;
>   
>   	return DIV_ROUND_CLOSEST(ctx->src_fmt.width, ctx->dst_fmt.width);
diff mbox series

Patch

diff --git a/drivers/media/platform/verisilicon/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
index 0224ff68ab3fc..64d6fb852ae9b 100644
--- a/drivers/media/platform/verisilicon/hantro_postproc.c
+++ b/drivers/media/platform/verisilicon/hantro_postproc.c
@@ -107,7 +107,7 @@  static void hantro_postproc_g1_enable(struct hantro_ctx *ctx)
 
 static int down_scale_factor(struct hantro_ctx *ctx)
 {
-	if (ctx->src_fmt.width == ctx->dst_fmt.width)
+	if (ctx->src_fmt.width <= ctx->dst_fmt.width)
 		return 0;
 
 	return DIV_ROUND_CLOSEST(ctx->src_fmt.width, ctx->dst_fmt.width);