Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v2] lavc/vp9: reset segmentation fields when segmentation isn't enabled
@ 2024-02-29  5:58 llyyr
  0 siblings, 0 replies; 6+ messages in thread
From: llyyr @ 2024-02-29  5:58 UTC (permalink / raw)
  To: ffmpeg-devel

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@yukari.in>
---
 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

base-commit: 2e877090f958131accb8c7e5ac10e5b9865d1735
-- 
2.45.1
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] lavc/vp9: reset segmentation fields when segmentation isn't enabled
  2024-05-23  8:37 llyyr.public
  2024-05-23  8:43 ` Andreas Rheinhardt
@ 2024-05-24  9:15 ` Hendrik Leppkes
  1 sibling, 0 replies; 6+ messages in thread
From: Hendrik Leppkes @ 2024-05-24  9:15 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] lavc/vp9: reset segmentation fields when segmentation isn't enabled
  2024-05-23  8:43 ` Andreas Rheinhardt
@ 2024-05-23  8:50   ` llyyr
  0 siblings, 0 replies; 6+ messages in thread
From: llyyr @ 2024-05-23  8:50 UTC (permalink / raw)
  To: ffmpeg-devel

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] lavc/vp9: reset segmentation fields when segmentation isn't enabled
  2024-05-23  8:37 llyyr.public
@ 2024-05-23  8:43 ` Andreas Rheinhardt
  2024-05-23  8:50   ` llyyr
  2024-05-24  9:15 ` Hendrik Leppkes
  1 sibling, 1 reply; 6+ messages in thread
From: Andreas Rheinhardt @ 2024-05-23  8:43 UTC (permalink / raw)
  To: ffmpeg-devel

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [FFmpeg-devel] [PATCH v2] lavc/vp9: reset segmentation fields when segmentation isn't enabled
@ 2024-05-23  8:37 llyyr.public
  2024-05-23  8:43 ` Andreas Rheinhardt
  2024-05-24  9:15 ` Hendrik Leppkes
  0 siblings, 2 replies; 6+ messages in thread
From: llyyr.public @ 2024-05-23  8:37 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: llyyr

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [FFmpeg-devel] [PATCH v2] lavc/vp9: reset segmentation fields when segmentation isn't enabled
@ 2024-02-29  5:58 llyyr
  0 siblings, 0 replies; 6+ messages in thread
From: llyyr @ 2024-02-29  5:58 UTC (permalink / raw)
  To: ffmpeg-devel

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@yukari.in>
---
 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

base-commit: 2e877090f958131accb8c7e5ac10e5b9865d1735
-- 
2.45.1
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-05-24  9:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-29  5:58 [FFmpeg-devel] [PATCH v2] lavc/vp9: reset segmentation fields when segmentation isn't enabled llyyr
2024-02-29  5:58 llyyr
2024-05-23  8:37 llyyr.public
2024-05-23  8:43 ` Andreas Rheinhardt
2024-05-23  8:50   ` llyyr
2024-05-24  9:15 ` Hendrik Leppkes

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git