| Message ID | 20220328195936.82552-8-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 |
Hey Nicolas, On 28.03.2022 15:59, Nicolas Dufresne wrote: >In the reference list builder, frame_num referers to FrameNumWrap s/referers/refers/ >in the spec, which is the same as the pic_num for frame decoding. >The same applied for long_tern_pic_num and long_ter_frame_idx. s/same applied/same applies/ s/long_ter_frame_idx/long_term_frame_idx/ > >Sort all type of references by frame_num so the sort can be reused s/frame_num so the sort/frame_num, so the order/ >for field reflist were the sorting is done using frame_num instead. s/field/the field/ s/reflist were the sorting is done using frame_num instead/ reflist, which is sorted by the frame_num instead/ >In short, pic_num is never actually used for building reference >lists. > >Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com> >--- > drivers/media/v4l2-core/v4l2-h264.c | 22 ++++++++++++---------- > include/media/v4l2-h264.h | 2 -- > 2 files changed, 12 insertions(+), 12 deletions(-) > >diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c >index c9e18fd51d78..c3fad382882d 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,18 @@ 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 field this is the same direction s/For field this is the same direction/For fields the same direction is used/ >+ * but with frame_num (wrapped). As for frame the pic_num and frame_num >+ * are the same (see formula (8-28) and (8-29)) use the frame_num and share s/As for frame the pic_num and frame_num are the same/ The fields pic_num and frame_num are the same for frames/ >+ * this funciton between frame and field reflist. s/funciton/function/ Greetings, Sebastian > */ > 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 +175,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 +220,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 >
diff --git a/drivers/media/v4l2-core/v4l2-h264.c b/drivers/media/v4l2-core/v4l2-h264.c index c9e18fd51d78..c3fad382882d 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,18 @@ 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 field this is the same direction + * but with frame_num (wrapped). As for frame the pic_num and frame_num + * are the same (see formula (8-28) and (8-29)) use the frame_num and share + * this funciton between frame and field 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 +175,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 +220,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];
In the reference list builder, frame_num referers to FrameNumWrap in the spec, which is the same as the pic_num for frame decoding. The same applied for long_tern_pic_num and long_ter_frame_idx. Sort all type of references by frame_num so the sort can be reused for field 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 <nicolas.dufresne@collabora.com> --- drivers/media/v4l2-core/v4l2-h264.c | 22 ++++++++++++---------- include/media/v4l2-h264.h | 2 -- 2 files changed, 12 insertions(+), 12 deletions(-)