[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