Until now, only Cedrus (a slice base decoder) supported interlaced
decoding. In order to support field decoding in our frame-based decoder,
the v4l2-h264 library needed adaptation to produce the appropriate
reference lists.
This patch extends the v4l2-h264 library to produce the larger references
list needed to represent fields separately. Hantro, MTK-VCODEC and RKVDEC
drivers have been adapted to accommodate the larger lists. Though, only
Hantro and RKVDEC actually have HW support for field decoding. So only
these two have been updated to make use of the larger lists. All this work
has been done using the H.264 specification, LibreELEC downstream kernel
patches, Rockchip MPP reference software and Hantro reference software.
For reviewers, the following is the map of all commit. Patches that could
be merge independently of this serie are marked as independent. Note that
the test results do depend on the generic fixes.
01 : Documentation fix (independent)
02-03 : Improving some generic traces (independent)
04 : Minor v4l2-h264 fix (independent)
05-11 : v4l2-h264 field decoding support
12-15 : rkvdec h.264 generic fixes (independent)
16-19 : rkvdec h.264 field decoding support
20-23 : hantro h.264 field decoding support
All this work have been tested using GStreamer mainline implementation
but also with FFMPEG LibreELEC fork using the testing tool fluster
running through the ITU-T H.264 (2016-02) AVCv2 set of bitstream. Before
this patch, the scores were:
Hantro:
FFMPEG: 88/135
GSteamer: 90/135
RKVDEC:
FFMPEG: 73/135
GSteamer: 77/135
And after these changes:
Hantro:
FFMPEG: 118/135
GSteamer: 129/135
RKVDEC:
FFMPEG: 118/135
GSteamer: 129/135
Note that a bug in FFMPEG / LibreELEC fork was noticed and fixed with the
following change:
diff --git a/libavcodec/v4l2_request_h264.c b/libavcodec/v4l2_request_h264.c
index 88da8f0a2d..394bae0550 100644
--- a/libavcodec/v4l2_request_h264.c
+++ b/libavcodec/v4l2_request_h264.c
@@ -66,7 +66,7 @@ static void fill_dpb_entry(struct v4l2_h264_dpb_entry *entry, const H264Picture
{
entry->reference_ts = ff_v4l2_request_get_capture_timestamp(pic->f);
entry->pic_num = pic->pic_id;
- entry->frame_num = pic->frame_num;
+ entry->frame_num = pic->long_ref ? pic->pic_id : pic->frame_num;
entry->fields = pic->reference & V4L2_H264_FRAME_REF;
entry->flags = V4L2_H264_DPB_ENTRY_FLAG_VALID;
if (entry->fields)
Some useful links:
Detailed Hantro Results: https://gitlab.freedesktop.org/-/snippets/5189
Detailed RKVDEC Results: https://gitlab.freedesktop.org/-/snippets/5253
ITU-T H.264 (2016-02) AVCv2: https://www.itu.int/net/itu-t/sigdb/spevideo/VideoForm-s.aspx?val=102002641
Fluster: https://github.com/fluendo/fluster
GStreamer: https://gitlab.freedesktop.org/gstreamer/gstreamer/
FFMPEG Fork: https://github.com/jernejsk/FFmpeg/tree/v4l2-request-hwaccel-4.4
Rockchip MPP: https://github.com/rockchip-linux/mpp
Changes in v2:
- Applied most of Sebastian's suggestion in comments and commit messages.
- Use a bool for dpb_valid and dpb_bottom in rkvdec
- Dropped one wrong typo fix (media: v4l2-mem2mem: Fix typo in trace message)
- Dropped Alex fix (media: rkvdec-h264: Don't hardcode SPS/PPS parameters
+ I will carry this one later, it seems cosmetic
Jonas Karlman (5):
media: rkvdec: h264: Fix bit depth wrap in pps packet
media: rkvdec: h264: Validate and use pic width and height in mbs
media: rkvdec: h264: Fix reference frame_num wrap for second field
media: rkvdec: Ensure decoded resolution fit coded resolution
media: hantro: h264: Make dpb entry management more robust
Nicolas Dufresne (17):
media: doc: Document dual use of H.264 pic_num/frame_num
media: v4l2-mem2mem: Trace on implicit un-hold
media: h264: Avoid wrapping long_term_frame_idx
media: h264: Use v4l2_h264_reference for reflist
media: h264: Increase reference lists size to 32
media: h264: Store current picture fields
media: h264: Store all fields into the unordered list
media: v4l2: Trace calculated p/b0/b1 initial reflist
media: h264: Sort p/b reflist using frame_num
media: v4l2: Reorder field reflist
media: rkvdec: Stop overclocking the decoder
media: rkvdec: h264: Fix dpb_valid implementation
media: rkvdec: Enable capture buffer holding for H264
media: rkvdec-h264: Add field decoding support
media: hantro: Enable HOLD_CAPTURE_BUF for H.264
media: hantro: Stop using H.264 parameter pic_num
media: hantro: Add H.264 field decoding support
Sebastian Fricke (1):
media: videobuf2-v4l2: Warn on holding buffers without support
.../media/v4l/ext-ctrls-codec-stateless.rst | 10 +-
.../media/common/videobuf2/videobuf2-v4l2.c | 7 +-
.../mediatek/vcodec/vdec/vdec_h264_req_if.c | 17 +-
drivers/media/v4l2-core/v4l2-h264.c | 261 ++++++++++++++----
drivers/media/v4l2-core/v4l2-mem2mem.c | 1 +
.../staging/media/hantro/hantro_g1_h264_dec.c | 38 +--
drivers/staging/media/hantro/hantro_h264.c | 119 ++++++--
drivers/staging/media/hantro/hantro_hw.h | 7 +-
drivers/staging/media/hantro/hantro_v4l2.c | 25 ++
.../media/hantro/rockchip_vpu2_hw_h264_dec.c | 98 +++----
drivers/staging/media/rkvdec/rkvdec-h264.c | 91 +++---
drivers/staging/media/rkvdec/rkvdec.c | 22 +-
drivers/staging/media/rkvdec/rkvdec.h | 1 +
include/media/v4l2-h264.h | 31 ++-
14 files changed, 520 insertions(+), 208 deletions(-)
--
2.34.1
From: Jonas Karlman <[email protected]>
When decoding the second field in a complementary field pair the second
field is sharing the same frame_num with the first field.
Currently the frame_num for the first field is wrapped when it matches the
field being decoded, this cause issues to decode the second field in a
complementary field pair.
Fix this by using inclusive comparison, less than or equal.
Signed-off-by: Jonas Karlman <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
drivers/staging/media/rkvdec/rkvdec-h264.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
index a42cf19bcc6d..730f8ebf7f58 100644
--- a/drivers/staging/media/rkvdec/rkvdec-h264.c
+++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
@@ -773,7 +773,7 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
continue;
if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM ||
- dpb[i].frame_num < dec_params->frame_num) {
+ dpb[i].frame_num <= dec_params->frame_num) {
p[i] = dpb[i].frame_num;
continue;
}
--
2.34.1
From: Jonas Karlman <[email protected]>
The driver maintains stable slot locations for reference pictures. This
change makes the code more robust by using the reference_ts as key and
by marking all entries invalid right from the start.
Signed-off-by: Jonas Karlman <[email protected]>
Signed-off-by: Nicolas Dufresne <[email protected]>
---
drivers/staging/media/hantro/hantro_h264.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_h264.c b/drivers/staging/media/hantro/hantro_h264.c
index 228629fb3cdf..7377fc26f780 100644
--- a/drivers/staging/media/hantro/hantro_h264.c
+++ b/drivers/staging/media/hantro/hantro_h264.c
@@ -258,8 +258,7 @@ static void prepare_table(struct hantro_ctx *ctx)
static bool dpb_entry_match(const struct v4l2_h264_dpb_entry *a,
const struct v4l2_h264_dpb_entry *b)
{
- return a->top_field_order_cnt == b->top_field_order_cnt &&
- a->bottom_field_order_cnt == b->bottom_field_order_cnt;
+ return a->reference_ts == b->reference_ts;
}
static void update_dpb(struct hantro_ctx *ctx)
@@ -273,13 +272,13 @@ static void update_dpb(struct hantro_ctx *ctx)
/* Disable all entries by default. */
for (i = 0; i < ARRAY_SIZE(ctx->h264_dec.dpb); i++)
- ctx->h264_dec.dpb[i].flags &= ~V4L2_H264_DPB_ENTRY_FLAG_ACTIVE;
+ ctx->h264_dec.dpb[i].flags = 0;
/* Try to match new DPB entries with existing ones by their POCs. */
for (i = 0; i < ARRAY_SIZE(dec_param->dpb); i++) {
const struct v4l2_h264_dpb_entry *ndpb = &dec_param->dpb[i];
- if (!(ndpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
+ if (!(ndpb->flags & V4L2_H264_DPB_ENTRY_FLAG_VALID))
continue;
/*
@@ -290,8 +289,7 @@ static void update_dpb(struct hantro_ctx *ctx)
struct v4l2_h264_dpb_entry *cdpb;
cdpb = &ctx->h264_dec.dpb[j];
- if (cdpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE ||
- !dpb_entry_match(cdpb, ndpb))
+ if (!dpb_entry_match(cdpb, ndpb))
continue;
*cdpb = *ndpb;
--
2.34.1
In the reference list builder, frame_num refers to FrameNumWrap
in the spec, which is the same as the pic_num for frame decoding.
The same applies for long_term_pic_num and long_term_frame_idx.
Sort all type of references by frame_num so the sort can be reused
for fields reflist were the sorting is done using frame_num instead.
In short, pic_num is never actually used for building reference
lists.
Signed-off-by: Nicolas Dufresne <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
drivers/media/v4l2-core/v4l2-h264.c | 23 +++++++++++++----------
include/media/v4l2-h264.h | 2 --
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
index bcf9b7774560..7e1eba03099a 100644
--- a/drivers/media/v4l2-core/v4l2-h264.c
+++ b/drivers/media/v4l2-core/v4l2-h264.c
@@ -50,7 +50,6 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
if (!(dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
continue;
- b->refs[i].pic_num = dpb[i].pic_num;
if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM)
b->refs[i].longterm = true;
@@ -139,15 +138,19 @@ static int v4l2_h264_p_ref_list_cmp(const void *ptra, const void *ptrb,
}
/*
- * Short term pics in descending pic num order, long term ones in
- * ascending order.
+ * For frames, short term pics are in descending pic num order and long
+ * term ones in ascending order. For fields, the same direction is used
+ * but with frame_num (wrapped). For frames, the value of pic_num and
+ * frame_num are the same (see formula (8-28) and (8-29)). For this
+ * reason we can use frame_num only and share this funciton between
+ * frames and fields reflist.
*/
if (!builder->refs[idxa].longterm)
return builder->refs[idxb].frame_num <
builder->refs[idxa].frame_num ?
-1 : 1;
- return builder->refs[idxa].pic_num < builder->refs[idxb].pic_num ?
+ return builder->refs[idxa].frame_num < builder->refs[idxb].frame_num ?
-1 : 1;
}
@@ -173,10 +176,10 @@ static int v4l2_h264_b0_ref_list_cmp(const void *ptra, const void *ptrb,
return 1;
}
- /* Long term pics in ascending pic num order. */
+ /* Long term pics in ascending frame num order. */
if (builder->refs[idxa].longterm)
- return builder->refs[idxa].pic_num <
- builder->refs[idxb].pic_num ?
+ return builder->refs[idxa].frame_num <
+ builder->refs[idxb].frame_num ?
-1 : 1;
poca = v4l2_h264_get_poc(builder, ptra);
@@ -218,10 +221,10 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
return 1;
}
- /* Long term pics in ascending pic num order. */
+ /* Long term pics in ascending frame num order. */
if (builder->refs[idxa].longterm)
- return builder->refs[idxa].pic_num <
- builder->refs[idxb].pic_num ?
+ return builder->refs[idxa].frame_num <
+ builder->refs[idxb].frame_num ?
-1 : 1;
poca = v4l2_h264_get_poc(builder, ptra);
diff --git a/include/media/v4l2-h264.h b/include/media/v4l2-h264.h
index 4cef717b3f18..0d9eaa956123 100644
--- a/include/media/v4l2-h264.h
+++ b/include/media/v4l2-h264.h
@@ -18,7 +18,6 @@
* @refs.top_field_order_cnt: top field order count
* @refs.bottom_field_order_cnt: bottom field order count
* @refs.frame_num: reference frame number
- * @refs.pic_num: reference picture number
* @refs.longterm: set to true for a long term reference
* @refs: array of references
* @cur_pic_order_count: picture order count of the frame being decoded
@@ -36,7 +35,6 @@ struct v4l2_h264_reflist_builder {
s32 top_field_order_cnt;
s32 bottom_field_order_cnt;
int frame_num;
- u32 pic_num;
u16 longterm : 1;
} refs[V4L2_H264_NUM_DPB_ENTRIES];
--
2.34.1
From: Jonas Karlman <[email protected]>
The width and height in macroblocks 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 <[email protected]>
Signed-off-by: Nicolas Dufresne <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
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 8d44a884a52e..a42cf19bcc6d 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 2df8cf4883e2..1b805710e195 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
This information, also called picture structure, is required in field
decoding mode to construct reference lists.
Signed-off-by: Nicolas Dufresne <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
drivers/media/v4l2-core/v4l2-h264.c | 10 +++++++---
include/media/v4l2-h264.h | 4 ++++
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
index 4b46b36526c0..58f18bb0afb6 100644
--- a/drivers/media/v4l2-core/v4l2-h264.c
+++ b/drivers/media/v4l2-core/v4l2-h264.c
@@ -34,13 +34,17 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
cur_frame_num = dec_params->frame_num;
memset(b, 0, sizeof(*b));
- if (!(dec_params->flags & V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC))
+ if (!(dec_params->flags & V4L2_H264_DECODE_PARAM_FLAG_FIELD_PIC)) {
b->cur_pic_order_count = min(dec_params->bottom_field_order_cnt,
dec_params->top_field_order_cnt);
- else if (dec_params->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD)
+ b->cur_pic_fields = V4L2_H264_FRAME_REF;
+ } else if (dec_params->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD) {
b->cur_pic_order_count = dec_params->bottom_field_order_cnt;
- else
+ b->cur_pic_fields = V4L2_H264_BOTTOM_FIELD_REF;
+ } else {
b->cur_pic_order_count = dec_params->top_field_order_cnt;
+ b->cur_pic_fields = V4L2_H264_TOP_FIELD_REF;
+ }
for (i = 0; i < V4L2_H264_NUM_DPB_ENTRIES; i++) {
u32 pic_order_count;
diff --git a/include/media/v4l2-h264.h b/include/media/v4l2-h264.h
index e282fb16ac58..e165a54c68fa 100644
--- a/include/media/v4l2-h264.h
+++ b/include/media/v4l2-h264.h
@@ -21,6 +21,7 @@
* @refs.longterm: set to true for a long term reference
* @refs: array of references
* @cur_pic_order_count: picture order count of the frame being decoded
+ * @cur_pic_fields: fields present in the frame being decoded
* @unordered_reflist: unordered list of references. Will be used to generate
* ordered P/B0/B1 lists
* @num_valid: number of valid references in the refs array
@@ -36,7 +37,10 @@ struct v4l2_h264_reflist_builder {
u32 pic_num;
u16 longterm : 1;
} refs[V4L2_H264_NUM_DPB_ENTRIES];
+
s32 cur_pic_order_count;
+ u8 cur_pic_fields;
+
struct v4l2_h264_reference unordered_reflist[V4L2_H264_REF_LIST_LEN];
u8 num_valid;
};
--
2.34.1
In preparation for adding field decoding support, convert the byte arrays
for reflist into array of struct v4l2_h264_reference. That struct will
allow us to mark which field of the reference picture is being referenced.
Signed-off-by: Nicolas Dufresne <[email protected]>
---
.../mediatek/vcodec/vdec/vdec_h264_req_if.c | 17 +++-
drivers/media/v4l2-core/v4l2-h264.c | 33 ++++---
.../staging/media/hantro/hantro_g1_h264_dec.c | 38 +++----
drivers/staging/media/hantro/hantro_hw.h | 6 +-
.../media/hantro/rockchip_vpu2_hw_h264_dec.c | 98 +++++++++----------
drivers/staging/media/rkvdec/rkvdec-h264.c | 12 +--
include/media/v4l2-h264.h | 19 ++--
7 files changed, 116 insertions(+), 107 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
index 43542de11e9c..72c599e05a47 100644
--- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
+++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
@@ -442,9 +442,16 @@ static void update_dpb(const struct v4l2_ctrl_h264_decode_params *dec_param,
/*
* The firmware expects unused reflist entries to have the value 0x20.
*/
-static void fixup_ref_list(u8 *ref_list, size_t num_valid)
+static void get_ref_list(u8 *ref_list, struct v4l2_h264_reflist_builder *b)
{
- memset(&ref_list[num_valid], 0x20, 32 - num_valid);
+ u32 i;
+
+ /* FIXME mark the reference parity */
+ for (i = 0; i < b->num_valid; i++)
+ ref_list[i] = b->index;
+
+ for (; i < 32; i++)
+ ref_list[i] = 0x20;
}
static void get_vdec_decode_parameters(struct vdec_h264_slice_inst *inst)
@@ -478,9 +485,9 @@ static void get_vdec_decode_parameters(struct vdec_h264_slice_inst *inst)
v4l2_h264_build_p_ref_list(&reflist_builder, p0_reflist);
v4l2_h264_build_b_ref_lists(&reflist_builder, b0_reflist, b1_reflist);
/* Adapt the built lists to the firmware's expectations */
- fixup_ref_list(p0_reflist, reflist_builder.num_valid);
- fixup_ref_list(b0_reflist, reflist_builder.num_valid);
- fixup_ref_list(b1_reflist, reflist_builder.num_valid);
+ get_ref_list(p0_reflist, reflist_builder);
+ get_ref_list(b0_reflist, reflist_builder);
+ get_ref_list(b1_reflist, reflist_builder);
memcpy(&inst->vsi_ctx.h264_slice_params, slice_param,
sizeof(inst->vsi_ctx.h264_slice_params));
diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
index ac47519a9fbe..afbfcf78efe4 100644
--- a/drivers/media/v4l2-core/v4l2-h264.c
+++ b/drivers/media/v4l2-core/v4l2-h264.c
@@ -75,12 +75,12 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
pic_order_count = dpb[i].top_field_order_cnt;
b->refs[i].pic_order_count = pic_order_count;
- b->unordered_reflist[b->num_valid] = i;
+ b->unordered_reflist[b->num_valid].index = i;
b->num_valid++;
}
for (i = b->num_valid; i < ARRAY_SIZE(b->unordered_reflist); i++)
- b->unordered_reflist[i] = i;
+ b->unordered_reflist[i].index = i;
}
EXPORT_SYMBOL_GPL(v4l2_h264_init_reflist_builder);
@@ -90,8 +90,8 @@ static int v4l2_h264_p_ref_list_cmp(const void *ptra, const void *ptrb,
const struct v4l2_h264_reflist_builder *builder = data;
u8 idxa, idxb;
- idxa = *((u8 *)ptra);
- idxb = *((u8 *)ptrb);
+ idxa = ((struct v4l2_h264_reference *)ptra)->index;
+ idxb = ((struct v4l2_h264_reference *)ptrb)->index;
if (WARN_ON(idxa >= V4L2_H264_NUM_DPB_ENTRIES ||
idxb >= V4L2_H264_NUM_DPB_ENTRIES))
@@ -125,8 +125,8 @@ static int v4l2_h264_b0_ref_list_cmp(const void *ptra, const void *ptrb,
s32 poca, pocb;
u8 idxa, idxb;
- idxa = *((u8 *)ptra);
- idxb = *((u8 *)ptrb);
+ idxa = ((struct v4l2_h264_reference *)ptra)->index;
+ idxb = ((struct v4l2_h264_reference *)ptrb)->index;
if (WARN_ON(idxa >= V4L2_H264_NUM_DPB_ENTRIES ||
idxb >= V4L2_H264_NUM_DPB_ENTRIES))
@@ -170,8 +170,8 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
s32 poca, pocb;
u8 idxa, idxb;
- idxa = *((u8 *)ptra);
- idxb = *((u8 *)ptrb);
+ idxa = ((struct v4l2_h264_reference *)ptra)->index;
+ idxb = ((struct v4l2_h264_reference *)ptrb)->index;
if (WARN_ON(idxa >= V4L2_H264_NUM_DPB_ENTRIES ||
idxb >= V4L2_H264_NUM_DPB_ENTRIES))
@@ -212,8 +212,8 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
* v4l2_h264_build_p_ref_list() - Build the P reference list
*
* @builder: reference list builder context
- * @reflist: 16-bytes array used to store the P reference list. Each entry
- * is an index in the DPB
+ * @reflist: 16 sized array used to store the P reference list. Each entry
+ * is a v4l2_h264_reference structure
*
* This functions builds the P reference lists. This procedure is describe in
* section '8.2.4 Decoding process for reference picture lists construction'
@@ -222,7 +222,7 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
*/
void
v4l2_h264_build_p_ref_list(const struct v4l2_h264_reflist_builder *builder,
- u8 *reflist)
+ struct v4l2_h264_reference *reflist)
{
memcpy(reflist, builder->unordered_reflist,
sizeof(builder->unordered_reflist[0]) * builder->num_valid);
@@ -235,10 +235,10 @@ EXPORT_SYMBOL_GPL(v4l2_h264_build_p_ref_list);
* v4l2_h264_build_b_ref_lists() - Build the B0/B1 reference lists
*
* @builder: reference list builder context
- * @b0_reflist: 16-bytes array used to store the B0 reference list. Each entry
- * is an index in the DPB
- * @b1_reflist: 16-bytes array used to store the B1 reference list. Each entry
- * is an index in the DPB
+ * @b0_reflist: 16 sized array used to store the B0 reference list. Each entry
+ * is a v4l2_h264_reference structure
+ * @b1_reflist: 16 sized array used to store the B1 reference list. Each entry
+ * is a v4l2_h264_reference structure
*
* This functions builds the B0/B1 reference lists. This procedure is described
* in section '8.2.4 Decoding process for reference picture lists construction'
@@ -247,7 +247,8 @@ EXPORT_SYMBOL_GPL(v4l2_h264_build_p_ref_list);
*/
void
v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
- u8 *b0_reflist, u8 *b1_reflist)
+ struct v4l2_h264_reference *b0_reflist,
+ struct v4l2_h264_reference *b1_reflist)
{
memcpy(b0_reflist, builder->unordered_reflist,
sizeof(builder->unordered_reflist[0]) * builder->num_valid);
diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
index f49dbfb8a843..9de7f05eff2a 100644
--- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
@@ -126,7 +126,7 @@ static void set_params(struct hantro_ctx *ctx, struct vb2_v4l2_buffer *src_buf)
static void set_ref(struct hantro_ctx *ctx)
{
- const u8 *b0_reflist, *b1_reflist, *p_reflist;
+ const struct v4l2_h264_reference *b0_reflist, *b1_reflist, *p_reflist;
struct hantro_dev *vpu = ctx->dev;
int reg_num;
u32 reg;
@@ -157,12 +157,12 @@ static void set_ref(struct hantro_ctx *ctx)
*/
reg_num = 0;
for (i = 0; i < 15; i += 3) {
- reg = G1_REG_BD_REF_PIC_BINIT_RLIST_F0(b0_reflist[i]) |
- G1_REG_BD_REF_PIC_BINIT_RLIST_F1(b0_reflist[i + 1]) |
- G1_REG_BD_REF_PIC_BINIT_RLIST_F2(b0_reflist[i + 2]) |
- G1_REG_BD_REF_PIC_BINIT_RLIST_B0(b1_reflist[i]) |
- G1_REG_BD_REF_PIC_BINIT_RLIST_B1(b1_reflist[i + 1]) |
- G1_REG_BD_REF_PIC_BINIT_RLIST_B2(b1_reflist[i + 2]);
+ reg = G1_REG_BD_REF_PIC_BINIT_RLIST_F0(b0_reflist[i].index) |
+ G1_REG_BD_REF_PIC_BINIT_RLIST_F1(b0_reflist[i + 1].index) |
+ G1_REG_BD_REF_PIC_BINIT_RLIST_F2(b0_reflist[i + 2].index) |
+ G1_REG_BD_REF_PIC_BINIT_RLIST_B0(b1_reflist[i].index) |
+ G1_REG_BD_REF_PIC_BINIT_RLIST_B1(b1_reflist[i + 1].index) |
+ G1_REG_BD_REF_PIC_BINIT_RLIST_B2(b1_reflist[i + 2].index);
vdpu_write_relaxed(vpu, reg, G1_REG_BD_REF_PIC(reg_num++));
}
@@ -171,12 +171,12 @@ static void set_ref(struct hantro_ctx *ctx)
* of forward and backward reference picture lists and first 4 entries
* of P forward picture list.
*/
- reg = G1_REG_BD_P_REF_PIC_BINIT_RLIST_F15(b0_reflist[15]) |
- G1_REG_BD_P_REF_PIC_BINIT_RLIST_B15(b1_reflist[15]) |
- G1_REG_BD_P_REF_PIC_PINIT_RLIST_F0(p_reflist[0]) |
- G1_REG_BD_P_REF_PIC_PINIT_RLIST_F1(p_reflist[1]) |
- G1_REG_BD_P_REF_PIC_PINIT_RLIST_F2(p_reflist[2]) |
- G1_REG_BD_P_REF_PIC_PINIT_RLIST_F3(p_reflist[3]);
+ reg = G1_REG_BD_P_REF_PIC_BINIT_RLIST_F15(b0_reflist[15].index) |
+ G1_REG_BD_P_REF_PIC_BINIT_RLIST_B15(b1_reflist[15].index) |
+ G1_REG_BD_P_REF_PIC_PINIT_RLIST_F0(p_reflist[0].index) |
+ G1_REG_BD_P_REF_PIC_PINIT_RLIST_F1(p_reflist[1].index) |
+ G1_REG_BD_P_REF_PIC_PINIT_RLIST_F2(p_reflist[2].index) |
+ G1_REG_BD_P_REF_PIC_PINIT_RLIST_F3(p_reflist[3].index);
vdpu_write_relaxed(vpu, reg, G1_REG_BD_P_REF_PIC);
/*
@@ -185,12 +185,12 @@ static void set_ref(struct hantro_ctx *ctx)
*/
reg_num = 0;
for (i = 4; i < HANTRO_H264_DPB_SIZE; i += 6) {
- reg = G1_REG_FWD_PIC_PINIT_RLIST_F0(p_reflist[i]) |
- G1_REG_FWD_PIC_PINIT_RLIST_F1(p_reflist[i + 1]) |
- G1_REG_FWD_PIC_PINIT_RLIST_F2(p_reflist[i + 2]) |
- G1_REG_FWD_PIC_PINIT_RLIST_F3(p_reflist[i + 3]) |
- G1_REG_FWD_PIC_PINIT_RLIST_F4(p_reflist[i + 4]) |
- G1_REG_FWD_PIC_PINIT_RLIST_F5(p_reflist[i + 5]);
+ reg = G1_REG_FWD_PIC_PINIT_RLIST_F0(p_reflist[i].index) |
+ G1_REG_FWD_PIC_PINIT_RLIST_F1(p_reflist[i + 1].index) |
+ G1_REG_FWD_PIC_PINIT_RLIST_F2(p_reflist[i + 2].index) |
+ G1_REG_FWD_PIC_PINIT_RLIST_F3(p_reflist[i + 3].index) |
+ G1_REG_FWD_PIC_PINIT_RLIST_F4(p_reflist[i + 4].index) |
+ G1_REG_FWD_PIC_PINIT_RLIST_F5(p_reflist[i + 5].index);
vdpu_write_relaxed(vpu, reg, G1_REG_FWD_PIC(reg_num++));
}
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index ed018e293ba0..2bc6b8f088f5 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -69,9 +69,9 @@ struct hantro_h264_dec_ctrls {
* @b1: B1 reflist
*/
struct hantro_h264_dec_reflists {
- u8 p[HANTRO_H264_DPB_SIZE];
- u8 b0[HANTRO_H264_DPB_SIZE];
- u8 b1[HANTRO_H264_DPB_SIZE];
+ struct v4l2_h264_reference p[HANTRO_H264_DPB_SIZE];
+ struct v4l2_h264_reference b0[HANTRO_H264_DPB_SIZE];
+ struct v4l2_h264_reference b1[HANTRO_H264_DPB_SIZE];
};
/**
diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c b/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c
index 64a6330475eb..46c1a83bcc4e 100644
--- a/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c
+++ b/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c
@@ -298,7 +298,7 @@ static void set_params(struct hantro_ctx *ctx, struct vb2_v4l2_buffer *src_buf)
static void set_ref(struct hantro_ctx *ctx)
{
- const u8 *b0_reflist, *b1_reflist, *p_reflist;
+ const struct v4l2_h264_reference *b0_reflist, *b1_reflist, *p_reflist;
struct hantro_dev *vpu = ctx->dev;
u32 reg;
int i;
@@ -307,20 +307,20 @@ static void set_ref(struct hantro_ctx *ctx)
b1_reflist = ctx->h264_dec.reflists.b1;
p_reflist = ctx->h264_dec.reflists.p;
- reg = VDPU_REG_PINIT_RLIST_F9(p_reflist[9]) |
- VDPU_REG_PINIT_RLIST_F8(p_reflist[8]) |
- VDPU_REG_PINIT_RLIST_F7(p_reflist[7]) |
- VDPU_REG_PINIT_RLIST_F6(p_reflist[6]) |
- VDPU_REG_PINIT_RLIST_F5(p_reflist[5]) |
- VDPU_REG_PINIT_RLIST_F4(p_reflist[4]);
+ reg = VDPU_REG_PINIT_RLIST_F9(p_reflist[9].index) |
+ VDPU_REG_PINIT_RLIST_F8(p_reflist[8].index) |
+ VDPU_REG_PINIT_RLIST_F7(p_reflist[7].index) |
+ VDPU_REG_PINIT_RLIST_F6(p_reflist[6].index) |
+ VDPU_REG_PINIT_RLIST_F5(p_reflist[5].index) |
+ VDPU_REG_PINIT_RLIST_F4(p_reflist[4].index);
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(74));
- reg = VDPU_REG_PINIT_RLIST_F15(p_reflist[15]) |
- VDPU_REG_PINIT_RLIST_F14(p_reflist[14]) |
- VDPU_REG_PINIT_RLIST_F13(p_reflist[13]) |
- VDPU_REG_PINIT_RLIST_F12(p_reflist[12]) |
- VDPU_REG_PINIT_RLIST_F11(p_reflist[11]) |
- VDPU_REG_PINIT_RLIST_F10(p_reflist[10]);
+ reg = VDPU_REG_PINIT_RLIST_F15(p_reflist[15].index) |
+ VDPU_REG_PINIT_RLIST_F14(p_reflist[14].index) |
+ VDPU_REG_PINIT_RLIST_F13(p_reflist[13].index) |
+ VDPU_REG_PINIT_RLIST_F12(p_reflist[12].index) |
+ VDPU_REG_PINIT_RLIST_F11(p_reflist[11].index) |
+ VDPU_REG_PINIT_RLIST_F10(p_reflist[10].index);
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(75));
reg = VDPU_REG_REFER1_NBR(hantro_h264_get_ref_nbr(ctx, 1)) |
@@ -355,54 +355,54 @@ static void set_ref(struct hantro_ctx *ctx)
VDPU_REG_REFER14_NBR(hantro_h264_get_ref_nbr(ctx, 14));
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(83));
- reg = VDPU_REG_BINIT_RLIST_F5(b0_reflist[5]) |
- VDPU_REG_BINIT_RLIST_F4(b0_reflist[4]) |
- VDPU_REG_BINIT_RLIST_F3(b0_reflist[3]) |
- VDPU_REG_BINIT_RLIST_F2(b0_reflist[2]) |
- VDPU_REG_BINIT_RLIST_F1(b0_reflist[1]) |
- VDPU_REG_BINIT_RLIST_F0(b0_reflist[0]);
+ reg = VDPU_REG_BINIT_RLIST_F5(b0_reflist[5].index) |
+ VDPU_REG_BINIT_RLIST_F4(b0_reflist[4].index) |
+ VDPU_REG_BINIT_RLIST_F3(b0_reflist[3].index) |
+ VDPU_REG_BINIT_RLIST_F2(b0_reflist[2].index) |
+ VDPU_REG_BINIT_RLIST_F1(b0_reflist[1].index) |
+ VDPU_REG_BINIT_RLIST_F0(b0_reflist[0].index);
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(100));
- reg = VDPU_REG_BINIT_RLIST_F11(b0_reflist[11]) |
- VDPU_REG_BINIT_RLIST_F10(b0_reflist[10]) |
- VDPU_REG_BINIT_RLIST_F9(b0_reflist[9]) |
- VDPU_REG_BINIT_RLIST_F8(b0_reflist[8]) |
- VDPU_REG_BINIT_RLIST_F7(b0_reflist[7]) |
- VDPU_REG_BINIT_RLIST_F6(b0_reflist[6]);
+ reg = VDPU_REG_BINIT_RLIST_F11(b0_reflist[11].index) |
+ VDPU_REG_BINIT_RLIST_F10(b0_reflist[10].index) |
+ VDPU_REG_BINIT_RLIST_F9(b0_reflist[9].index) |
+ VDPU_REG_BINIT_RLIST_F8(b0_reflist[8].index) |
+ VDPU_REG_BINIT_RLIST_F7(b0_reflist[7].index) |
+ VDPU_REG_BINIT_RLIST_F6(b0_reflist[6].index);
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(101));
- reg = VDPU_REG_BINIT_RLIST_F15(b0_reflist[15]) |
- VDPU_REG_BINIT_RLIST_F14(b0_reflist[14]) |
- VDPU_REG_BINIT_RLIST_F13(b0_reflist[13]) |
- VDPU_REG_BINIT_RLIST_F12(b0_reflist[12]);
+ reg = VDPU_REG_BINIT_RLIST_F15(b0_reflist[15].index) |
+ VDPU_REG_BINIT_RLIST_F14(b0_reflist[14].index) |
+ VDPU_REG_BINIT_RLIST_F13(b0_reflist[13].index) |
+ VDPU_REG_BINIT_RLIST_F12(b0_reflist[12].index);
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(102));
- reg = VDPU_REG_BINIT_RLIST_B5(b1_reflist[5]) |
- VDPU_REG_BINIT_RLIST_B4(b1_reflist[4]) |
- VDPU_REG_BINIT_RLIST_B3(b1_reflist[3]) |
- VDPU_REG_BINIT_RLIST_B2(b1_reflist[2]) |
- VDPU_REG_BINIT_RLIST_B1(b1_reflist[1]) |
- VDPU_REG_BINIT_RLIST_B0(b1_reflist[0]);
+ reg = VDPU_REG_BINIT_RLIST_B5(b1_reflist[5].index) |
+ VDPU_REG_BINIT_RLIST_B4(b1_reflist[4].index) |
+ VDPU_REG_BINIT_RLIST_B3(b1_reflist[3].index) |
+ VDPU_REG_BINIT_RLIST_B2(b1_reflist[2].index) |
+ VDPU_REG_BINIT_RLIST_B1(b1_reflist[1].index) |
+ VDPU_REG_BINIT_RLIST_B0(b1_reflist[0].index);
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(103));
- reg = VDPU_REG_BINIT_RLIST_B11(b1_reflist[11]) |
- VDPU_REG_BINIT_RLIST_B10(b1_reflist[10]) |
- VDPU_REG_BINIT_RLIST_B9(b1_reflist[9]) |
- VDPU_REG_BINIT_RLIST_B8(b1_reflist[8]) |
- VDPU_REG_BINIT_RLIST_B7(b1_reflist[7]) |
- VDPU_REG_BINIT_RLIST_B6(b1_reflist[6]);
+ reg = VDPU_REG_BINIT_RLIST_B11(b1_reflist[11].index) |
+ VDPU_REG_BINIT_RLIST_B10(b1_reflist[10].index) |
+ VDPU_REG_BINIT_RLIST_B9(b1_reflist[9].index) |
+ VDPU_REG_BINIT_RLIST_B8(b1_reflist[8].index) |
+ VDPU_REG_BINIT_RLIST_B7(b1_reflist[7].index) |
+ VDPU_REG_BINIT_RLIST_B6(b1_reflist[6].index);
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(104));
- reg = VDPU_REG_BINIT_RLIST_B15(b1_reflist[15]) |
- VDPU_REG_BINIT_RLIST_B14(b1_reflist[14]) |
- VDPU_REG_BINIT_RLIST_B13(b1_reflist[13]) |
- VDPU_REG_BINIT_RLIST_B12(b1_reflist[12]);
+ reg = VDPU_REG_BINIT_RLIST_B15(b1_reflist[15].index) |
+ VDPU_REG_BINIT_RLIST_B14(b1_reflist[14].index) |
+ VDPU_REG_BINIT_RLIST_B13(b1_reflist[13].index) |
+ VDPU_REG_BINIT_RLIST_B12(b1_reflist[12].index);
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(105));
- reg = VDPU_REG_PINIT_RLIST_F3(p_reflist[3]) |
- VDPU_REG_PINIT_RLIST_F2(p_reflist[2]) |
- VDPU_REG_PINIT_RLIST_F1(p_reflist[1]) |
- VDPU_REG_PINIT_RLIST_F0(p_reflist[0]);
+ reg = VDPU_REG_PINIT_RLIST_F3(p_reflist[3].index) |
+ VDPU_REG_PINIT_RLIST_F2(p_reflist[2].index) |
+ VDPU_REG_PINIT_RLIST_F1(p_reflist[1].index) |
+ VDPU_REG_PINIT_RLIST_F0(p_reflist[0].index);
vdpu_write_relaxed(vpu, reg, VDPU_SWREG(106));
reg = VDPU_REG_REFER_LTERM_E(ctx->h264_dec.dpb_longterm);
diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
index 951e19231da2..3c7f3d87fab4 100644
--- a/drivers/staging/media/rkvdec/rkvdec-h264.c
+++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
@@ -100,9 +100,9 @@ struct rkvdec_h264_priv_tbl {
#define RKVDEC_H264_DPB_SIZE 16
struct rkvdec_h264_reflists {
- u8 p[RKVDEC_H264_DPB_SIZE];
- u8 b0[RKVDEC_H264_DPB_SIZE];
- u8 b1[RKVDEC_H264_DPB_SIZE];
+ struct v4l2_h264_reference p[RKVDEC_H264_DPB_SIZE];
+ struct v4l2_h264_reference b0[RKVDEC_H264_DPB_SIZE];
+ struct v4l2_h264_reference b1[RKVDEC_H264_DPB_SIZE];
u8 num_valid;
};
@@ -767,13 +767,13 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
switch (j) {
case 0:
- idx = h264_ctx->reflists.p[i];
+ idx = h264_ctx->reflists.p[i].index;
break;
case 1:
- idx = h264_ctx->reflists.b0[i];
+ idx = h264_ctx->reflists.b0[i].index;
break;
case 2:
- idx = h264_ctx->reflists.b1[i];
+ idx = h264_ctx->reflists.b1[i].index;
break;
}
diff --git a/include/media/v4l2-h264.h b/include/media/v4l2-h264.h
index 4b1c71c935e0..ef9a894e3c32 100644
--- a/include/media/v4l2-h264.h
+++ b/include/media/v4l2-h264.h
@@ -37,7 +37,7 @@ struct v4l2_h264_reflist_builder {
u16 longterm : 1;
} refs[V4L2_H264_NUM_DPB_ENTRIES];
s32 cur_pic_order_count;
- u8 unordered_reflist[V4L2_H264_NUM_DPB_ENTRIES];
+ struct v4l2_h264_reference unordered_reflist[V4L2_H264_NUM_DPB_ENTRIES];
u8 num_valid;
};
@@ -51,10 +51,10 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
* v4l2_h264_build_b_ref_lists() - Build the B0/B1 reference lists
*
* @builder: reference list builder context
- * @b0_reflist: 16-bytes array used to store the B0 reference list. Each entry
- * is an index in the DPB
- * @b1_reflist: 16-bytes array used to store the B1 reference list. Each entry
- * is an index in the DPB
+ * @b0_reflist: 16 sized array used to store the B0 reference list. Each entry
+ * is a v4l2_h264_reference structure
+ * @b1_reflist: 16 sized array used to store the B1 reference list. Each entry
+ * is a v4l2_h264_reference structure
*
* This functions builds the B0/B1 reference lists. This procedure is described
* in section '8.2.4 Decoding process for reference picture lists construction'
@@ -63,14 +63,15 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
*/
void
v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
- u8 *b0_reflist, u8 *b1_reflist);
+ struct v4l2_h264_reference *b0_reflist,
+ struct v4l2_h264_reference *b1_reflist);
/**
* v4l2_h264_build_p_ref_list() - Build the P reference list
*
* @builder: reference list builder context
- * @reflist: 16-bytes array used to store the P reference list. Each entry
- * is an index in the DPB
+ * @reflist: 16 sized array used to store the P reference list. Each entry
+ * is a v4l2_h264_reference structure
*
* This functions builds the P reference lists. This procedure is describe in
* section '8.2.4 Decoding process for reference picture lists construction'
@@ -79,6 +80,6 @@ v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
*/
void
v4l2_h264_build_p_ref_list(const struct v4l2_h264_reflist_builder *builder,
- u8 *reflist);
+ struct v4l2_h264_reference *reflist);
#endif /* _MEDIA_V4L2_H264_H */
--
2.34.1
This is to accommodate support for field decoding, which splits the top
and the bottom references into the reference list.
Signed-off-by: Nicolas Dufresne <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
drivers/media/v4l2-core/v4l2-h264.c | 6 +++---
drivers/staging/media/hantro/hantro_hw.h | 6 +++---
drivers/staging/media/rkvdec/rkvdec-h264.c | 6 +++---
include/media/v4l2-h264.h | 8 ++++----
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
index afbfcf78efe4..4b46b36526c0 100644
--- a/drivers/media/v4l2-core/v4l2-h264.c
+++ b/drivers/media/v4l2-core/v4l2-h264.c
@@ -212,7 +212,7 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
* v4l2_h264_build_p_ref_list() - Build the P reference list
*
* @builder: reference list builder context
- * @reflist: 16 sized array used to store the P reference list. Each entry
+ * @reflist: 32 sized array used to store the P reference list. Each entry
* is a v4l2_h264_reference structure
*
* This functions builds the P reference lists. This procedure is describe in
@@ -235,9 +235,9 @@ EXPORT_SYMBOL_GPL(v4l2_h264_build_p_ref_list);
* v4l2_h264_build_b_ref_lists() - Build the B0/B1 reference lists
*
* @builder: reference list builder context
- * @b0_reflist: 16 sized array used to store the B0 reference list. Each entry
+ * @b0_reflist: 32 sized array used to store the B0 reference list. Each entry
* is a v4l2_h264_reference structure
- * @b1_reflist: 16 sized array used to store the B1 reference list. Each entry
+ * @b1_reflist: 32 sized array used to store the B1 reference list. Each entry
* is a v4l2_h264_reference structure
*
* This functions builds the B0/B1 reference lists. This procedure is described
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 2bc6b8f088f5..292aaaabaf24 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -69,9 +69,9 @@ struct hantro_h264_dec_ctrls {
* @b1: B1 reflist
*/
struct hantro_h264_dec_reflists {
- struct v4l2_h264_reference p[HANTRO_H264_DPB_SIZE];
- struct v4l2_h264_reference b0[HANTRO_H264_DPB_SIZE];
- struct v4l2_h264_reference b1[HANTRO_H264_DPB_SIZE];
+ struct v4l2_h264_reference p[V4L2_H264_REF_LIST_LEN];
+ struct v4l2_h264_reference b0[V4L2_H264_REF_LIST_LEN];
+ struct v4l2_h264_reference b1[V4L2_H264_REF_LIST_LEN];
};
/**
diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
index 3c7f3d87fab4..dff89732ddd0 100644
--- a/drivers/staging/media/rkvdec/rkvdec-h264.c
+++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
@@ -100,9 +100,9 @@ struct rkvdec_h264_priv_tbl {
#define RKVDEC_H264_DPB_SIZE 16
struct rkvdec_h264_reflists {
- struct v4l2_h264_reference p[RKVDEC_H264_DPB_SIZE];
- struct v4l2_h264_reference b0[RKVDEC_H264_DPB_SIZE];
- struct v4l2_h264_reference b1[RKVDEC_H264_DPB_SIZE];
+ struct v4l2_h264_reference p[V4L2_H264_REF_LIST_LEN];
+ struct v4l2_h264_reference b0[V4L2_H264_REF_LIST_LEN];
+ struct v4l2_h264_reference b1[V4L2_H264_REF_LIST_LEN];
u8 num_valid;
};
diff --git a/include/media/v4l2-h264.h b/include/media/v4l2-h264.h
index ef9a894e3c32..e282fb16ac58 100644
--- a/include/media/v4l2-h264.h
+++ b/include/media/v4l2-h264.h
@@ -37,7 +37,7 @@ struct v4l2_h264_reflist_builder {
u16 longterm : 1;
} refs[V4L2_H264_NUM_DPB_ENTRIES];
s32 cur_pic_order_count;
- struct v4l2_h264_reference unordered_reflist[V4L2_H264_NUM_DPB_ENTRIES];
+ struct v4l2_h264_reference unordered_reflist[V4L2_H264_REF_LIST_LEN];
u8 num_valid;
};
@@ -51,9 +51,9 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
* v4l2_h264_build_b_ref_lists() - Build the B0/B1 reference lists
*
* @builder: reference list builder context
- * @b0_reflist: 16 sized array used to store the B0 reference list. Each entry
+ * @b0_reflist: 32 sized array used to store the B0 reference list. Each entry
* is a v4l2_h264_reference structure
- * @b1_reflist: 16 sized array used to store the B1 reference list. Each entry
+ * @b1_reflist: 32 sized array used to store the B1 reference list. Each entry
* is a v4l2_h264_reference structure
*
* This functions builds the B0/B1 reference lists. This procedure is described
@@ -70,7 +70,7 @@ v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
* v4l2_h264_build_p_ref_list() - Build the P reference list
*
* @builder: reference list builder context
- * @reflist: 16 sized array used to store the P reference list. Each entry
+ * @reflist: 32 sized array used to store the P reference list. Each entry
* is a v4l2_h264_reference structure
*
* This functions builds the P reference lists. This procedure is describe in
--
2.34.1
For long term references, frame_num is set to long_term_frame_idx which
does not require wrapping. This is fixed by observation, no directly
related issue have been found yet.
Signed-off-by: Nicolas Dufresne <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
drivers/media/v4l2-core/v4l2-h264.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
index 5633a242520a..ac47519a9fbe 100644
--- a/drivers/media/v4l2-core/v4l2-h264.c
+++ b/drivers/media/v4l2-core/v4l2-h264.c
@@ -57,8 +57,10 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
* '8.2.4.1 Decoding process for picture numbers' of the spec.
* TODO: This logic will have to be adjusted when we start
* supporting interlaced content.
+ * For long term references, frame_num is set to
+ * long_term_frame_idx which requires no wrapping.
*/
- if (dpb[i].frame_num > cur_frame_num)
+ if (!b->refs[i].longterm && dpb[i].frame_num > cur_frame_num)
b->refs[i].frame_num = (int)dpb[i].frame_num -
max_frame_num;
else
--
2.34.1
This is needed to optimize field decoding. Each field will be
decoded into the same capture buffer. To be able to queue multiple
buffers, we need to be able to ask the driver to hold the capture
buffer.
Signed-off-by: Nicolas Dufresne <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
drivers/staging/media/hantro/hantro_v4l2.c | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 67148ba346f5..50d636678ff3 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -409,6 +409,30 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
}
}
+static void
+hantro_update_requires_hold_capture_buf(struct hantro_ctx *ctx, u32 fourcc)
+{
+ struct vb2_queue *vq;
+
+ vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+ V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
+
+ switch (fourcc) {
+ case V4L2_PIX_FMT_JPEG:
+ case V4L2_PIX_FMT_MPEG2_SLICE:
+ case V4L2_PIX_FMT_VP8_FRAME:
+ case V4L2_PIX_FMT_HEVC_SLICE:
+ case V4L2_PIX_FMT_VP9_FRAME:
+ vq->subsystem_flags &= ~(VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF);
+ break;
+ case V4L2_PIX_FMT_H264_SLICE:
+ vq->subsystem_flags |= VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF;
+ break;
+ default:
+ break;
+ }
+}
+
static int hantro_set_fmt_out(struct hantro_ctx *ctx,
struct v4l2_pix_format_mplane *pix_mp)
{
@@ -472,6 +496,7 @@ static int hantro_set_fmt_out(struct hantro_ctx *ctx,
ctx->dst_fmt.quantization = pix_mp->quantization;
hantro_update_requires_request(ctx, pix_mp->pixelformat);
+ hantro_update_requires_hold_capture_buf(ctx, pix_mp->pixelformat);
vpu_debug(0, "OUTPUT codec mode: %d\n", ctx->vpu_src_fmt->codec_mode);
vpu_debug(0, "fmt - w: %d, h: %d\n",
--
2.34.1
From: Jonas Karlman <[email protected]>
The luma and chroma bit depth fields in the pps packet are 3 bits wide.
8 is wrongly added to the bit depth values written to these 3 bit fields.
Because only the 3 LSB are written, the hardware was configured
correctly.
Correct this by not adding 8 to the luma and chroma bit depth value.
Signed-off-by: Jonas Karlman <[email protected]>
Signed-off-by: Nicolas Dufresne <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>
---
drivers/staging/media/rkvdec/rkvdec-h264.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
index bcde37d72244..8d44a884a52e 100644
--- a/drivers/staging/media/rkvdec/rkvdec-h264.c
+++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
@@ -662,8 +662,8 @@ static void assemble_hw_pps(struct rkvdec_ctx *ctx,
WRITE_PPS(0xff, PROFILE_IDC);
WRITE_PPS(1, CONSTRAINT_SET3_FLAG);
WRITE_PPS(sps->chroma_format_idc, CHROMA_FORMAT_IDC);
- WRITE_PPS(sps->bit_depth_luma_minus8 + 8, BIT_DEPTH_LUMA);
- WRITE_PPS(sps->bit_depth_chroma_minus8 + 8, BIT_DEPTH_CHROMA);
+ WRITE_PPS(sps->bit_depth_luma_minus8, BIT_DEPTH_LUMA);
+ WRITE_PPS(sps->bit_depth_chroma_minus8, BIT_DEPTH_CHROMA);
WRITE_PPS(0, QPPRIME_Y_ZERO_TRANSFORM_BYPASS_FLAG);
WRITE_PPS(sps->log2_max_frame_num_minus4, LOG2_MAX_FRAME_NUM_MINUS4);
WRITE_PPS(sps->max_num_ref_frames, MAX_NUM_REF_FRAMES);
--
2.34.1
These two fields need documentation as they have dual meaning. It is also
confusing since pic_num is a derived value from frame_num, so this should
help application developers. If we ever need to make a V2 of this API, I
would suggest to remove pic_num entirely.
Signed-off-by: Nicolas Dufresne <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
.../media/v4l/ext-ctrls-codec-stateless.rst | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst
index 6541e4c32b26..49f89b702068 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst
@@ -649,10 +649,16 @@ Stateless Codec Control ID
:c:type:`timeval` in struct :c:type:`v4l2_buffer` to a __u64.
* - __u32
- ``pic_num``
- -
+ - For short term references, this should match the derived value PicNum
+ (8-28) and for long term references it should match the derived value
+ LongTermPicNum (8-29). Note that pic_num is the same as FrameNumWrap
+ for frame decoding.
* - __u16
- ``frame_num``
- -
+ - For short term references, this should match the frame_num value from
+ the slice header syntax (the driver will wrap the value if neeeded). For
+ long term references, this should be set to the value of
+ long_term_frame_idx described in the dec_ref_pic_marking() syntax.
* - __u8
- ``fields``
- Specifies how the DPB entry is referenced. See :ref:`Reference Fields <h264_ref_fields>`
--
2.34.1
On Thu, Mar 31, 2022 at 03:37:16PM -0400, Nicolas Dufresne wrote:
> From: Jonas Karlman <[email protected]>
>
> The luma and chroma bit depth fields in the pps packet are 3 bits wide.
> 8 is wrongly added to the bit depth values written to these 3 bit fields.
> Because only the 3 LSB are written, the hardware was configured
> correctly.
>
> Correct this by not adding 8 to the luma and chroma bit depth value.
>
> Signed-off-by: Jonas Karlman <[email protected]>
> Signed-off-by: Nicolas Dufresne <[email protected]>
> Reviewed-by: Ezequiel Garcia <[email protected]>
Fixes: cd33c830448ba ("media: rkvdec: Add the rkvdec driver")
Thanks!
> ---
> drivers/staging/media/rkvdec/rkvdec-h264.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> index bcde37d72244..8d44a884a52e 100644
> --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> @@ -662,8 +662,8 @@ static void assemble_hw_pps(struct rkvdec_ctx *ctx,
> WRITE_PPS(0xff, PROFILE_IDC);
> WRITE_PPS(1, CONSTRAINT_SET3_FLAG);
> WRITE_PPS(sps->chroma_format_idc, CHROMA_FORMAT_IDC);
> - WRITE_PPS(sps->bit_depth_luma_minus8 + 8, BIT_DEPTH_LUMA);
> - WRITE_PPS(sps->bit_depth_chroma_minus8 + 8, BIT_DEPTH_CHROMA);
> + WRITE_PPS(sps->bit_depth_luma_minus8, BIT_DEPTH_LUMA);
> + WRITE_PPS(sps->bit_depth_chroma_minus8, BIT_DEPTH_CHROMA);
> WRITE_PPS(0, QPPRIME_Y_ZERO_TRANSFORM_BYPASS_FLAG);
> WRITE_PPS(sps->log2_max_frame_num_minus4, LOG2_MAX_FRAME_NUM_MINUS4);
> WRITE_PPS(sps->max_num_ref_frames, MAX_NUM_REF_FRAMES);
> --
> 2.34.1
>
Hi Nicolas,
On Thu, Mar 31, 2022 at 03:37:18PM -0400, Nicolas Dufresne wrote:
> From: Jonas Karlman <[email protected]>
>
> When decoding the second field in a complementary field pair the second
> field is sharing the same frame_num with the first field.
>
> Currently the frame_num for the first field is wrapped when it matches the
> field being decoded, this cause issues to decode the second field in a
> complementary field pair.
>
> Fix this by using inclusive comparison, less than or equal.
>
> Signed-off-by: Jonas Karlman <[email protected]>
> Reviewed-by: Ezequiel Garcia <[email protected]>
> Reviewed-by: Sebastian Fricke <[email protected]>
To complete the Signed-off-by chain, can you add your Signed-off-by: at
the bottom?
Thanks,
Ezequiel
> ---
> drivers/staging/media/rkvdec/rkvdec-h264.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> index a42cf19bcc6d..730f8ebf7f58 100644
> --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> @@ -773,7 +773,7 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
> continue;
>
> if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM ||
> - dpb[i].frame_num < dec_params->frame_num) {
> + dpb[i].frame_num <= dec_params->frame_num) {
> p[i] = dpb[i].frame_num;
> continue;
> }
> --
> 2.34.1
>
Add debug print statements to print the content of P & B reference
lists, to verify that the ordering of the generated reference lists is
correct. This is especially important for the field decoding mode,
where sorting is more complex.
Signed-off-by: Nicolas Dufresne <[email protected]>
Tested-by: Sebastian Fricke <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
drivers/media/v4l2-core/v4l2-h264.c | 86 +++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
index 38d8dbda0045..bcf9b7774560 100644
--- a/drivers/media/v4l2-core/v4l2-h264.c
+++ b/drivers/media/v4l2-core/v4l2-h264.c
@@ -241,6 +241,87 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
return poca < pocb ? -1 : 1;
}
+static char ref_type_to_char (u8 ref_type)
+{
+ switch (ref_type) {
+ case V4L2_H264_FRAME_REF:
+ return 'f';
+ case V4L2_H264_TOP_FIELD_REF:
+ return 't';
+ case V4L2_H264_BOTTOM_FIELD_REF:
+ return 'b';
+ }
+
+ return '?';
+}
+
+static const char *format_ref_list_p(const struct v4l2_h264_reflist_builder *builder,
+ struct v4l2_h264_reference *reflist,
+ char *out_str, const int len)
+{
+ int n = 0, i;
+
+ n += snprintf(out_str + n, len - n, "|");
+
+ for (i = 0; i < builder->num_valid; i++) {
+ /* this is pic_num for frame and frame_num (wrapped) for field,
+ * but for frame pic_num is equal to frame_num (wrapped).
+ */
+ int frame_num = builder->refs[reflist[i].index].frame_num;
+ bool longterm = builder->refs[reflist[i].index].longterm;
+
+ n += scnprintf(out_str + n, len - n, "%i%c%c|",
+ frame_num, longterm ? 'l' : 's',
+ ref_type_to_char (reflist[i].fields));
+ }
+
+ return out_str;
+}
+
+static void print_ref_list_p(const struct v4l2_h264_reflist_builder *builder,
+ struct v4l2_h264_reference *reflist)
+{
+ char buf[1024];
+
+ pr_debug("ref_pic_list_p (cur_poc %u%c) %s\n",
+ builder->cur_pic_order_count,
+ ref_type_to_char(builder->cur_pic_fields),
+ format_ref_list_p(builder, reflist, buf, sizeof(buf)));
+}
+
+static const char *format_ref_list_b(const struct v4l2_h264_reflist_builder *builder,
+ struct v4l2_h264_reference *reflist,
+ char *out_str, const int len)
+{
+ int n = 0, i;
+
+ n += snprintf(out_str + n, len - n, "|");
+
+ for (i = 0; i < builder->num_valid; i++) {
+ int frame_num = builder->refs[reflist[i].index].frame_num;
+ u32 poc = v4l2_h264_get_poc(builder, reflist + i);
+ bool longterm = builder->refs[reflist[i].index].longterm;
+
+ n += scnprintf(out_str + n, len - n, "%i%c%c|",
+ longterm ? frame_num : poc,
+ longterm ? 'l' : 's',
+ ref_type_to_char(reflist[i].fields));
+ }
+
+ return out_str;
+}
+
+static void print_ref_list_b(const struct v4l2_h264_reflist_builder *builder,
+ struct v4l2_h264_reference *reflist, u8 list_num)
+{
+ char buf[1024];
+
+ pr_debug("ref_pic_list_b%u (cur_poc %u%c) %s",
+ list_num, builder->cur_pic_order_count,
+ ref_type_to_char (builder->cur_pic_fields),
+ format_ref_list_b(builder, reflist, buf, sizeof(buf)));
+}
+
/**
* v4l2_h264_build_p_ref_list() - Build the P reference list
*
@@ -261,6 +342,8 @@ v4l2_h264_build_p_ref_list(const struct v4l2_h264_reflist_builder *builder,
sizeof(builder->unordered_reflist[0]) * builder->num_valid);
sort_r(reflist, builder->num_valid, sizeof(*reflist),
v4l2_h264_p_ref_list_cmp, NULL, builder);
+
+ print_ref_list_p(builder, reflist);
}
EXPORT_SYMBOL_GPL(v4l2_h264_build_p_ref_list);
@@ -296,6 +379,9 @@ v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
if (builder->num_valid > 1 &&
!memcmp(b1_reflist, b0_reflist, builder->num_valid))
swap(b1_reflist[0], b1_reflist[1]);
+
+ print_ref_list_b(builder, b0_reflist, 0);
+ print_ref_list_b(builder, b1_reflist, 1);
}
EXPORT_SYMBOL_GPL(v4l2_h264_build_b_ref_lists);
--
2.34.1
On Thu, Mar 31, 2022 at 03:37:06PM -0400, Nicolas Dufresne wrote:
> For long term references, frame_num is set to long_term_frame_idx which
> does not require wrapping. This is fixed by observation, no directly
> related issue have been found yet.
>
> Signed-off-by: Nicolas Dufresne <[email protected]>
> Reviewed-by: Sebastian Fricke <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>
> ---
> drivers/media/v4l2-core/v4l2-h264.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
> index 5633a242520a..ac47519a9fbe 100644
> --- a/drivers/media/v4l2-core/v4l2-h264.c
> +++ b/drivers/media/v4l2-core/v4l2-h264.c
> @@ -57,8 +57,10 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
> * '8.2.4.1 Decoding process for picture numbers' of the spec.
> * TODO: This logic will have to be adjusted when we start
> * supporting interlaced content.
> + * For long term references, frame_num is set to
> + * long_term_frame_idx which requires no wrapping.
> */
> - if (dpb[i].frame_num > cur_frame_num)
> + if (!b->refs[i].longterm && dpb[i].frame_num > cur_frame_num)
> b->refs[i].frame_num = (int)dpb[i].frame_num -
> max_frame_num;
> else
> --
> 2.34.1
>
The hardware expects FrameNumWrap or long_term_frame_idx. Picture
numbers are per field, and are mostly used during the memory
management process, which is done in userland. This fixes two
ITU conformance tests:
- MR6_BT_B
- MR8_BT_B
Signed-off-by: Nicolas Dufresne <[email protected]>
Reviewed-by: Sebastian Fricke <[email protected]>
---
drivers/staging/media/hantro/hantro_h264.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_h264.c b/drivers/staging/media/hantro/hantro_h264.c
index 0b4d2491be3b..228629fb3cdf 100644
--- a/drivers/staging/media/hantro/hantro_h264.c
+++ b/drivers/staging/media/hantro/hantro_h264.c
@@ -354,8 +354,6 @@ u16 hantro_h264_get_ref_nbr(struct hantro_ctx *ctx, unsigned int dpb_idx)
if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
return 0;
- if (dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM)
- return dpb->pic_num;
return dpb->frame_num;
}
--
2.34.1
On Thu, Mar 31, 2022 at 03:37:08PM -0400, Nicolas Dufresne wrote:
> This is to accommodate support for field decoding, which splits the top
> and the bottom references into the reference list.
>
> Signed-off-by: Nicolas Dufresne <[email protected]>
> Reviewed-by: Sebastian Fricke <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>
> ---
> drivers/media/v4l2-core/v4l2-h264.c | 6 +++---
> drivers/staging/media/hantro/hantro_hw.h | 6 +++---
> drivers/staging/media/rkvdec/rkvdec-h264.c | 6 +++---
> include/media/v4l2-h264.h | 8 ++++----
> 4 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c
> index afbfcf78efe4..4b46b36526c0 100644
> --- a/drivers/media/v4l2-core/v4l2-h264.c
> +++ b/drivers/media/v4l2-core/v4l2-h264.c
> @@ -212,7 +212,7 @@ static int v4l2_h264_b1_ref_list_cmp(const void *ptra, const void *ptrb,
> * v4l2_h264_build_p_ref_list() - Build the P reference list
> *
> * @builder: reference list builder context
> - * @reflist: 16 sized array used to store the P reference list. Each entry
> + * @reflist: 32 sized array used to store the P reference list. Each entry
> * is a v4l2_h264_reference structure
> *
> * This functions builds the P reference lists. This procedure is describe in
> @@ -235,9 +235,9 @@ EXPORT_SYMBOL_GPL(v4l2_h264_build_p_ref_list);
> * v4l2_h264_build_b_ref_lists() - Build the B0/B1 reference lists
> *
> * @builder: reference list builder context
> - * @b0_reflist: 16 sized array used to store the B0 reference list. Each entry
> + * @b0_reflist: 32 sized array used to store the B0 reference list. Each entry
> * is a v4l2_h264_reference structure
> - * @b1_reflist: 16 sized array used to store the B1 reference list. Each entry
> + * @b1_reflist: 32 sized array used to store the B1 reference list. Each entry
> * is a v4l2_h264_reference structure
> *
> * This functions builds the B0/B1 reference lists. This procedure is described
> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> index 2bc6b8f088f5..292aaaabaf24 100644
> --- a/drivers/staging/media/hantro/hantro_hw.h
> +++ b/drivers/staging/media/hantro/hantro_hw.h
> @@ -69,9 +69,9 @@ struct hantro_h264_dec_ctrls {
> * @b1: B1 reflist
> */
> struct hantro_h264_dec_reflists {
> - struct v4l2_h264_reference p[HANTRO_H264_DPB_SIZE];
> - struct v4l2_h264_reference b0[HANTRO_H264_DPB_SIZE];
> - struct v4l2_h264_reference b1[HANTRO_H264_DPB_SIZE];
> + struct v4l2_h264_reference p[V4L2_H264_REF_LIST_LEN];
> + struct v4l2_h264_reference b0[V4L2_H264_REF_LIST_LEN];
> + struct v4l2_h264_reference b1[V4L2_H264_REF_LIST_LEN];
> };
>
> /**
> diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> index 3c7f3d87fab4..dff89732ddd0 100644
> --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> @@ -100,9 +100,9 @@ struct rkvdec_h264_priv_tbl {
> #define RKVDEC_H264_DPB_SIZE 16
>
> struct rkvdec_h264_reflists {
> - struct v4l2_h264_reference p[RKVDEC_H264_DPB_SIZE];
> - struct v4l2_h264_reference b0[RKVDEC_H264_DPB_SIZE];
> - struct v4l2_h264_reference b1[RKVDEC_H264_DPB_SIZE];
> + struct v4l2_h264_reference p[V4L2_H264_REF_LIST_LEN];
> + struct v4l2_h264_reference b0[V4L2_H264_REF_LIST_LEN];
> + struct v4l2_h264_reference b1[V4L2_H264_REF_LIST_LEN];
> u8 num_valid;
> };
>
> diff --git a/include/media/v4l2-h264.h b/include/media/v4l2-h264.h
> index ef9a894e3c32..e282fb16ac58 100644
> --- a/include/media/v4l2-h264.h
> +++ b/include/media/v4l2-h264.h
> @@ -37,7 +37,7 @@ struct v4l2_h264_reflist_builder {
> u16 longterm : 1;
> } refs[V4L2_H264_NUM_DPB_ENTRIES];
> s32 cur_pic_order_count;
> - struct v4l2_h264_reference unordered_reflist[V4L2_H264_NUM_DPB_ENTRIES];
> + struct v4l2_h264_reference unordered_reflist[V4L2_H264_REF_LIST_LEN];
> u8 num_valid;
> };
>
> @@ -51,9 +51,9 @@ v4l2_h264_init_reflist_builder(struct v4l2_h264_reflist_builder *b,
> * v4l2_h264_build_b_ref_lists() - Build the B0/B1 reference lists
> *
> * @builder: reference list builder context
> - * @b0_reflist: 16 sized array used to store the B0 reference list. Each entry
> + * @b0_reflist: 32 sized array used to store the B0 reference list. Each entry
> * is a v4l2_h264_reference structure
> - * @b1_reflist: 16 sized array used to store the B1 reference list. Each entry
> + * @b1_reflist: 32 sized array used to store the B1 reference list. Each entry
> * is a v4l2_h264_reference structure
> *
> * This functions builds the B0/B1 reference lists. This procedure is described
> @@ -70,7 +70,7 @@ v4l2_h264_build_b_ref_lists(const struct v4l2_h264_reflist_builder *builder,
> * v4l2_h264_build_p_ref_list() - Build the P reference list
> *
> * @builder: reference list builder context
> - * @reflist: 16 sized array used to store the P reference list. Each entry
> + * @reflist: 32 sized array used to store the P reference list. Each entry
> * is a v4l2_h264_reference structure
> *
> * This functions builds the P reference lists. This procedure is describe in
> --
> 2.34.1
>
Hi Nicolas,
On Thu, Mar 31, 2022 at 03:37:17PM -0400, Nicolas Dufresne wrote:
> From: Jonas Karlman <[email protected]>
>
> The width and height in macroblocks 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 <[email protected]>
> Signed-off-by: Nicolas Dufresne <[email protected]>
> Reviewed-by: Sebastian Fricke <[email protected]>
> ---
> 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 8d44a884a52e..a42cf19bcc6d 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);
Please add a comment so we don't forget why we use the bitstream
fields here.
> + 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 2df8cf4883e2..1b805710e195 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;
> +
Let's please add a comment here, clarifying it's legal to check
the coded format (OUTPUT queue format) at .try_ctrl time,
because the stateless decoder specification [1] mandates
S_FMT on the OUTPUT queue, before passing the SPS/PPS controls.
[1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/dev-stateless-decoder.html
> + if (width > ctx->coded_fmt.fmt.pix_mp.width ||
> + height > ctx->coded_fmt.fmt.pix_mp.height)
Can you add a debug message or error message?
These silent errors tend to get super hard to track.
With these changes:
Reviewed-by: Ezequiel Garcia <[email protected]>
Thanks,
Ezequiel
Le mardi 05 avril 2022 à 11:44 -0400, Nicolas Dufresne a écrit :
> Le samedi 02 avril 2022 à 08:32 -0300, Ezequiel Garcia a écrit :
> > Hi Nicolas,
> >
> > On Thu, Mar 31, 2022 at 03:37:17PM -0400, Nicolas Dufresne wrote:
> > > From: Jonas Karlman <[email protected]>
> > >
> > > The width and height in macroblocks 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 <[email protected]>
> > > Signed-off-by: Nicolas Dufresne <[email protected]>
> > > Reviewed-by: Sebastian Fricke <[email protected]>
> > > ---
> > > 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 8d44a884a52e..a42cf19bcc6d 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);
> >
> > Please add a comment so we don't forget why we use the bitstream
> > fields here.
>
> And perhaps I should clarify that only the height will vary. It remains nice if
> we can decode smaller images into larger image/format, that will be needed to
> handle the sub-layers in SVC (these are not to be displayed, so we don't care
> much about the output stride and all). So that is also an improvement.
>
> >
> > > + 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 2df8cf4883e2..1b805710e195 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;
> > > +
> >
> > Let's please add a comment here, clarifying it's legal to check
> > the coded format (OUTPUT queue format) at .try_ctrl time,
> > because the stateless decoder specification [1] mandates
> > S_FMT on the OUTPUT queue, before passing the SPS/PPS controls.
>
> Indeed, though I come to see some flaw in the validation. First, the height
> formula shall be base on formula 7-18 from the spec:
>
> FrameHeightInMbs = ( 2 − frame_mbs_only_flag ) * PicHeightInMapUnits
>
> So would be
>
> + height = (sps->pic_height_in_map_units_minus1 + 1) * 16;
> + if (sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY)
> + height *= 2;
Oops, I meant:
+ if (!(sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY))
+ height *= 2;
>
> As this driver do interleaved interlaced buffer, not alternate. Finally, we
> should validate this again at STREAMON, since userland may have simply omitted
> to set the SPS at all. I'll try to improve this and drop the review tag to
> ensure this get fully reviewed again.
>
> >
> > [1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/dev-stateless-decoder.html
> >
> > > + if (width > ctx->coded_fmt.fmt.pix_mp.width ||
> > > + height > ctx->coded_fmt.fmt.pix_mp.height)
> >
> > Can you add a debug message or error message?
> > These silent errors tend to get super hard to track.
> >
> > With these changes:
> >
> > Reviewed-by: Ezequiel Garcia <[email protected]>
> >
> > Thanks,
> > Ezequiel
>
Le samedi 02 avril 2022 à 08:32 -0300, Ezequiel Garcia a écrit :
> Hi Nicolas,
>
> On Thu, Mar 31, 2022 at 03:37:17PM -0400, Nicolas Dufresne wrote:
> > From: Jonas Karlman <[email protected]>
> >
> > The width and height in macroblocks 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 <[email protected]>
> > Signed-off-by: Nicolas Dufresne <[email protected]>
> > Reviewed-by: Sebastian Fricke <[email protected]>
> > ---
> > 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 8d44a884a52e..a42cf19bcc6d 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);
>
> Please add a comment so we don't forget why we use the bitstream
> fields here.
And perhaps I should clarify that only the height will vary. It remains nice if
we can decode smaller images into larger image/format, that will be needed to
handle the sub-layers in SVC (these are not to be displayed, so we don't care
much about the output stride and all). So that is also an improvement.
>
> > + 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 2df8cf4883e2..1b805710e195 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;
> > +
>
> Let's please add a comment here, clarifying it's legal to check
> the coded format (OUTPUT queue format) at .try_ctrl time,
> because the stateless decoder specification [1] mandates
> S_FMT on the OUTPUT queue, before passing the SPS/PPS controls.
Indeed, though I come to see some flaw in the validation. First, the height
formula shall be base on formula 7-18 from the spec:
FrameHeightInMbs = ( 2 − frame_mbs_only_flag ) * PicHeightInMapUnits
So would be
+ height = (sps->pic_height_in_map_units_minus1 + 1) * 16;
+ if (sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY)
+ height *= 2;
As this driver do interleaved interlaced buffer, not alternate. Finally, we
should validate this again at STREAMON, since userland may have simply omitted
to set the SPS at all. I'll try to improve this and drop the review tag to
ensure this get fully reviewed again.
>
> [1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/dev-stateless-decoder.html
>
> > + if (width > ctx->coded_fmt.fmt.pix_mp.width ||
> > + height > ctx->coded_fmt.fmt.pix_mp.height)
>
> Can you add a debug message or error message?
> These silent errors tend to get super hard to track.
>
> With these changes:
>
> Reviewed-by: Ezequiel Garcia <[email protected]>
>
> Thanks,
> Ezequiel