[FFmpeg-devel] [PATCH] avfilter/vf_scale: fix interlaced chroma for other formats

[FFmpeg-devel] [PATCH] avfilter/vf_scale: fix interlaced chroma for other formats

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

This logic only covers the case of yuv420p. Extend this logic to cover
*all* vertically subsampled YUV formats, which require the same
interlaced scaling logic.

Fortunately, we can get away with re-using the same code for both JPEG
and MPEG range YUV, because the only difference here is the horizontal
alignment. (To be fixed in a separate commit)
---
 libavfilter/vf_scale.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index b0221e8538..23335cef4b 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -518,6 +518,7 @@ static int config_props(AVFilterLink *outlink)
                             outlink->src->inputs[0];
     enum AVPixelFormat outfmt = outlink->format;
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
+    const AVPixFmtDescriptor *outdesc = av_pix_fmt_desc_get(outfmt);
     ScaleContext *scale = ctx->priv;
     uint8_t *flags_val = NULL;
     int ret;
@@ -588,14 +589,15 @@ static int config_props(AVFilterLink *outlink)
                 av_opt_set_int(s, "dst_range",
                                scale->out_range == AVCOL_RANGE_JPEG, 0);
 
-            /* Override YUV420P default settings to have the correct (MPEG-2) chroma positions
-             * MPEG-2 chroma positions are used by convention
-             * XXX: support other 4:2:0 pixel formats */
-            if (inlink0->format == AV_PIX_FMT_YUV420P && scale->in_v_chr_pos == -513) {
+            /* Override chroma location default settings to have the correct
+             * chroma positions. MPEG chroma positions are used by convention.
+             * Note that this works for both MPEG-1/JPEG and MPEG-2/4 chroma
+             * locations, since they share a vertical alignment */
+            if (desc->log2_chroma_h == 1 && scale->in_v_chr_pos == -513) {
                 in_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192;
             }
 
-            if (outlink->format == AV_PIX_FMT_YUV420P && scale->out_v_chr_pos == -513) {
+            if (outdesc->log2_chroma_h == 1 && scale->out_v_chr_pos == -513) {
                 out_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192;
             }
 
-- 
2.42.0

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

Re: [FFmpeg-devel] [PATCH] avfilter/vf_scale: fix interlaced chroma for other formats

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1528 bytes --]

On Fri, Oct 13, 2023 at 04:22:05PM +0200, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> This logic only covers the case of yuv420p. Extend this logic to cover
> *all* vertically subsampled YUV formats, which require the same
> interlaced scaling logic.
> 
> Fortunately, we can get away with re-using the same code for both JPEG
> and MPEG range YUV, because the only difference here is the horizontal
> alignment. (To be fixed in a separate commit)
> ---
>  libavfilter/vf_scale.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

the patches from H4JO.txt
cause several fate tests to worsen in their stddev. like:

--- ./tests/ref/vsynth/vsynth1-mjpeg-huffman    2023-09-29 01:05:25.534962942 +0200
+++ tests/data/fate/vsynth1-mjpeg-huffman       2023-10-13 20:45:05.228633099 +0200
@@ -1,4 +1,4 @@
 63ea9bd494e16bad8f3a0c8dbb3dc11e *tests/data/fate/vsynth1-mjpeg-huffman.avi
 1391380 tests/data/fate/vsynth1-mjpeg-huffman.avi
-9a3b8169c251d19044f7087a95458c55 *tests/data/fate/vsynth1-mjpeg-huffman.out.rawvideo
-stddev:    7.87 PSNR: 30.21 MAXDIFF:   63 bytes:  7603200/  7603200
+64e440d0421e6b1bf3fbbc539b53e09c *tests/data/fate/vsynth1-mjpeg-huffman.out.rawvideo
+stddev:    8.37 PSNR: 29.67 MAXDIFF:   69 bytes:  7603200/  7603200



This patch in this mail here seems fine

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

Re: [FFmpeg-devel] [PATCH] avfilter/vf_scale: fix interlaced chroma for other formats

[Niklas Haas]

On Sat, 14 Oct 2023 00:52:23 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Fri, Oct 13, 2023 at 04:22:05PM +0200, Niklas Haas wrote:
> > From: Niklas Haas <git@haasn.dev>
> > 
> > This logic only covers the case of yuv420p. Extend this logic to cover
> > *all* vertically subsampled YUV formats, which require the same
> > interlaced scaling logic.
> > 
> > Fortunately, we can get away with re-using the same code for both JPEG
> > and MPEG range YUV, because the only difference here is the horizontal
> > alignment. (To be fixed in a separate commit)
> > ---
> >  libavfilter/vf_scale.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> the patches from H4JO.txt
> cause several fate tests to worsen in their stddev. like:

I investigated these regressions and came to the conclusion that the raw
input to those tests use mpeg1/jpeg/center-aligned chroma, but the
rawvideo demuxer does not tag them as such.

So this change in logic (i.e. treating unspecified yuv as mpeg2/mpeg4
chroma loc instead of mpeg1/jpeg chroma loc) regresses those tests by
design.

A solution would either to continue treating unspecified yuv as
mpeg1/jpeg chroma loc (status quo), or change the FATE test to
explicitly mark the rawvideo source as center chroma.

That said, if the status quo for the past decades is to for vf_scale
treat unspecified chroma loc as center-aligned, I am no longer sure if
suddenly changing this behavior is a good idea. At the same time, this
is also terribly inconsistent across implementations. For example, VLC
treats all chroma as center-aligned (ignoring tags), mpv treats untagged
*limited range* yuv as mpeg2/left-aligned (and full range as
mpeg1/jpeg/center), while libplacebo treats all untagged yuv as
mpeg2/left-aligned. There really is no consistent standard here across
software, and I haven't even looked at what proprietary players do.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_scale: fix interlaced chroma for other formats

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2990 bytes --]

