Re: [PATCH v6 14/18] media: verisilicon: vp9: Use destination buffer height to compute chroma offset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Le 11/09/2023 à 18:36, Jernej Škrabec a écrit :
Dne ponedeljek, 11. september 2023 ob 10:55:02 CEST je Benjamin Gaignard
napisal(a):
Le 10/09/2023 à 15:21, Jernej Škrabec a écrit :
Hi Benjamin!

Dne petek, 01. september 2023 ob 14:44:10 CEST je Benjamin Gaignard

napisal(a):
Source and destination buffer height may not be the same because
alignment constraint are different.
Use destination height to compute chroma offset because we target
this buffer as hardware output.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
Fixes: e2da465455ce ("media: hantro: Support VP9 on the G2 core")
---

   drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c | 4 +---
   1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
b/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c index
6db1c32fce4d..1f3f5e7ce978 100644
--- a/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
+++ b/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
@@ -93,9 +93,7 @@ static int start_prepare_run(struct hantro_ctx *ctx,
const struct v4l2_ctrl_vp9_ static size_t chroma_offset(const struct
hantro_ctx *ctx,

   			    const struct v4l2_ctrl_vp9_frame
*dec_params)

   {

-	int bytes_per_pixel = dec_params->bit_depth == 8 ? 1 : 2;
-
-	return ctx->src_fmt.width * ctx->src_fmt.height * bytes_per_pixel;
+	return ctx->dst_fmt.width * ctx->dst_fmt.height * ctx->bit_depth /
8;

Commit message doesn't mention bit_depth change at all. While I think
there is no difference between dec_params->bit_depth and ctx->bit_depth,
you shouldn't just use ordinary division. If bit_depth is 10, it will be
rounded down. And if you decide to use bit_depth from context, please
remove dec_params argument.
I will change this patch and create a helpers function for chroma and motion
vectors offsets that VP9 and HEVC code will use since they are identical.
I don't see issue with the division. If you have in mind a solution please
write it so I could test it.
Solution is same as the code that you removed:
int bytes_per_pixel = dec_params->bit_depth == 8 ? 1 : 2;

Or alternatively:
int bytes_per_pixel = DIV_ROUND_UP(dec_params->bit_depth, 8);

Consider bit_depth being 10. With old code you get 2, with yours you get 1.

The old code is wrong ;-)
If the format depth is 10 bits per pixel then chroma offset (in bytes) formula is
width * height * 10 / 8 not width * height * 16 / 8.

I have already confirm that with HEVC on the same hardware.

Regards,
Benjamin


Best regards,
Jernej

Regards,
Benjamin

Best regards,
Jernej

   }
static size_t mv_offset(const struct hantro_ctx *ctx,







[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux