| Message ID | 20240523083750.1332-2-llyyr.public@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2] lavc/vp9: reset segmentation fields when segmentation isn't enabled | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
llyyr.public@gmail.com: > From: llyyr <llyyr.public@gmail.com> > > Fields under the segmentation switch are never reset on a new frame, and > retain the value from the previous frame. This bugs out a bunch of > hwaccel drivers when segmentation is disabled but update_map isn't > reset because they don't ignore values behind switches. This commit also > resets the temporal field, though it may not be required. > > We also do this for vp8 [1] so this commit is just mirroring the vp8 > logic. > > This fixes an issue with certain samples [2] that causes blocky > artifacts with vaapi, d3d11va and cuda (and possibly others). > Mesa worked around [3] this by ignoring these fields if > segmentation.enabled is 0, but d3d11va still displays blocky artifacts. > > [1] https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavcodec/vp8.c#l811 You should probably not link to HEAD here, because the line number will be wrong in the future. > [2] https://github.com/mpv-player/mpv/issues/13533 > [3] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27816 > > Signed-off-by: llyyr <llyyr.public@gmail.com> > --- > libavcodec/vp9.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c > index 6e2d18bf9595..8ede2e2eb358 100644 > --- a/libavcodec/vp9.c > +++ b/libavcodec/vp9.c > @@ -709,6 +709,12 @@ static int decode_frame_header(AVCodecContext *avctx, > s->s.h.segmentation.feat[i].skip_enabled = get_bits1(&s->gb); > } > } > + } else { > + // Reset fields under segmentation switch if segmentation is disabled. > + // This is necessary because some hwaccels don't ignore these fields > + // if segmentation is disabled. > + s->s.h.segmentation.temporal = 0; > + s->s.h.segmentation.update_map = 0; > } > > // set qmul[] based on Y/UV, AC/DC and segmentation Q idx deltas
On 5/23/24 2:13 PM, Andreas Rheinhardt wrote: > llyyr.public@gmail.com: >> From: llyyr <llyyr.public@gmail.com> >> >> Fields under the segmentation switch are never reset on a new frame, and >> retain the value from the previous frame. This bugs out a bunch of >> hwaccel drivers when segmentation is disabled but update_map isn't >> reset because they don't ignore values behind switches. This commit also >> resets the temporal field, though it may not be required. >> >> We also do this for vp8 [1] so this commit is just mirroring the vp8 >> logic. >> >> This fixes an issue with certain samples [2] that causes blocky >> artifacts with vaapi, d3d11va and cuda (and possibly others). >> Mesa worked around [3] this by ignoring these fields if >> segmentation.enabled is 0, but d3d11va still displays blocky artifacts. >> >> [1] https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavcodec/vp8.c#l811 > > You should probably not link to HEAD here, because the line number will > be wrong in the future. > Should I resent the patch with a v3 or could whoever applies the patch fix it when applying it? The correct url is https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/2e877090f958131accb8c7e5ac10e5b9865d1735:/libavcodec/vp8.c#l797
On Thu, May 23, 2024 at 10:38 AM <llyyr.public@gmail.com> wrote: > > From: llyyr <llyyr.public@gmail.com> > > Fields under the segmentation switch are never reset on a new frame, and > retain the value from the previous frame. This bugs out a bunch of > hwaccel drivers when segmentation is disabled but update_map isn't > reset because they don't ignore values behind switches. This commit also > resets the temporal field, though it may not be required. > > We also do this for vp8 [1] so this commit is just mirroring the vp8 > logic. > > This fixes an issue with certain samples [2] that causes blocky > artifacts with vaapi, d3d11va and cuda (and possibly others). > Mesa worked around [3] this by ignoring these fields if > segmentation.enabled is 0, but d3d11va still displays blocky artifacts. > > [1] https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavcodec/vp8.c#l811 > [2] https://github.com/mpv-player/mpv/issues/13533 > [3] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27816 > > Signed-off-by: llyyr <llyyr.public@gmail.com> > --- > libavcodec/vp9.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c > index 6e2d18bf9595..8ede2e2eb358 100644 > --- a/libavcodec/vp9.c > +++ b/libavcodec/vp9.c > @@ -709,6 +709,12 @@ static int decode_frame_header(AVCodecContext *avctx, > s->s.h.segmentation.feat[i].skip_enabled = get_bits1(&s->gb); > } > } > + } else { > + // Reset fields under segmentation switch if segmentation is disabled. > + // This is necessary because some hwaccels don't ignore these fields > + // if segmentation is disabled. > + s->s.h.segmentation.temporal = 0; > + s->s.h.segmentation.update_map = 0; > } > > // set qmul[] based on Y/UV, AC/DC and segmentation Q idx deltas > -- > 2.45.1 Last call for comments. Otherwise will apply on the weekend with the updated Git URL. - Hendrik
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c index 6e2d18bf9595..8ede2e2eb358 100644 --- a/libavcodec/vp9.c +++ b/libavcodec/vp9.c @@ -709,6 +709,12 @@ static int decode_frame_header(AVCodecContext *avctx, s->s.h.segmentation.feat[i].skip_enabled = get_bits1(&s->gb); } } + } else { + // Reset fields under segmentation switch if segmentation is disabled. + // This is necessary because some hwaccels don't ignore these fields + // if segmentation is disabled. + s->s.h.segmentation.temporal = 0; + s->s.h.segmentation.update_map = 0; } // set qmul[] based on Y/UV, AC/DC and segmentation Q idx deltas