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