diff mbox series

[FFmpeg-devel] avformat/movenc: use enum values directly for colr atom

Message ID CAHUoETK4frWW-3+_MuC3h=6t+CgnF1ba9ZiA_sfheDEFA5O3zQ@mail.gmail.com
State Accepted
Headers show
Series [FFmpeg-devel] avformat/movenc: use enum values directly for colr atom | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Michael Bradshaw March 31, 2020, 4:08 p.m. UTC 
The switch cases were missing:

   - Primaries: bt470m, film, smpte428, and ebu3213.
   - TRCs: gamma22, gamma28, linear, log, log_sqrt, iec61966_2_4, bt1361,
   iec61966_2_1, bt2020_10bit, and bt2020_12bit.
   - Space: rgb, fcc, ycgco, bt2020_cl, smpte2085, chroma-derived-nc,
   chroma-derived-c, and ictcp.

They also annoyingly remapped the following (which are functionally
equivalent but can be treated differently by clients):

  - smpte240m primaries to smpte170m.
  - smpte170m TRC to bt709.
  - bt470bg color space to smpte170m.

The enum values in FFmpeg are the same values as ITU-T H.273 and ISO/IEC
23001-8 so we can just use them directly, which is both simpler and
preserves the user intent.

Comments

Michael Bradshaw April 13, 2020, 4:02 p.m. UTC | #1 
Given the lack of objections I've pushed this to the master branch.
diff mbox series

Patch

From 711e2b488c7208b0a02594b32e696a20b050e928 Mon Sep 17 00:00:00 2001
From: Michael Bradshaw <mjbshaw@google.com>
Date: Tue, 31 Mar 2020 09:36:31 -0600
Subject: [PATCH] avformat/movenc: use enum values directly for colr atom

The switch cases were missing:

  - Primaries: bt470m, film, smpte428, and ebu3213.
  - TRCs: gamma22, gamma28, linear, log, log_sqrt, iec61966_2_4, bt1361,
    iec61966_2_1, bt2020_10bit, and bt2020_12bit.
  - Space: rgb, fcc, ycgco, bt2020_cl, smpte2085, chroma-derived-nc,
    chroma-derived-c, and ictcp.

They also annoyingly remapped the following (which are functionally
equivalent but can be treated differently by clients):

  - smpte240m primaries to smpte170m.
  - smpte170m TRC to bt709.
  - bt470bg color space to smpte170m.

The enum values in FFmpeg are the same values as ITU-T H.273 and
ISO/IEC 23001-8 so we can just use them directly, which is both simpler
and preserves the user intent.

Signed-off-by: Michael Bradshaw <mjbshaw@google.com>
---
 libavformat/movenc.c | 31 +++----------------------------
 1 file changed, 3 insertions(+), 28 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 1c178fc4bc..6024d62c07 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1931,34 +1931,9 @@  static int mov_write_colr_tag(AVIOContext *pb, MOVTrack *track, int prefer_icc)
         ffio_wfourcc(pb, "nclx");
     else
         ffio_wfourcc(pb, "nclc");
-    switch (track->par->color_primaries) {
-    case AVCOL_PRI_BT709:     avio_wb16(pb, 1); break;
-    case AVCOL_PRI_BT470BG:   avio_wb16(pb, 5); break;
-    case AVCOL_PRI_SMPTE170M:
-    case AVCOL_PRI_SMPTE240M: avio_wb16(pb, 6); break;
-    case AVCOL_PRI_BT2020:    avio_wb16(pb, 9); break;
-    case AVCOL_PRI_SMPTE431:  avio_wb16(pb, 11); break;
-    case AVCOL_PRI_SMPTE432:  avio_wb16(pb, 12); break;
-    default:                  avio_wb16(pb, 2);
-    }
-    switch (track->par->color_trc) {
-    case AVCOL_TRC_BT709:        avio_wb16(pb, 1); break;
-    case AVCOL_TRC_SMPTE170M:    avio_wb16(pb, 1); break; // remapped
-    case AVCOL_TRC_SMPTE240M:    avio_wb16(pb, 7); break;
-    case AVCOL_TRC_SMPTEST2084:  avio_wb16(pb, 16); break;
-    case AVCOL_TRC_SMPTE428:     avio_wb16(pb, 17); break;
-    case AVCOL_TRC_ARIB_STD_B67: avio_wb16(pb, 18); break;
-    default:                     avio_wb16(pb, 2);
-    }
-    switch (track->par->color_space) {
-    case AVCOL_SPC_BT709:      avio_wb16(pb, 1); break;
-    case AVCOL_SPC_BT470BG:
-    case AVCOL_SPC_SMPTE170M:  avio_wb16(pb, 6); break;
-    case AVCOL_SPC_SMPTE240M:  avio_wb16(pb, 7); break;
-    case AVCOL_SPC_BT2020_NCL: avio_wb16(pb, 9); break;
-    default:                   avio_wb16(pb, 2);
-    }
-
+    avio_wb16(pb, track->par->color_primaries);
+    avio_wb16(pb, track->par->color_trc);
+    avio_wb16(pb, track->par->color_space);
     if (track->mode == MODE_MP4) {
         int full_range = track->par->color_range == AVCOL_RANGE_JPEG;
         avio_w8(pb, full_range << 7);
-- 
2.24.0.525.g8f36a354ae-goog