diff mbox series

[v1,17/24] media: rkvdec: h264: Validate and use pic width and height in mbs

Message ID 20220328195936.82552-18-nicolas.dufresne@collabora.com (mailing list archive)
State New, archived
Headers show
Series [v1,01/24] media: h264: Increase reference lists size to 32 | expand

Commit Message

Nicolas Dufresne March 28, 2022, 7:59 p.m. UTC 
From: Jonas Karlman <jonas@kwiboo.se>

The width and height in mbs is currently configured based on OUTPUT buffer
resolution, this works for frame pictures but can cause issues for field
pictures.

When frame_mbs_only_flag is 0 the height in mbs should be height of
the field instead of height of frame.

Validate pic_width_in_mbs_minus1 and pic_height_in_map_units_minus1
against OUTPUT buffer resolution and use these values to configure HW.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/staging/media/rkvdec/rkvdec-h264.c |  4 ++--
 drivers/staging/media/rkvdec/rkvdec.c      | 10 ++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

Comments

Sebastian Fricke March 30, 2022, 6:48 a.m. UTC | #1 
Hey Nicolas,

On 28.03.2022 15:59, Nicolas Dufresne wrote:
>From: Jonas Karlman <jonas@kwiboo.se>
>
>The width and height in mbs is currently configured based on OUTPUT buffer
>resolution, this works for frame pictures but can cause issues for field
>pictures.

How about:

"""
The width and height measured in macroblocks (mbs) is currently
configured based on the resolution of the OUTPUT buffer, this works for
frame pictures but can cause issues for field pictures..
"""

(I think it improves readability to explain at least once what mbs means
to anyone not aware)

>
>When frame_mbs_only_flag is 0 the height in mbs should be height of
>the field instead of height of frame.
>
>Validate pic_width_in_mbs_minus1 and pic_height_in_map_units_minus1
>against OUTPUT buffer resolution and use these values to configure HW.

s/OUTPUT buffer resolution/the resolution of the OUTPUT buffer/

>
>Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>

Greetings,
Sebastian
>---
> drivers/staging/media/rkvdec/rkvdec-h264.c |  4 ++--
> drivers/staging/media/rkvdec/rkvdec.c      | 10 ++++++++++
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
>index db1e762baee5..847b8957dad3 100644
>--- a/drivers/staging/media/rkvdec/rkvdec-h264.c
>+++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
>@@ -672,8 +672,8 @@ static void assemble_hw_pps(struct rkvdec_ctx *ctx,
> 		  LOG2_MAX_PIC_ORDER_CNT_LSB_MINUS4);
> 	WRITE_PPS(!!(sps->flags & V4L2_H264_SPS_FLAG_DELTA_PIC_ORDER_ALWAYS_ZERO),
> 		  DELTA_PIC_ORDER_ALWAYS_ZERO_FLAG);
>-	WRITE_PPS(DIV_ROUND_UP(ctx->coded_fmt.fmt.pix_mp.width, 16), PIC_WIDTH_IN_MBS);
>-	WRITE_PPS(DIV_ROUND_UP(ctx->coded_fmt.fmt.pix_mp.height, 16), PIC_HEIGHT_IN_MBS);
>+	WRITE_PPS(sps->pic_width_in_mbs_minus1 + 1, PIC_WIDTH_IN_MBS);
>+	WRITE_PPS(sps->pic_height_in_map_units_minus1 + 1, PIC_HEIGHT_IN_MBS);
> 	WRITE_PPS(!!(sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY),
> 		  FRAME_MBS_ONLY_FLAG);
> 	WRITE_PPS(!!(sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD),
>diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
>index 22c0382c579e..67539f4bf382 100644
>--- a/drivers/staging/media/rkvdec/rkvdec.c
>+++ b/drivers/staging/media/rkvdec/rkvdec.c
>@@ -29,8 +29,11 @@
>
> static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
> {
>+	struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
>+
> 	if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
> 		const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
>+		unsigned int width, height;
> 		/*
> 		 * TODO: The hardware supports 10-bit and 4:2:2 profiles,
> 		 * but it's currently broken in the driver.
>@@ -45,6 +48,13 @@ static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
> 		if (sps->bit_depth_luma_minus8 != 0)
> 			/* Only 8-bit is supported */
> 			return -EINVAL;
>+
>+		width = (sps->pic_width_in_mbs_minus1 + 1) * 16;
>+		height = (sps->pic_height_in_map_units_minus1 + 1) * 16;
>+
>+		if (width > ctx->coded_fmt.fmt.pix_mp.width ||
>+		    height > ctx->coded_fmt.fmt.pix_mp.height)
>+			return -EINVAL;
> 	}
> 	return 0;
> }
>-- 
>2.34.1
>
diff mbox series

Patch

diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
index db1e762baee5..847b8957dad3 100644
--- a/drivers/staging/media/rkvdec/rkvdec-h264.c
+++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
@@ -672,8 +672,8 @@  static void assemble_hw_pps(struct rkvdec_ctx *ctx,
 		  LOG2_MAX_PIC_ORDER_CNT_LSB_MINUS4);
 	WRITE_PPS(!!(sps->flags & V4L2_H264_SPS_FLAG_DELTA_PIC_ORDER_ALWAYS_ZERO),
 		  DELTA_PIC_ORDER_ALWAYS_ZERO_FLAG);
-	WRITE_PPS(DIV_ROUND_UP(ctx->coded_fmt.fmt.pix_mp.width, 16), PIC_WIDTH_IN_MBS);
-	WRITE_PPS(DIV_ROUND_UP(ctx->coded_fmt.fmt.pix_mp.height, 16), PIC_HEIGHT_IN_MBS);
+	WRITE_PPS(sps->pic_width_in_mbs_minus1 + 1, PIC_WIDTH_IN_MBS);
+	WRITE_PPS(sps->pic_height_in_map_units_minus1 + 1, PIC_HEIGHT_IN_MBS);
 	WRITE_PPS(!!(sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY),
 		  FRAME_MBS_ONLY_FLAG);
 	WRITE_PPS(!!(sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD),
diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
index 22c0382c579e..67539f4bf382 100644
--- a/drivers/staging/media/rkvdec/rkvdec.c
+++ b/drivers/staging/media/rkvdec/rkvdec.c
@@ -29,8 +29,11 @@ 
 
 static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
 {
+	struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
+
 	if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
 		const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
+		unsigned int width, height;
 		/*
 		 * TODO: The hardware supports 10-bit and 4:2:2 profiles,
 		 * but it's currently broken in the driver.
@@ -45,6 +48,13 @@  static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
 		if (sps->bit_depth_luma_minus8 != 0)
 			/* Only 8-bit is supported */
 			return -EINVAL;
+
+		width = (sps->pic_width_in_mbs_minus1 + 1) * 16;
+		height = (sps->pic_height_in_map_units_minus1 + 1) * 16;
+
+		if (width > ctx->coded_fmt.fmt.pix_mp.width ||
+		    height > ctx->coded_fmt.fmt.pix_mp.height)
+			return -EINVAL;
 	}
 	return 0;
 }