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

[David Rosca]

On Tue, May 7, 2024 at 10:22 PM Mark Thompson <sw at jkqxz.net> wrote:
>
> On 07/05/2024 07:00, David Rosca wrote:
> > If there are missing references, h264 decode does error concealment
> > by copying previous refs which means there will be duplicated surfaces
> > and this code would try to merge them instead of correctly appending
> > to DPB. Make sure the fields were actually merged before early return.
> > ---
> >  libavcodec/vaapi_h264.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/vaapi_h264.c b/libavcodec/vaapi_h264.c
> > index b47531ce1c..c62a320e97 100644
> > --- a/libavcodec/vaapi_h264.c
> > +++ b/libavcodec/vaapi_h264.c
> > @@ -112,8 +112,9 @@ static int dpb_add(DPB *dpb, const H264Picture *pic)
> >                  } else {
> >                      va_pic->BottomFieldOrderCnt = temp_va_pic.BottomFieldOrderCnt;
> >                  }
> > +                return 0;
> >              }
> > -            return 0;
> > +            break;
> >          }
> >      }
> >
>
> I agree that the old code did nasty things, but can you explain a bit more about the reasoning for what you've got now?
>
> I'm thinking that given duplication a top field could appear twice, so you'd want to merge twice if you have the matching bottom field.

Right, that may be possible.

Also I've looked at vdpau code and there it checks frame_num (in
addition to surface) when searching for the other field, which seems
better to avoid trying to merge when it should not. Posted v2 with
this change + multiple merge.

Thanks,
David

>
> Thanks,
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".