[PATCH v4 1/2] media: coda: Fix reported H264 profile

[PATCH v4 1/2] media: coda: Fix reported H264 profile

[Fabio Estevam]

From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

The CODA960 manual states that ASO/FMO features of baseline are not
supported, so for this reason this driver should only report
constrained baseline support.

This fixes negotiation issue with constrained baseline content
on GStreamer 1.17.1.

ASO/FMO features are unsupported for the encoder and untested for the
decoder because there is currently no userspace support. Neither GStreamer
parsers nor FFMPEG parsers support ASO/FMO.

Cc: stable@vger.kernel.org
Fixes: 42a68012e67c2 ("media: coda: add read-only h.264 decoder profile/level controls")
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Pascal Speck <kernel@iktek.de>
Signed-off-by: Fabio Estevam <festevam@denx.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v3:
- Rebased against linux-next and took the coda->chips-media rename
in consideration.

 drivers/media/platform/chips-media/coda-common.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c
index a57822b05070..53b2dd1b268c 100644
--- a/drivers/media/platform/chips-media/coda-common.c
+++ b/drivers/media/platform/chips-media/coda-common.c
@@ -2344,8 +2344,8 @@ static void coda_encode_ctrls(struct coda_ctx *ctx)
 		V4L2_CID_MPEG_VIDEO_H264_CHROMA_QP_INDEX_OFFSET, -12, 12, 1, 0);
 	v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
 		V4L2_CID_MPEG_VIDEO_H264_PROFILE,
-		V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, 0x0,
-		V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE);
+		V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE, 0x0,
+		V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE);
 	if (ctx->dev->devtype->product == CODA_HX4 ||
 	    ctx->dev->devtype->product == CODA_7541) {
 		v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
@@ -2426,7 +2426,7 @@ static void coda_decode_ctrls(struct coda_ctx *ctx)
 	ctx->h264_profile_ctrl = v4l2_ctrl_new_std_menu(&ctx->ctrls,
 		&coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_H264_PROFILE,
 		V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
-		~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
+		~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE) |
 		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
 		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_HIGH)),
 		V4L2_MPEG_VIDEO_H264_PROFILE_HIGH);
-- 
2.25.1

[PATCH v4 2/2] media: coda: Add more H264 levels for CODA960

[Fabio Estevam]

From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

This add H264 level 4.1, 4.2 and 5.0 to the list of supported formats.
While the hardware does not fully support these levels, it do support
most of them. The constraints on frame size and pixel formats already
cover the limitation.

This fixes negotiation of level on GStreamer 1.17.1.

Cc: stable@vger.kernel.org
Fixes: 42a68012e67c2 ("media: coda: add read-only h.264 decoder profile/level controls")
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Tested-by: Pascal Speck <kernel@iktek.de>
Signed-off-by: Fabio Estevam <festevam@denx.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v3:
- Rebased against linux-next and took the coda->chips-media rename
in consideration.
- Added Philipp's Reviewed-by tag.

 drivers/media/platform/chips-media/coda-common.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c
index 53b2dd1b268c..f1234ad24f65 100644
--- a/drivers/media/platform/chips-media/coda-common.c
+++ b/drivers/media/platform/chips-media/coda-common.c
@@ -2364,7 +2364,10 @@ static void coda_encode_ctrls(struct coda_ctx *ctx)
 			  (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_0) |
 			  (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_1) |
 			  (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_2) |
-			  (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_0)),
+			  (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_0) |
+			  (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_1) |
+			  (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_2) |
+			  (1 << V4L2_MPEG_VIDEO_H264_LEVEL_5_0)),
 			V4L2_MPEG_VIDEO_H264_LEVEL_4_0);
 	}
 	v4l2_ctrl_new_std(&ctx->ctrls, &coda_ctrl_ops,
@@ -2437,7 +2440,7 @@ static void coda_decode_ctrls(struct coda_ctx *ctx)
 	    ctx->dev->devtype->product == CODA_7541)
 		max = V4L2_MPEG_VIDEO_H264_LEVEL_4_0;
 	else if (ctx->dev->devtype->product == CODA_960)
-		max = V4L2_MPEG_VIDEO_H264_LEVEL_4_1;
+		max = V4L2_MPEG_VIDEO_H264_LEVEL_5_0;
 	else
 		return;
 	ctx->h264_level_ctrl = v4l2_ctrl_new_std_menu(&ctx->ctrls,
-- 
2.25.1

Re: [PATCH v4 2/2] media: coda: Add more H264 levels for CODA960

[Philipp Zabel]

Hi Fabio,

On Di, 2022-04-05 at 10:59 -0300, Fabio Estevam wrote:
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> This add H264 level 4.1, 4.2 and 5.0 to the list of supported
> formats.
> While the hardware does not fully support these levels, it do support
> most of them. The constraints on frame size and pixel formats already
> cover the limitation.
> 
> This fixes negotiation of level on GStreamer 1.17.1.
> 
> Cc: stable@vger.kernel.org
> Fixes: 42a68012e67c2 ("media: coda: add read-only h.264 decoder
> profile/level controls")
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> Tested-by: Pascal Speck <kernel@iktek.de>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v3:
> - Rebased against linux-next and took the coda->chips-media rename
> in consideration.
> - Added Philipp's Reviewed-by tag.

I think you rebased the wrong version of this patch.

>  drivers/media/platform/chips-media/coda-common.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/coda-common.c
> b/drivers/media/platform/chips-media/coda-common.c
> index 53b2dd1b268c..f1234ad24f65 100644
> --- a/drivers/media/platform/chips-media/coda-common.c
> +++ b/drivers/media/platform/chips-media/coda-common.c
> @@ -2364,7 +2364,10 @@ static void coda_encode_ctrls(struct coda_ctx
> *ctx)
>                           (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_0) |
>                           (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_1) |
>                           (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_2) |
> -                         (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_0)),
> +                         (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_0) |
> +                         (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_1) |
> +                         (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_2) |
> +                         (1 << V4L2_MPEG_VIDEO_H264_LEVEL_5_0)),

I asked to remove the 5.0 level and to increase the max level to 4.2,
you already fixed this in v3.

>                         V4L2_MPEG_VIDEO_H264_LEVEL_4_0);
>         }
>         v4l2_ctrl_new_std(&ctx->ctrls, &coda_ctrl_ops,
> @@ -2437,7 +2440,7 @@ static void coda_decode_ctrls(struct coda_ctx
> *ctx)
>             ctx->dev->devtype->product == CODA_7541)
>                 max = V4L2_MPEG_VIDEO_H264_LEVEL_4_0;
>         else if (ctx->dev->devtype->product == CODA_960)
> -               max = V4L2_MPEG_VIDEO_H264_LEVEL_4_1;
> +               max = V4L2_MPEG_VIDEO_H264_LEVEL_5_0;

Same as above, there is no way to support level 5.0 frame size
requirements.

regards
Philipp

Re: [PATCH v4 2/2] media: coda: Add more H264 levels for CODA960

[Fabio Estevam]

Hi Philipp,

On Wed, Apr 6, 2022 at 11:59 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:

> I think you rebased the wrong version of this patch.

Oops, good catch!

I have sent v5 with the correct one, thanks.