At the risk of being too non-committal, attached patch is mostly the same but leaves the write_colr flag as experimental. I would love to remove the write_colr flag entirely but we should provide some kind of escape hatch to allow ffmpeg to write "unspecified" (enum value 2) values for the colr atom (after all, these are legitimate values in H.273). Here are the reasons I'm not sold on stabilizing write_colr right now:
- It only fixes mov files. mkv has the same problem. It would be nice to stabilize a solution that isn't per-format. - I'm not motivated to spend the time exploring and debating alternative solutions like signaling "unspecified" (enum value 2) values differently from "unset" (no value from H.273) somehow (e.g., use enum value -1, or add a separate bool flag to signal whether the values are just ffmpeg defaults or not). - This approach solves the use cases with which I'm most familiar. I'm not as familiar with situations where people want to write "unspecified" enum values in the colr atom. I'd rather leave this for other people (who hopefully have more experience (and opinions) on this than I do).
From d6fcb5705d2c451b9ec1146b4fe66517f9eb6692 Mon Sep 17 00:00:00 2001 From: Michael Bradshaw <mjbshaw@google.com> Date: Mon, 13 Apr 2020 11:11:38 -0600 Subject: [PATCH] avformat/movenc: write the colr atom by default The write_colr flag has been marked as experimental for over 5 years. It should be safe to enable its behavior by default as follows: - Write the colr atom by default for mp4/mov if any of the following: - The primaries/trc/matrix are all specified, OR - There is an ICC profile, OR - The user specified +write_colr - Keep the write_colr flag for situations where the user wants to write the colr atom even if the color info is unspecified (e.g., http://ffmpeg.org/pipermail/ffmpeg-devel/2020-March/259334.html) This fixes https://trac.ffmpeg.org/ticket/7961 Signed-off-by: Michael Bradshaw <mjbshaw@google.com> --- libavformat/movenc.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/libavformat/movenc.c b/libavformat/movenc.c index bc8d08044e..4a4c9ce481 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -77,7 +77,7 @@ 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_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" }, + { "write_colr", "Write colr atom even if the color info is unspecified (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" }, { "use_metadata_tags", "Use mdta atom for metadata.", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_USE_MDTA}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "movflags" }, @@ -2135,11 +2135,17 @@ static int mov_write_video_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContex else av_log(mov->fc, AV_LOG_WARNING, "Not writing 'gama' atom. Format is not MOV.\n"); } - if (mov->flags & FF_MOV_FLAG_WRITE_COLR) { - if (track->mode == MODE_MOV || track->mode == MODE_MP4) - mov_write_colr_tag(pb, track, mov->flags & FF_MOV_FLAG_PREFER_ICC); - else - av_log(mov->fc, AV_LOG_WARNING, "Not writing 'colr' atom. Format is not MOV or MP4.\n"); + if (track->mode == MODE_MOV || track->mode == MODE_MP4) { + int has_color_info = track->par->color_primaries != AVCOL_PRI_UNSPECIFIED && + track->par->color_trc != AVCOL_TRC_UNSPECIFIED && + track->par->color_space != AVCOL_SPC_UNSPECIFIED; + if (has_color_info || mov->flags & FF_MOV_FLAG_WRITE_COLR || + av_stream_get_side_data(track->st, AV_PKT_DATA_ICC_PROFILE, NULL)) { + int prefer_icc = mov->flags & FF_MOV_FLAG_PREFER_ICC || !has_color_info; + mov_write_colr_tag(pb, track, prefer_icc); + } else if (mov->flags & FF_MOV_FLAG_WRITE_COLR) { + av_log(mov->fc, AV_LOG_WARNING, "Not writing 'colr' atom. Format is not MOV or MP4.\n"); + } } if (track->mode == MODE_MOV || track->mode == MODE_MP4) { mov_write_clli_tag(pb, track); -- 2.24.0.525.g8f36a354ae-goog
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".