diff mbox series

[FFmpeg-devel,v3] avformat/movenc: write the colr atom by default

Message ID CAHUoETJ_2pJTgHYeOoNOjoDAcpE2tAhkG5=Bee7bJybW9Z3BFA@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel,v3] avformat/movenc: write the colr atom by default | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Michael Bradshaw April 13, 2020, 10:14 p.m. UTC 
Attached is an updated patch that passes fate.

Comments

Michael Bradshaw Aug. 19, 2020, 8:10 p.m. UTC | #1 
On Mon, Apr 13, 2020 at 4:14 PM Michael Bradshaw <mjbshaw@google.com> wrote:

> Attached is an updated patch that passes fate.
>

I completely forgot about this patch. If no one objects over the next few
days I plan on pushing this. FATE still passes.
Paul B Mahol Aug. 19, 2020, 8:19 p.m. UTC | #2 
On 8/19/20, Michael Bradshaw <mjbshaw-at-google.com@ffmpeg.org> wrote:
> On Mon, Apr 13, 2020 at 4:14 PM Michael Bradshaw <mjbshaw@google.com> wrote:
>
>> Attached is an updated patch that passes fate.
>>
>
> I completely forgot about this patch. If no one objects over the next few
> days I plan on pushing this. FATE still passes.

Fine by me.
Michael Bradshaw Aug. 21, 2020, 5:06 p.m. UTC | #3 
On Wed, Aug 19, 2020 at 2:19 PM Paul B Mahol <onemda@gmail.com> wrote:

> On 8/19/20, Michael Bradshaw <mjbshaw-at-google.com@ffmpeg.org> wrote:
> > I completely forgot about this patch. If no one objects over the next few
> > days I plan on pushing this. FATE still passes.
>
> Fine by me.


Thanks. Pushed.
diff mbox series

Patch

From 28a67ab952dabb0bba0c1349a9eabffa74269b46 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 ++++++++++++------
 tests/ref/fate/copy-trac236 |  4 ++--
 2 files changed, 14 insertions(+), 8 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);
diff --git a/tests/ref/fate/copy-trac236 b/tests/ref/fate/copy-trac236
index 1583ae5704..c34b309b3f 100644
--- a/tests/ref/fate/copy-trac236
+++ b/tests/ref/fate/copy-trac236
@@ -1,5 +1,5 @@ 
-984a33c6292e3d35e2cfdfbf66d8e82b *tests/data/fate/copy-trac236.mov
-630860 tests/data/fate/copy-trac236.mov
+34b0ad38518b0eb8464aff04e6d0e143 *tests/data/fate/copy-trac236.mov
+630878 tests/data/fate/copy-trac236.mov
 #tb 0: 100/2997
 #media_type 0: video
 #codec_id 0: rawvideo
-- 
2.24.0.525.g8f36a354ae-goog