[FFmpeg-devel] [PATCH] avformat/movenc: remove the write_clli mov flag

[James Almer]

On 4/13/2020 1:12 PM, Michael Bradshaw wrote:
> The clli atom is expected to be standardized soon. See
> http://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/259529.html
> 
> This patch will write the clli atom by default.
> 
> Please review.

> From a3e74f36c8035716bd89796547722a05e7e014de Mon Sep 17 00:00:00 2001
> From: Michael Bradshaw <mjbshaw at google.com>
> Date: Mon, 13 Apr 2020 10:08:43 -0600
> Subject: [PATCH] avformat/movenc: remove the write_clli mov flag
> 
> The clli atom is expected to be standardized soon. See
> http://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/259529.html
> 
> We now write the clli atom by default.
> 
> Signed-off-by: Michael Bradshaw <mjbshaw at google.com>
> ---
>  libavformat/movenc.c | 11 +++--------
>  libavformat/movenc.h |  1 -
>  2 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 3042e3ccd2..880434d6ef 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -77,7 +77,6 @@ static const AVOption options[] = {
>      { "delay_moov", "Delay writing the initial moov until the first fragment is cut, or until the first fragment flush", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_DELAY_MOOV}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
>      { "global_sidx", "Write a global sidx index at the start of the file", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_GLOBAL_SIDX}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
>      { "skip_sidx", "Skip writing of sidx atom", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_SKIP_SIDX}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
> -    { "write_clli", "Write clli atom (Experimental, may be renamed or changed, do not use from scripts)", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_WRITE_CLLI}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
>      { "write_colr", "Write colr atom (Experimental, may be renamed or changed, do not use from scripts)", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_WRITE_COLR}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
>      { "prefer_icc", "If writing colr atom prioritise usage of ICC profile if it exists in stream packet side data", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_PREFER_ICC}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
>      { "write_gama", "Write deprecated gama atom", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_WRITE_GAMA}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" },
> @@ -1950,7 +1949,7 @@ static int mov_write_clli_tag(AVIOContext *pb, MOVTrack *track)
>  
>      side_data = av_stream_get_side_data(track->st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL, NULL);
>      if (!side_data) {
> -        av_log(NULL, AV_LOG_WARNING, "Not writing 'clli' atom. No content light level info.\n");
> +        av_log(NULL, AV_LOG_INFO, "Not writing 'clli' atom. No content light level info.\n");

Should be AV_LOG_VERBOSE, or just removed. Otherwise every muxing
process where there's no CLL side data will print this, and that'll be
the vast majority of cases.

>          return 0;
>      }
>      content_light_metadata = (const AVContentLightMetadata*)side_data;
> @@ -2135,12 +2134,8 @@ static int mov_write_video_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContex
>          else
>              av_log(mov->fc, AV_LOG_WARNING, "Not writing 'colr' atom. Format is not MOV or MP4.\n");
>      }
> -    if (mov->flags & FF_MOV_FLAG_WRITE_CLLI) {
> -        if (track->mode == MODE_MOV || track->mode == MODE_MP4)
> -            mov_write_clli_tag(pb, track);
> -        else
> -            av_log(mov->fc, AV_LOG_WARNING, "Not writing 'clli' atom. Format is not MOV or MP4.\n");
> -    }
> +    if (track->mode == MODE_MOV || track->mode == MODE_MP4)
> +        mov_write_clli_tag(pb, track);
>  
>      if (track->mode == MODE_MP4 && mov->fc->strict_std_compliance <= FF_COMPLIANCE_UNOFFICIAL) {
>          AVStereo3D* stereo_3d = (AVStereo3D*) av_stream_get_side_data(track->st, AV_PKT_DATA_STEREO3D, NULL);
> diff --git a/libavformat/movenc.h b/libavformat/movenc.h
> index a7a0841f55..997b2d61c0 100644
> --- a/libavformat/movenc.h
> +++ b/libavformat/movenc.h
> @@ -261,7 +261,6 @@ typedef struct MOVMuxContext {
>  #define FF_MOV_FLAG_SKIP_SIDX             (1 << 21)
>  #define FF_MOV_FLAG_CMAF                  (1 << 22)
>  #define FF_MOV_FLAG_PREFER_ICC            (1 << 23)
> -#define FF_MOV_FLAG_WRITE_CLLI            (1 << 24)
>  
>  int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt);


LGTM otherwise.