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

[Niklas Haas <ffmpeg@haasn.xyz>]

On Sun, 24 Mar 2024 14:47:27 +0100 Damiano Galassi <damiog@gmail.com> wrote:
> On Sun, Mar 24, 2024 at 2:14 PM Niklas Haas <ffmpeg@haasn.xyz> wrote:
> 
> > On Sun, 24 Mar 2024 13:49:04 +0100 Damiano Galassi <damiog@gmail.com>
> > wrote:
> > > AVFilterLink colorspace and color_range are first set in
> > > avfiltergraph.c pick_format(),
> > > so in ff_filter_config_links() they will never be AVCOL_SPC_NONE or
> > > AVCOL_SPC_NONE.
> >
> > Wait, now I'm confused what this patch even accomplishes then. If it's
> > already set, what else is there to do?
> 
> 
> Because pick_format() doesn't set the right values, it doesn't know anything
> outside the AVFilterLink it's working on.
> So it sets a value that works for the link incfg and outcfg, but it doesn't
> propagate
> values between different links, and 99% of the times sets an unspecified
> value.

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?

> 
> Sorry if I'm not clear enough.
> _______________________________________________
> 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".
_______________________________________________
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".