Re: [FFmpeg-devel] [PATCH 1/2] avcodec: add YUV color space metadata to AVCodec

[Niklas Haas <ffmpeg@haasn.xyz>]

On Fri, 09 Feb 2024 13:11:38 +0100 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> On Mon, 05 Feb 2024 19:04:30 +0100 Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> > This presumes the relevant states to be a cartesian product. Which need
> > not be true. A callback would be better; this would also allow to base
> > the list on other values already set in an AVCodecContext. And if this
> > were extended, it would also allow to remove init_static_data one day.
> > It is furthermore quite wasteful to store color_ranges in a list,
> > although there are only very few states for it.
> 
> There is also the consideration to be made that using a callback is
> inconsistent with the established design. Consider that framerates,
> pix_fmts, samplerates, sample fmts and channel layouts are all currently
> provided as static arrays in AVCodec. There is a natural symmetry
> between these items and the ones I intend to add (yuv matrix, range,
> chroma location, primaries and gamma) - all of them are descriptive of
> the way data is encoded, and are therefore also (or should be)
> negotiable filter link properties.
> 
> If we add a new callback API, should we then extend it to also include
> all of the existing items from the above list? Is there a reason that
> yuv range supports needs to be more dynamic than the others?
> 
> Food for thought: mjpeg is not the only codec that puts restrictions on
> the format support based on the strictness level. For example,
> yuv4mpegpipe_muxer errors out with a strictness warning if you use
> a non-standard pixel format. And arguably, in this case, this is
> **preferred** behavior over "silently" inserting a scale filter to
> convert to a supported format, as the whole point of y4m2 is to
> encapsulate raw data as-is.
> 
> Should we:
> 
> 1. Add a new dynamic callback that can query lists for all of the above
>    in a way dependent on the strictness level, and use it as
>    a replacement for the static lists currently in AVCodec?
> 
> 2. Continue with the status quo of having these lists be static, plus
>    dynamic checks at open() time, and continue using the "convenience
>    hack" of having ffmpeg_tools automatically restrict limited range mjpeg?
> 
> It is not immediately obvious to me that an automatic conversion to
> a supported format is *necessarily* preferred to erroring out unless the
> user specifies a lower strictness level.
> 
> As for an API, I think that rather than having an AVCodecContext-aware
> callback at all, I would just make callbacks that directly ingest the
> strictness level in AVCodec. That makes it far less of a black box about
> which fields of the AVCodecContext are relevant here.
> 
> i.e.
> 
> struct AVCodec {
>     const enum AVColorRange (*get_color_ranges)(int strictness);
>     const enum AVColorSpace (*get_color_spaces)(int strictness);
>     // ditto for the other parameters?
> }

Ping for review?
_______________________________________________
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".