diff mbox series

[v3,10/24] media: h264: Sort p/b reflist using frame_num

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

Commit Message

Nicolas Dufresne April 5, 2022, 8:44 p.m. UTC
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(-)

Comments

Hans Verkuil April 22, 2022, 7:29 a.m. UTC | #1
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 mbox series

Patch

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