2022-04-06 05:59:26

by Nicolas Dufresne

[permalink] [raw]
Subject: [PATCH v3 10/24] media: h264: Sort p/b reflist using frame_num

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


2022-04-22 19:31:47

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v3 10/24] media: h264: Sort p/b reflist using frame_num

On 05/04/2022 22:44, Nicolas Dufresne wrote:
> 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

funciton -> function

Regards,

Hans

> + * 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];
>