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

[Niklas Haas]

On Mon, 05 Feb 2024 19:04:30 +0100 Andreas Rheinhardt <andreas.rheinhardt at 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?
}