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

[Niklas Haas]

On Thu, 08 Feb 2024 13:33:51 +0100 Andreas Rheinhardt <andreas.rheinhardt at outlook.com> wrote:
> Sorry for not answering earlier.
> My intention is to allow users who only want to deal with the common
> case of a cartesian product to continue to do so, but to also support
> other usecases.
> The public function would look like
> 
> int avcodec_get_supported_config(const AVCodecContext *avctx, const
> AVCodec *codec, int **supported_configs, unsigned desired_configs,
> unsigned flags, void *logctx);
> 
> avctx can be NULL (in which case this allows to return all potential
> configurations, irrespective of e.g. the level of strictness).
> codec can be omitted if avctx->codec is set, but if both are supplied
> and avctx->codec is set, they have to match (like in avcodec_open2()).
> desired_configs is a bitfield of configs that the user wants to get
> information about; your patch would have to add flags for color_ranges
> and color_spaces.
> supported_configs will on return point to something like an array of
> struct { int desired_config0, desired_config1,...;}. The order of the
> entries will be fixed (say coincide with the order of the bits in the
> desired_configs bitfields).
> If one member is the unspec value for its type, then this means that it
> works with everything.
> supported_configs will be allocated; ownership passes to the user.
> Using a multidimensional sentinel (that would depend on desired_configs)
> is clumsy, so there will be two supported ways for this (depends upon a
> flag to be supplied in flags): One method that really adds a
> multidimensional sentinel, the other method that writes the number of
> entries into **supported_configs, so that the first entry starts at
> (*supported_configs)[1]. This allows users that only want to deal with
> the factors of a cartesian product separately to continue to do so.

OTOH this design will necessarily either result in exponential
explosion, or end up requiring the caller to make assumptions about
which fields are independent (and should thus be queried separately),
the moment a codec imposes *any* restriction (cartesian or not) on
multiple fields at the same time.

I also think that a `test()` callback, as I previously proposed, is also
overkill and doesn't actually solve anything. Codecs can already error
out on invalid configurations at open() time, and any practical use of
such an API would also end up having to make the cartesian assumption
one way or the other.

So in summary, I still think that we should enforce the assumption that
these fields form a cartesian set - it's simple, fast, useful and
doesn't overengineer for hypothetical constraints that we couldn't realistically
address one way or the other. Afaict, all current codecs support
a cartesian set of metadata, but feel free to correct me if I'm wrong.