| Message ID | 20220405204426.259074-11-nicolas.dufresne@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,01/24] media: doc: Document dual use of H.264 pic_num/frame_num | expand |
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 <nicolas.dufresne@collabora.com> > Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com> > --- > 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]; >
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];