Re: [FFmpeg-devel] [PATCH 01/10] avcodec: add dolbyvision option

From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 01/10] avcodec: add dolbyvision option
Date: Wed, 3 Apr 2024 18:29:06 +0200
Message-ID: <GV1P250MB0737AB629D0C2DF2FAF88BAB8F3D2@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20240403154330.71585-1-ffmpeg@haasn.xyz>

Niklas Haas:
> From: Niklas Haas <git@haasn.dev>
> 
> Tri-state yes/no/auto option. Allows users to set `dolbyvision` to `no`
> to suppress coding dolby vision even when supported by the target codec.
> ---
>  doc/APIchanges             |  3 +++
>  doc/codecs.texi            | 12 ++++++++++++
>  libavcodec/avcodec.h       | 11 +++++++++++
>  libavcodec/options_table.h |  2 ++
>  libavcodec/version.h       |  2 +-
>  5 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 7eda1321cb0..a4484ceb670 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07
>  
>  API changes, most recent first:
>  
> +2024-03-23 - f17e18d2922 - lavc 61.6.100 - avcodec.h
> +  Add AVCodecContext.dolbyvision option.
> +
>  2024-04-xx - xxxxxxxxxx - lavu 59.12.100 - dovi_meta.h
>    Add AVDOVIMetadata.ext_block_{offset,size}, AVDOVIMetadata.num_ext_blocks,
>    AVDOVIDmData and AVDOVIDmLevel{1..6,8..11,254..255}, av_dovi_get_ext()
> diff --git a/doc/codecs.texi b/doc/codecs.texi
> index 6bdeb664e72..7203adc0489 100644
> --- a/doc/codecs.texi
> +++ b/doc/codecs.texi
> @@ -1018,6 +1018,18 @@ Note: The required alignment depends on if @code{AV_CODEC_FLAG_UNALIGNED} is set
>  CPU. @code{AV_CODEC_FLAG_UNALIGNED} cannot be changed from the command line. Also hardware
>  decoders will not apply left/top Cropping.
>  
> +@item dolbyvision @var{integer} (@emph{encoding,video})
> +Whether to encode Dolby Vision metadata when transcoding.
> +Possible values:
> +@table @samp
> +@item auto
> +Enable when coded frames contain Dolby Vision side data (default)
> +@item yes/on
> +Enable always, error out when frames do not contain metadata
> +@item no/off
> +Disable always, strip any tagged metadata
> +@end table
> +
>  
>  @end table
>  
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 83dc487251c..f54f758608d 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2075,6 +2075,17 @@ typedef struct AVCodecContext {
>       */
>      AVFrameSideData  **decoded_side_data;
>      int             nb_decoded_side_data;
> +
> +    /**
> +     * Video encoding only. Whether to mark the coded stream as Dolby Vision.
> +     * If set to FF_DOLBYVISION_AUTO, this will be enabled only if
> +     * decoded_side_data contains a valid RPU.
> +     *
> +     * If enabled, sending frames without AV_FRAME_DATA_DOVI_METADATA is
> +     * considered an error.
> +     */
> +    int dolbyvision;
> +#define FF_DOLBYVISION_AUTO -1
>  } AVCodecContext;
>  
>  /**
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index 7a2ef3474e7..d92269d2ff7 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -407,6 +407,8 @@ static const AVOption avcodec_options[] = {
>      {"mastering_display_metadata",  .default_val.i64 = AV_PKT_DATA_MASTERING_DISPLAY_METADATA,  .type = AV_OPT_TYPE_CONST, .flags = A|D, .unit = "side_data_pkt" },
>      {"content_light_level",         .default_val.i64 = AV_PKT_DATA_CONTENT_LIGHT_LEVEL,         .type = AV_OPT_TYPE_CONST, .flags = A|D, .unit = "side_data_pkt" },
>      {"icc_profile",                 .default_val.i64 = AV_PKT_DATA_ICC_PROFILE,                 .type = AV_OPT_TYPE_CONST, .flags = A|D, .unit = "side_data_pkt" },
> +{"dolbyvision", "flag stream as Dolby Vision", OFFSET(dolbyvision), AV_OPT_TYPE_INT, {.i64 = FF_DOLBYVISION_AUTO }, -1, 1, V|E, .unit = "dolbyvision" },
> +{"auto", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_DOLBYVISION_AUTO }, .flags = V|E, .unit = "dolbyvision" },
>  {NULL},
>  };
>  
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 7aa95fc3f1c..da54f878874 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>  
>  #include "version_major.h"
>  
> -#define LIBAVCODEC_VERSION_MINOR   5
> +#define LIBAVCODEC_VERSION_MINOR   6
>  #define LIBAVCODEC_VERSION_MICRO 100
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \

Why is this a global option when only a handful of encoders support it?
Apart from that, adding an option is not an APIChange.

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