On Sat, Oct 14, 2023 at 01:00:50AM +0200, Niklas Haas wrote:
> On Sat, 14 Oct 2023 00:52:23 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Fri, Oct 13, 2023 at 04:22:05PM +0200, Niklas Haas wrote:
> > > From: Niklas Haas <git@haasn.dev>
> > > 
> > > This logic only covers the case of yuv420p. Extend this logic to cover
> > > *all* vertically subsampled YUV formats, which require the same
> > > interlaced scaling logic.
> > > 
> > > Fortunately, we can get away with re-using the same code for both JPEG
> > > and MPEG range YUV, because the only difference here is the horizontal
> > > alignment. (To be fixed in a separate commit)
> > > ---
> > >  libavfilter/vf_scale.c | 12 +++++++-----
> > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > the patches from H4JO.txt
> > cause several fate tests to worsen in their stddev. like:
> 
> I investigated these regressions and came to the conclusion that the raw
> input to those tests use mpeg1/jpeg/center-aligned chroma, but the
> rawvideo demuxer does not tag them as such.
> 
> So this change in logic (i.e. treating unspecified yuv as mpeg2/mpeg4
> chroma loc instead of mpeg1/jpeg chroma loc) regresses those tests by
> design.
> 

> A solution would either to continue treating unspecified yuv as
> mpeg1/jpeg chroma loc (status quo), or change the FATE test to
> explicitly mark the rawvideo source as center chroma.

do we even have fate tests for all chroma locs ?
when you are already working on tuning these. Maybe some quick test
could cycle through the cases and test all


> 
> That said, if the status quo for the past decades is to for vf_scale
> treat unspecified chroma loc as center-aligned, I am no longer sure if
> suddenly changing this behavior is a good idea. At the same time, this
> is also terribly inconsistent across implementations. For example, VLC
> treats all chroma as center-aligned (ignoring tags), mpv treats untagged
> *limited range* yuv as mpeg2/left-aligned (and full range as
> mpeg1/jpeg/center), while libplacebo treats all untagged yuv as
> mpeg2/left-aligned. There really is no consistent standard here across
> software, and I haven't even looked at what proprietary players do.

I dont have a good awnser here either. I liked the result you get from
taking the samples in the middle of rectangles and tiling the whole
continuous image with a plane of rectangles for each luma and chroma plane
I felt long ago that was the simplest and most logic way to position
chroma in relation to luma.

but, some chroma loc autodetection filter that uses correlation or such
would be interresting given this mess.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

Re: [FFmpeg-devel] [PATCH] avfilter/vf_scale: fix interlaced chroma for other formats

[Cosmin Stejerean via ffmpeg-devel]

> On Oct 13, 2023, at 4:00 PM, Niklas Haas <ffmpeg@haasn.xyz> wrote:
> 
> That said, if the status quo for the past decades is to for vf_scale
> treat unspecified chroma loc as center-aligned, I am no longer sure if
> suddenly changing this behavior is a good idea.

I'd say that the current default (jpeg chroma loc for untagged) is more likely to be wrong than right in practice. It is definitely a change but while we're fixing lots of color related issues the next major release this might be a good time to draw a line in the sand and fix the defaults going forward to be the typical case (while perhaps trying to minimize the instances that hit this default path and warning loudly when it does).

That said this seems like the kind of change that should be in a 7.0 release rather than a 6.1 release if a 6.1 is going to happen.

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