[FFmpeg-devel] [PATCH v2] avfilter: propagate colorspace and color_range from buffer filter and between AVFilterLink.

[Niklas Haas]

On Sun, 24 Mar 2024 20:03:03 +0100 Damiano Galassi <damiog at gmail.com> wrote:
> On Sun, Mar 24, 2024 at 6:53 PM Niklas Haas <ffmpeg at haasn.xyz> wrote:
> 
> > But wait - aren't all filter's lists set to the same reference? Isn't that
> > the
> > point of the design? If they share the same format list, they will all
> > inherit the correct setting (via pick_format). Except for filters like
> > vf_scale which deliberately use separate format lists, since they can
> > perform conversion. But even in this case,
> > swap_color_spaces_on_filters() should ensure that both lists are set to
> > the same value IFF they support the same color spaces.
> >
> > Upon further inspection, it actually appears like the propagation you
> > propose in this patch is only needed for things which *aren't* subject
> > to format negotiation, which is why width, height and sample aspect
> > ratio are propagated here, but things like format or sample rates are not.
> >
> > So this patch as written should not be applied, I think.
> >
> > > What I meant was that your patch didn't make any difference from the
> > > existing behavior
> > > because even if it sets the default AVFilterLink values to AVCOL_SPC_NONE
> > > and AVCOL_SPC_NONE,
> > > when it's the time to call ff_filter_config_links(), those two values
> > have
> > > already been reset
> > > to a default unspecified value, so you still get a wrongly configure
> > graph
> > > with something like:
> > >
> > > Buffer
> > >     Link:  AVCOL_SPC_BT709 AVCOL_RANGE_MPEG
> > > Scale
> > >     Link: AVCOL_SPC_UNSPECIFIED AVCOL_RANGE_UNSPECIFIED
> > > Whateverfilter
> > >     Link: AVCOL_SPC_UNSPECIFIED AVCOL_RANGE_UNSPECIFIED
> > > Buffersink
> > >
> > > instead of
> > >
> > > Buffer
> > >     Link: AVCOL_SPC_BT709 AVCOL_RANGE_MPEG
> > > Scale
> > >     Link: AVCOL_SPC_BT709 AVCOL_RANGE_MPEG
> > > Whateverfilter
> > >     Link: AVCOL_SPC_BT709 AVCOL_RANGE_MPEG
> > > Buffersink
> >
> > Perhaps you can describe this scenario more thoroughly? What filters are
> > you using downstream of 'scale' here, and does it advertise BT709 as
> > supported?
> >
> 
> It's entirely possible that I am completely wrong, I took a look at
> avfilter code
> for the first time just a few days ago.
> 
> Here's two way to reproduce with the cli something similar to what I am
> seeing
> when using the API directly:
> 
> ffmpeg -i in.mp4 -vf
> "scale='width=1920:height=1080',zscale='width=1920:height=1080'" out.mp4
> ffmpeg -i in.jpg -vf "zscale='width=1920:height=1080'" out.jpg
> 
> it doesn't need a specific mp4 or jpg file.

So, in the second case, the bug is actually that buffersrc.c has a check
ff_fmt_is_regular_yuv() in order to set the YUV colorspace/matrix
fields, but YUVJ formats return 0 for this function. So the colorspace
never actually gets set to anything, it is left as UNSPECIFIED, which
then propagates through the filter chain, eventually leading to zimg
trying to convert from the real colorspace to unspec, which fails.

In retrospect, this logic is wrong - since YUV matrix still applies to
irregular YUV formats. We should split the check into is_yuv() and
is_yuv_forced_full_range(), and handle the overlapping cases
accordingly.

I will fix it.