Re: [FFmpeg-devel] [PATCH v2 02/13] avcodec: add extended AVCodec color metadata

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Niklas Haas:
> From: Niklas Haas <git@haasn.dev>
> 
> This is motivated primarily by a desire for YUVJ removal, which will
> require signalling the supported color ranges as part of the codec
> capabilities. But since we're here anyway, we might as well add all of
> the metadata, which I foresee seeing more use in the future (e.g.
> automatic conversion from HDR to SDR when encoding to formats that don't
> support AVCOL_TRC_SMPTE2084, ...)
> ---
>  doc/APIchanges       | 4 ++++
>  libavcodec/codec.h   | 7 +++++++
>  libavcodec/version.h | 4 ++--
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 9b109e6fa7..f91e855e70 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,10 @@ The last version increases of all libraries were on 2023-02-09
>  
>  API changes, most recent first:
>  
> +2023-10-xx - xxxxxxxxxx - lavc 60.23.100 - avcodec.h
> +  Add AVCodec.csps, AVCodec.color_ranges, AVCodec.chroma_locs, AVCodec.primaries,
> +  AVCodec.trcs.
> +
>  2023-10-06 - xxxxxxxxxx - lavc 60.30.101 - avcodec.h
>    AVCodecContext.coded_side_data may now be used during decoding, to be set
>    by user before calling avcodec_open2() for initialization.
> diff --git a/libavcodec/codec.h b/libavcodec/codec.h
> index 8034f1a53c..5bc8f21868 100644
> --- a/libavcodec/codec.h
> +++ b/libavcodec/codec.h
> @@ -235,6 +235,13 @@ typedef struct AVCodec {
>       * Array of supported channel layouts, terminated with a zeroed layout.
>       */
>      const AVChannelLayout *ch_layouts;
> +
> +    /* Extended colorspace support metadata */
> +    const enum AVColorSpace *csps;                  ///< array of supported color spaces, or NULL if unknown, array is terminated by AVCOL_SPC_UNSPECIFIED
> +    const enum AVColorRange *color_ranges;          ///< array of supported color ranges, or NULL if unknown, array is terminated by 0
> +    const enum AVChromaLocation *chroma_locs;       ///< array of supported chroma locations, or NULL if unknown, array is terminated by 0
> +    const enum AVColorPrimaries *primaries;         ///< array of supported color primaries, or NULL if unknown, array is terminated by 0
> +    const enum AVColorTransferCharacteristic *trcs; ///< array of supported transfer characteristics, or NULL if known, array is terminated by 0
>  } AVCodec;
>  

This design has several drawbacks:
1. It adds stuff that will only be set by a tiny minority of AVCodec's
to all of them.
2. It is based around the underlying assumption that the set of
permissible states (tupels) is a cartesian product of a set of color
spaces, a set of color ranges etc. This is wrong: E.g. VP9 disallows
limited-range RGB (it is syntactically impossible to set the color range
when using RGB color space).
3. I don't see how the MJPEG encoder behaviour where the valid formats
de facto depend upon strictness can be encoded in this way; isn't the
aim to get rid of the necessity of the workaround in ffmpeg cli?

1. and 2. suggests using some form of function that returns a list of
supported tupels; if said function uses an AVCodecContext* parameter,
said list can depend upon the state of the AVCodecContext given to it,
thereby solving 3. to the extent that one can get the supported
combinations given AVCodecContext options (but I do not see a good way
to signal which options modify the supported combinations).

- Andreas

_______________________________________________
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".