| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <bff48ad7ec42344d81b930babda7777615171148.camel@ndufresne.ca>
Date: Mon, 16 May 2022 15:08:58 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Benjamin Gaignard <benjamin.gaignard@...labora.com>,
ezequiel@...guardiasur.com.ar, p.zabel@...gutronix.de,
mchehab@...nel.org, gregkh@...uxfoundation.org,
shawnguo@...nel.org, s.hauer@...gutronix.de, festevam@...il.com,
linux-imx@....com, heiko@...ech.de, wens@...e.org,
jernej.skrabec@...il.com, samuel@...lland.org
Cc: kernel@...gutronix.de, linux-media@...r.kernel.org,
linux-rockchip@...ts.infradead.org, linux-staging@...ts.linux.dev,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-sunxi@...ts.linux.dev, kernel@...labora.com
Subject: Re: [PATCH] media: hantro: Be more accurate on pixel formats
step_width constraints
Hi Benjamin,
thanks for your patch, but see below, perhaps something you could improve...
Le lundi 16 mai 2022 à 16:11 +0200, Benjamin Gaignard a écrit :
> On Hantro G2 decoder on IMX8MQ strides requirements aren't the same
> for NV12_4L4 and NV12 pixel formats. The first one use a 4 bytes padding
> while the last one needs 8 bytes.
> To be sure to provide the correct stride in all cases we need:
> - to relax the constraints on codec formats so set step_width to 4
> - use capture queue format and not the output queue format when applying
> the pixel format constraints.
> - put the correct step_width constraints on each pixel format.
>From v4l2_apply_frmsize_constraints() point of view, 8 bytes is a sub-set of 4
bytes. The application can request larger then coded size width/height after
this change and you'd still get a broken render. The reason is that it looks
like the tile mode has more constraints then what
v4l2_apply_frmsize_constraints(). It seems like from your description that the
width/height must match the coded size (+plus the alignment). For this reason, I
think you need when using tile mode validate the that the width/height passed to
v4l2_apply_frmsize_constraints() matched the coded size from the header
structure rather then user provided size passed in S_FMT.
regards,
Nicolas
>
> With this SAODBLK_A_MainConcept_4 and SAODBLK_B_MainConcept_4 conformance
> tests files are correctly decoded with both NV12 and NV12_4L4 pixel formats.
> These two files have a resolution of 1016x760.
> If step_width = 16 for the both pixel formats the selected capture
> resolution is 1024x768 which is wrong for NV12_4L4 (which expect 1016x760)
> on Hantro G2 on IMX8MQ (but correct for NV12).
>
> For other variants than Hantro G2 on IMX8M keep the same step_width to avoid
> regressions.
>
> Fluster HEVC test score is now 128/147 vs 126/147 with the both pixel
> formats as decoder output.
> Fluster VP9 test score stay at 147/303.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
> ---
> drivers/staging/media/hantro/hantro_v4l2.c | 2 +-
> drivers/staging/media/hantro/imx8m_vpu_hw.c | 40 +++++++++++++++++--
> .../staging/media/hantro/rockchip_vpu_hw.c | 32 +++++++++++++++
> .../staging/media/hantro/sama5d4_vdec_hw.c | 16 ++++++++
> drivers/staging/media/hantro/sunxi_vpu_hw.c | 16 ++++++++
> 5 files changed, 101 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
> index 71a6279750bf..93d0dcf69f4a 100644
> --- a/drivers/staging/media/hantro/hantro_v4l2.c
> +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> @@ -260,7 +260,7 @@ static int hantro_try_fmt(const struct hantro_ctx *ctx,
> } else if (ctx->is_encoder) {
> vpu_fmt = ctx->vpu_dst_fmt;
> } else {
> - vpu_fmt = ctx->vpu_src_fmt;
> + vpu_fmt = fmt;
> /*
> * Width/height on the CAPTURE end of a decoder are ignored and
> * replaced by the OUTPUT ones.
> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> index 9802508bade2..b6b2bf65e56d 100644
> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> @@ -83,6 +83,14 @@ static const struct hantro_fmt imx8m_vpu_postproc_fmts[] = {
> .fourcc = V4L2_PIX_FMT_YUYV,
> .codec_mode = HANTRO_MODE_NONE,
> .postprocessed = true,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 3840,
> + .step_width = MB_DIM,
> + .min_height = 48,
> + .max_height = 2160,
> + .step_height = MB_DIM,
> + },
> },
> };
>
> @@ -90,6 +98,14 @@ static const struct hantro_fmt imx8m_vpu_dec_fmts[] = {
> {
> .fourcc = V4L2_PIX_FMT_NV12,
> .codec_mode = HANTRO_MODE_NONE,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 3840,
> + .step_width = MB_DIM,
> + .min_height = 48,
> + .max_height = 2160,
> + .step_height = MB_DIM,
> + },
> },
> {
> .fourcc = V4L2_PIX_FMT_MPEG2_SLICE,
> @@ -137,6 +153,14 @@ static const struct hantro_fmt imx8m_vpu_g2_postproc_fmts[] = {
> .fourcc = V4L2_PIX_FMT_NV12,
> .codec_mode = HANTRO_MODE_NONE,
> .postprocessed = true,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 3840,
> + .step_width = MB_DIM,
> + .min_height = 48,
> + .max_height = 2160,
> + .step_height = MB_DIM,
> + },
> },
> };
>
> @@ -144,6 +168,14 @@ static const struct hantro_fmt imx8m_vpu_g2_dec_fmts[] = {
> {
> .fourcc = V4L2_PIX_FMT_NV12_4L4,
> .codec_mode = HANTRO_MODE_NONE,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 3840,
> + .step_width = 4,
> + .min_height = 48,
> + .max_height = 2160,
> + .step_height = 4,
> + },
> },
> {
> .fourcc = V4L2_PIX_FMT_HEVC_SLICE,
> @@ -152,10 +184,10 @@ static const struct hantro_fmt imx8m_vpu_g2_dec_fmts[] = {
> .frmsize = {
> .min_width = 48,
> .max_width = 3840,
> - .step_width = MB_DIM,
> + .step_width = 4,
> .min_height = 48,
> .max_height = 2160,
> - .step_height = MB_DIM,
> + .step_height = 4,
> },
> },
> {
> @@ -165,10 +197,10 @@ static const struct hantro_fmt imx8m_vpu_g2_dec_fmts[] = {
> .frmsize = {
> .min_width = 48,
> .max_width = 3840,
> - .step_width = MB_DIM,
> + .step_width = 4,
> .min_height = 48,
> .max_height = 2160,
> - .step_height = MB_DIM,
> + .step_height = 4,
> },
> },
> };
> diff --git a/drivers/staging/media/hantro/rockchip_vpu_hw.c b/drivers/staging/media/hantro/rockchip_vpu_hw.c
> index fc96501f3bc8..efba7fcdf207 100644
> --- a/drivers/staging/media/hantro/rockchip_vpu_hw.c
> +++ b/drivers/staging/media/hantro/rockchip_vpu_hw.c
> @@ -63,6 +63,14 @@ static const struct hantro_fmt rockchip_vpu1_postproc_fmts[] = {
> .fourcc = V4L2_PIX_FMT_YUYV,
> .codec_mode = HANTRO_MODE_NONE,
> .postprocessed = true,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 1920,
> + .step_width = MB_DIM,
> + .min_height = 48,
> + .max_height = 1088,
> + .step_height = MB_DIM,
> + },
> },
> };
>
> @@ -70,6 +78,14 @@ static const struct hantro_fmt rk3066_vpu_dec_fmts[] = {
> {
> .fourcc = V4L2_PIX_FMT_NV12,
> .codec_mode = HANTRO_MODE_NONE,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 1920,
> + .step_width = MB_DIM,
> + .min_height = 48,
> + .max_height = 1088,
> + .step_height = MB_DIM,
> + },
> },
> {
> .fourcc = V4L2_PIX_FMT_H264_SLICE,
> @@ -116,6 +132,14 @@ static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
> {
> .fourcc = V4L2_PIX_FMT_NV12,
> .codec_mode = HANTRO_MODE_NONE,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 4096,
> + .step_width = MB_DIM,
> + .min_height = 48,
> + .max_height = 2304,
> + .step_height = MB_DIM,
> + },
> },
> {
> .fourcc = V4L2_PIX_FMT_H264_SLICE,
> @@ -162,6 +186,14 @@ static const struct hantro_fmt rk3399_vpu_dec_fmts[] = {
> {
> .fourcc = V4L2_PIX_FMT_NV12,
> .codec_mode = HANTRO_MODE_NONE,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 1920,
> + .step_width = MB_DIM,
> + .min_height = 48,
> + .max_height = 1088,
> + .step_height = MB_DIM,
> + },
> },
> {
> .fourcc = V4L2_PIX_FMT_H264_SLICE,
> diff --git a/drivers/staging/media/hantro/sama5d4_vdec_hw.c b/drivers/staging/media/hantro/sama5d4_vdec_hw.c
> index b2fc1c5613e1..07ee804e706b 100644
> --- a/drivers/staging/media/hantro/sama5d4_vdec_hw.c
> +++ b/drivers/staging/media/hantro/sama5d4_vdec_hw.c
> @@ -16,6 +16,14 @@ static const struct hantro_fmt sama5d4_vdec_postproc_fmts[] = {
> .fourcc = V4L2_PIX_FMT_YUYV,
> .codec_mode = HANTRO_MODE_NONE,
> .postprocessed = true,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 1280,
> + .step_width = MB_DIM,
> + .min_height = 48,
> + .max_height = 720,
> + .step_height = MB_DIM,
> + },
> },
> };
>
> @@ -23,6 +31,14 @@ static const struct hantro_fmt sama5d4_vdec_fmts[] = {
> {
> .fourcc = V4L2_PIX_FMT_NV12,
> .codec_mode = HANTRO_MODE_NONE,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 1280,
> + .step_width = MB_DIM,
> + .min_height = 48,
> + .max_height = 720,
> + .step_height = MB_DIM,
> + },
> },
> {
> .fourcc = V4L2_PIX_FMT_MPEG2_SLICE,
> diff --git a/drivers/staging/media/hantro/sunxi_vpu_hw.c b/drivers/staging/media/hantro/sunxi_vpu_hw.c
> index c0edd5856a0c..c2392c08febb 100644
> --- a/drivers/staging/media/hantro/sunxi_vpu_hw.c
> +++ b/drivers/staging/media/hantro/sunxi_vpu_hw.c
> @@ -14,6 +14,14 @@ static const struct hantro_fmt sunxi_vpu_postproc_fmts[] = {
> .fourcc = V4L2_PIX_FMT_NV12,
> .codec_mode = HANTRO_MODE_NONE,
> .postprocessed = true,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 3840,
> + .step_width = 32,
> + .min_height = 48,
> + .max_height = 2160,
> + .step_height = 32,
> + },
> },
> };
>
> @@ -21,6 +29,14 @@ static const struct hantro_fmt sunxi_vpu_dec_fmts[] = {
> {
> .fourcc = V4L2_PIX_FMT_NV12_4L4,
> .codec_mode = HANTRO_MODE_NONE,
> + .frmsize = {
> + .min_width = 48,
> + .max_width = 3840,
> + .step_width = 32,
> + .min_height = 48,
> + .max_height = 2160,
> + .step_height = 32,
> + },
> },
> {
> .fourcc = V4L2_PIX_FMT_VP9_FRAME,
Powered by blists - more mailing lists