[FFmpeg-devel] [PATCH v2] lavc/vaapi_h264: Fix merging fields in DPB with missing references

[FFmpeg-devel] [PATCH v2] lavc/vaapi_h264: Fix merging fields in DPB with missing references

[David Rosca]

If there are missing references, h264 decode does error concealment
by copying previous refs which means there will be duplicated surfaces.
Check long_ref and frame_idx in addition to surface when looking for
the other field to avoid trying to merge with wrong picture.
Also allow to merge with multiple pictures in case there are duplicates
of the other field.
---
v2: Check long_ref/frame_idx + multiple merge

 libavcodec/vaapi_h264.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/libavcodec/vaapi_h264.c b/libavcodec/vaapi_h264.c
index b47531ce1c..398e92568c 100644
--- a/libavcodec/vaapi_h264.c
+++ b/libavcodec/vaapi_h264.c
@@ -93,14 +93,19 @@ typedef struct DPB {
  */
 static int dpb_add(DPB *dpb, const H264Picture *pic)
 {
-    int i;
+    int i, pic_frame_idx, merged = 0;
 
     if (dpb->size >= dpb->max_size)
         return -1;
 
+    pic_frame_idx = pic->long_ref ? pic->pic_id : pic->frame_num;
+
     for (i = 0; i < dpb->size; i++) {
         VAPictureH264 * const va_pic = &dpb->va_pics[i];
-        if (va_pic->picture_id == ff_vaapi_get_surface_id(pic->f)) {
+        int va_pic_long_ref = !!(va_pic->flags & VA_PICTURE_H264_LONG_TERM_REFERENCE);
+        if (va_pic->picture_id == ff_vaapi_get_surface_id(pic->f) &&
+            va_pic_long_ref == pic->long_ref &&
+            va_pic->frame_idx == pic_frame_idx) {
             VAPictureH264 temp_va_pic;
             fill_vaapi_pic(&temp_va_pic, pic, 0);
 
@@ -112,11 +117,14 @@ static int dpb_add(DPB *dpb, const H264Picture *pic)
                 } else {
                     va_pic->BottomFieldOrderCnt = temp_va_pic.BottomFieldOrderCnt;
                 }
+                merged = 1;
             }
-            return 0;
         }
     }
 
+    if (merged)
+        return 0;
+
     fill_vaapi_pic(&dpb->va_pics[dpb->size++], pic, 0);
     return 0;
 }
-- 
2.45.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] lavc/vaapi_h264: Fix merging fields in DPB with missing references

[Xiang, Haihao]

On Wo, 2024-05-08 at 17:41 +0200, David Rosca wrote:
> If there are missing references, h264 decode does error concealment
> by copying previous refs which means there will be duplicated surfaces.
> Check long_ref and frame_idx in addition to surface when looking for
> the other field to avoid trying to merge with wrong picture.
> Also allow to merge with multiple pictures in case there are duplicates
> of the other field.
> ---
> v2: Check long_ref/frame_idx + multiple merge
> 
>  libavcodec/vaapi_h264.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/vaapi_h264.c b/libavcodec/vaapi_h264.c
> index b47531ce1c..398e92568c 100644
> --- a/libavcodec/vaapi_h264.c
> +++ b/libavcodec/vaapi_h264.c
> @@ -93,14 +93,19 @@ typedef struct DPB {
>   */
>  static int dpb_add(DPB *dpb, const H264Picture *pic)
>  {
> -    int i;
> +    int i, pic_frame_idx, merged = 0;
>  
>      if (dpb->size >= dpb->max_size)
>          return -1;
>  
> +    pic_frame_idx = pic->long_ref ? pic->pic_id : pic->frame_num;
> +
>      for (i = 0; i < dpb->size; i++) {
>          VAPictureH264 * const va_pic = &dpb->va_pics[i];
> -        if (va_pic->picture_id == ff_vaapi_get_surface_id(pic->f)) {
> +        int va_pic_long_ref = !!(va_pic->flags &
> VA_PICTURE_H264_LONG_TERM_REFERENCE);
> +        if (va_pic->picture_id == ff_vaapi_get_surface_id(pic->f) &&
> +            va_pic_long_ref == pic->long_ref &&
> +            va_pic->frame_idx == pic_frame_idx) {
>              VAPictureH264 temp_va_pic;
>              fill_vaapi_pic(&temp_va_pic, pic, 0);
>  
> @@ -112,11 +117,14 @@ static int dpb_add(DPB *dpb, const H264Picture *pic)
>                  } else {
>                      va_pic->BottomFieldOrderCnt =
> temp_va_pic.BottomFieldOrderCnt;
>                  }
> +                merged = 1;
>              }
> -            return 0;
>          }
>      }
>  
> +    if (merged)
> +        return 0;
> +
>      fill_vaapi_pic(&dpb->va_pics[dpb->size++], pic, 0);
>      return 0;
>  }

LGTM, will apply

Thanks
Haihao


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".