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

[Andreas Rheinhardt]

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.

Ok, the gain of non-cartesian sets don't warrant the complexity incurred.

- Andreas