Re: [FFmpeg-devel] [PATCH] lavc/vp9: set update_map to 0 when segmentation.enabled is 0

[Hendrik Leppkes <h.leppkes@gmail.com>]

On Wed, May 22, 2024 at 7:16 PM Lynne via ffmpeg-devel
<ffmpeg-devel@ffmpeg.org> wrote:
>
> On 22/05/2024 18:17, Philip Langdale via ffmpeg-devel wrote:
> > On Wed, 22 May 2024 11:10:31 -0400
> > "Ronald S. Bultje" <rsbultje@gmail.com> wrote:
> >
> >> Hi,
> >>
> >> On Wed, May 22, 2024 at 10:36 AM Hendrik Leppkes <h.leppkes@gmail.com>
> >> wrote:
> >>
> >>> On Thu, Feb 29, 2024 at 7:19 AM llyyr <llyyr@yukari.in> wrote:
> >>>>
> >>>> segmentation.update_map is never reset to 0 on a new frame, and
> >>>> retains the value from the previous frame. This bugs out a bunch
> >>>> of hwaccel drivers when segmentation.enabled is 0 but update_map
> >>>> isn't because they don't ignore values behind switches. We also
> >>>> do this for vp8* so this commit is just mirroring the vp8 logic.
> >>>>
> >>>> This fixes an issue with certain samples** that causes blocky
> >>>> artifacts with vaapi and d3d11va (as far as known hwaccel drivers
> >>>> go). Mesa worked around*** this by ignoring this field if
> >>>> segmentation.enabled is 0, but d3d11va still doesn't work.
> >>>>
> >>>> *
> >>> https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavcodec/vp8.c#l811
> >>>
> >>>> ** https://github.com/mpv-player/mpv/issues/13533
> >>>> ***
> >>>> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27816
> >>>>
> >>>> Signed-off-by: llyyr <llyyr@yukari.in>
> >>>> ---
> >>>>   libavcodec/vp9.c | 2 ++
> >>>>   1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> >>>> index 855936cdc1c7..4a628625131e 100644
> >>>> --- a/libavcodec/vp9.c
> >>>> +++ b/libavcodec/vp9.c
> >>>> @@ -717,6 +717,8 @@ static int decode_frame_header(AVCodecContext
> >>>> *avctx, s->s.h.segmentation.feat[i].skip_enabled =
> >>> get_bits1(&s->gb);
> >>>>               }
> >>>>           }
> >>>> +    } else {
> >>>> +        s->s.h.segmentation.update_map = 0;
> >>>>       }
> >>>>
> >>>>       // set qmul[] based on Y/UV, AC/DC and segmentation Q idx
> >>>> deltas
> >>>>
> >>>> base-commit: d263fce2b209e86a5a1e8f1b6aa33430ecc2c187
> >>>> --
> >>>
> >>> Change LGTM.
> >>> I was debugging the same issue today, and found the same problem
> >>> with some hwaccels not properly ignoring update_map when
> >>> segmentation is disabled.
> >>>
> >>> Will apply soon if there are no further comments.
> >>>
> >>
> >> Is fine, please apply.
> >>
> >
> > Another LGTM. We've been seeing this reported on the mpv side as well.
> >
> > --phil
> > _______________________________________________
> > 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".
>
> Can't this get fixed by hwaccel code rather than globally?
> I'd hate to apply fixes with no information in shared code. This can get
> removed with no information about what relies on it.

Changing 5 different hwaccel modules to avoid one line here seems
rather silly, doesn't it?
We can add a comment, if that helps.

- Hendrik
_______________________________________________
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".