diff mbox series

[FFmpeg-devel,v2] avformat/mxfdec: Read color metadata from MXF

Message ID 3AAD3CB2-80D0-40C7-BF2E-A21D8E57BE36@codex.online
State Accepted
Commit 7031a7beae5af7648c885e05435f179beb00d4d2
Headers show
Series [FFmpeg-devel,v2] avformat/mxfdec: Read color metadata from MXF | expand

Checks

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

Commit Message

Harry Mallon July 31, 2020, 10:09 a.m. UTC 
Hi,

V2 fixes all the fate tests (I didn't know to download the test footage before. Doh).

The following patch adds reading colour metadata (transfer, primaries and colour space) from
standard MXF files. I wrote it without seeing the other patch but it turned out very similar to this
one that was never merged.

https://patchwork.ffmpeg.org/project/ffmpeg/patch/1474694344-31167-1-git-send-email-steven@strobe.cc/ .

Thanks.
Harry

From d16966e6b0a6367a5e5445987c0449fb1150785a Mon Sep 17 00:00:00 2001

From: Harry Mallon <harry.mallon@codex.online>
Date: Mon, 27 Jul 2020 15:52:17 +0100
Subject: [PATCH] avformat/mxfdec: Read color metadata from MXF

Reads color_primaries, color_trc and color_space from mxf
headers. ULs are from https://registry.smpte-ra.org/ site.

Signed-off-by: Harry Mallon <harry.mallon@codex.online>
---
 libavformat/mxf.c                       | 49 +++++++++++++++++++++++++
 libavformat/mxf.h                       |  3 ++
 libavformat/mxfdec.c                    | 15 ++++++++
 tests/ref/fate/mxf-d10-user-comments    |  2 +-
 tests/ref/fate/mxf-opatom-user-comments |  2 +-
 tests/ref/fate/mxf-probe-d10            |  2 +-
 tests/ref/fate/mxf-probe-dnxhd          |  4 +-
 tests/ref/fate/mxf-probe-dv25           |  2 +-
 tests/ref/fate/mxf-reel_name            |  2 +-
 tests/ref/fate/mxf-user-comments        |  2 +-
 10 files changed, 75 insertions(+), 8 deletions(-)

Comments

Paul B Mahol July 31, 2020, 3:57 p.m. UTC | #1 
On 7/31/20, Harry Mallon <harry.mallon@codex.online> wrote:
> Hi,
>
> V2 fixes all the fate tests (I didn't know to download the test footage
> before. Doh).
>
> The following patch adds reading colour metadata (transfer, primaries and
> colour space) from
> standard MXF files. I wrote it without seeing the other patch but it turned
> out very similar to this
> one that was never merged.
>
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1474694344-31167-1-git-send-email-steven@strobe.cc/
> .
>
> Thanks.
> Harry
>
> From d16966e6b0a6367a5e5445987c0449fb1150785a Mon Sep 17 00:00:00 2001
>
> From: Harry Mallon <harry.mallon@codex.online>
> Date: Mon, 27 Jul 2020 15:52:17 +0100
> Subject: [PATCH] avformat/mxfdec: Read color metadata from MXF
>
> Reads color_primaries, color_trc and color_space from mxf
> headers. ULs are from https://registry.smpte-ra.org/ site.
>
> Signed-off-by: Harry Mallon <harry.mallon@codex.online>
> ---
>  libavformat/mxf.c                       | 49 +++++++++++++++++++++++++
>  libavformat/mxf.h                       |  3 ++
>  libavformat/mxfdec.c                    | 15 ++++++++
>  tests/ref/fate/mxf-d10-user-comments    |  2 +-
>  tests/ref/fate/mxf-opatom-user-comments |  2 +-
>  tests/ref/fate/mxf-probe-d10            |  2 +-
>  tests/ref/fate/mxf-probe-dnxhd          |  4 +-
>  tests/ref/fate/mxf-probe-dv25           |  2 +-
>  tests/ref/fate/mxf-reel_name            |  2 +-
>  tests/ref/fate/mxf-user-comments        |  2 +-
>  10 files changed, 75 insertions(+), 8 deletions(-)
>

LGTM, but better wait for actual maintainer(s).
Tomas Härdin Aug. 1, 2020, 2:17 p.m. UTC | #2 
fre 2020-07-31 klockan 11:09 +0100 skrev Harry Mallon:
> Hi,
> 
> V2 fixes all the fate tests (I didn't know to download the test
> footage before. Doh).
> 
> The following patch adds reading colour metadata (transfer, primaries
> and colour space) from
> standard MXF files. I wrote it without seeing the other patch but it
> turned out very similar to this
> one that was never merged.
> 
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1474694344-31167-1-git-send-email-steven@strobe.cc/
> .
> 
> Thanks.
> Harry
> 
> From d16966e6b0a6367a5e5445987c0449fb1150785a Mon Sep 17 00:00:00
> 2001
> 
> From: Harry Mallon <harry.mallon@codex.online>
> Date: Mon, 27 Jul 2020 15:52:17 +0100
> Subject: [PATCH] avformat/mxfdec: Read color metadata from MXF
> 
> Reads color_primaries, color_trc and color_space from mxf
> headers. ULs are from https://registry.smpte-ra.org/ site.
> 
> Signed-off-by: Harry Mallon <harry.mallon@codex.online>
> ---
>  libavformat/mxf.c                       | 49
> +++++++++++++++++++++++++
>  libavformat/mxf.h                       |  3 ++
>  libavformat/mxfdec.c                    | 15 ++++++++
>  tests/ref/fate/mxf-d10-user-comments    |  2 +-
>  tests/ref/fate/mxf-opatom-user-comments |  2 +-
>  tests/ref/fate/mxf-probe-d10            |  2 +-
>  tests/ref/fate/mxf-probe-dnxhd          |  4 +-
>  tests/ref/fate/mxf-probe-dv25           |  2 +-
>  tests/ref/fate/mxf-reel_name            |  2 +-
>  tests/ref/fate/mxf-user-comments        |  2 +-
>  10 files changed, 75 insertions(+), 8 deletions(-)

Looks OK to me.

/Tomas
Tomas Härdin Aug. 6, 2020, 11:22 a.m. UTC | #3 
lör 2020-08-01 klockan 16:17 +0200 skrev Tomas Härdin:
> fre 2020-07-31 klockan 11:09 +0100 skrev Harry Mallon:
> > Hi,
> > 
> > V2 fixes all the fate tests (I didn't know to download the test
> > footage before. Doh).
> > 
> > The following patch adds reading colour metadata (transfer, primaries
> > and colour space) from
> > standard MXF files. I wrote it without seeing the other patch but it
> > turned out very similar to this
> > one that was never merged.
> > 
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/1474694344-31167-1-git-send-email-steven@strobe.cc/
> > .
> > 
> > Thanks.
> > Harry
> > 
> > From d16966e6b0a6367a5e5445987c0449fb1150785a Mon Sep 17 00:00:00
> > 2001
> > 
> > From: Harry Mallon <harry.mallon@codex.online>
> > Date: Mon, 27 Jul 2020 15:52:17 +0100
> > Subject: [PATCH] avformat/mxfdec: Read color metadata from MXF
> > 
> > Reads color_primaries, color_trc and color_space from mxf
> > headers. ULs are from https://registry.smpte-ra.org/ site.
> > 
> > Signed-off-by: Harry Mallon <harry.mallon@codex.online>
> > ---
> >  libavformat/mxf.c                       | 49
> > +++++++++++++++++++++++++
> >  libavformat/mxf.h                       |  3 ++
> >  libavformat/mxfdec.c                    | 15 ++++++++
> >  tests/ref/fate/mxf-d10-user-comments    |  2 +-
> >  tests/ref/fate/mxf-opatom-user-comments |  2 +-
> >  tests/ref/fate/mxf-probe-d10            |  2 +-
> >  tests/ref/fate/mxf-probe-dnxhd          |  4 +-
> >  tests/ref/fate/mxf-probe-dv25           |  2 +-
> >  tests/ref/fate/mxf-reel_name            |  2 +-
> >  tests/ref/fate/mxf-user-comments        |  2 +-
> >  10 files changed, 75 insertions(+), 8 deletions(-)
> 
> Looks OK to me.
> 
> /Tomas

Pushed

/Tomas
diff mbox series

Patch

diff --git a/libavformat/mxf.c b/libavformat/mxf.c
index 7d154ca9d3..e51fc48a84 100644
--- a/libavformat/mxf.c
+++ b/libavformat/mxf.c
@@ -86,6 +86,55 @@  const MXFCodecUL ff_mxf_codec_tag_uls[] = {
     { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },  0,                         0 },
 };
 
+const MXFCodecUL ff_mxf_color_primaries_uls[] = {
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x03,0x01,0x00,0x00 }, 14, AVCOL_PRI_SMPTE170M }, /* SMPTE 170M */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x03,0x02,0x00,0x00 }, 14, AVCOL_PRI_BT470BG }, /* ITU-R BT.470 PAL */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x03,0x03,0x00,0x00 }, 14, AVCOL_PRI_BT709 }, /* ITU-R BT.709 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x03,0x04,0x00,0x00 }, 14, AVCOL_PRI_BT2020 }, /* ITU-R BT.2020 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x03,0x05,0x00,0x00 }, 14, AVCOL_PRI_SMPTE428 }, /* SMPTE-DC28 DCDM */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x03,0x06,0x00,0x00 }, 14, AVCOL_PRI_SMPTE432 }, /* P3D65 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x03,0x08,0x00,0x00 }, 14, AVCOL_PRI_SMPTE428 }, /* Cinema Mezzanine */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x03,0x0a,0x00,0x00 }, 14, AVCOL_PRI_SMPTE431 }, /* P3DCI */
+    /* alternate mappings for encoding */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x03,0x01,0x00,0x00 }, 14, AVCOL_PRI_SMPTE240M }, /* = AVCOL_PRI_SMPTE170M */
+
+    { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },  0, AVCOL_PRI_UNSPECIFIED },
+};
+
+const MXFCodecUL ff_mxf_color_trc_uls[] = {
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00 }, 14, AVCOL_TRC_GAMMA22 }, /* ITU-R BT.470 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00 }, 14, AVCOL_TRC_BT709 }, /* ITU-R BT.709 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x03,0x00,0x00 }, 14, AVCOL_TRC_SMPTE240M }, /* SMPTE 240M */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x04,0x00,0x00 }, 14, AVCOL_TRC_BT709 }, /* SMPTE 274/296M (must appear after ITU-R BT.709) */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x05,0x00,0x00 }, 14, AVCOL_TRC_BT1361_ECG }, /* ITU-R BT.1361 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x01,0x06,0x00,0x00 }, 14, AVCOL_TRC_LINEAR }, /* Linear */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x08,0x04,0x01,0x01,0x01,0x01,0x07,0x00,0x00 }, 14, AVCOL_TRC_SMPTE428 }, /* SMPTE-DC28 DCDM */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x01,0x08,0x00,0x00 }, 14, AVCOL_TRC_IEC61966_2_4 }, /* IEC 61966-2-4 xvYCC */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0E,0x04,0x01,0x01,0x01,0x01,0x09,0x00,0x00 }, 14, AVCOL_TRC_BT2020_10 }, /* ITU-R BT.2020 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x01,0x0A,0x00,0x00 }, 14, AVCOL_TRC_SMPTE2084 }, /* SMPTE ST 2084 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x01,0x0B,0x00,0x00 }, 14, AVCOL_TRC_ARIB_STD_B67 }, /* Hybrid Log-Gamma OETF */
+    /* alternate mappings for encoding */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x01,0x00,0x00 }, 14, AVCOL_TRC_GAMMA28 }, /* = AVCOL_TRC_GAMMA22 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x02,0x00,0x00 }, 14, AVCOL_TRC_SMPTE170M }, /* = AVCOL_TRC_BT709 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0E,0x04,0x01,0x01,0x01,0x01,0x09,0x00,0x00 }, 14, AVCOL_TRC_BT2020_12 }, /* = AVCOL_TRC_BT2020_10 */
+
+    { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },  0, AVCOL_TRC_UNSPECIFIED },
+};
+
+/* aka Coding Equations */
+const MXFCodecUL ff_mxf_color_space_uls[] = {
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x02,0x01,0x00,0x00 }, 14, AVCOL_SPC_BT470BG }, /* ITU-R BT.601 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x02,0x02,0x00,0x00 }, 14, AVCOL_SPC_BT709 }, /* ITU-R BT.709 */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x02,0x03,0x00,0x00 }, 14, AVCOL_SPC_SMPTE240M }, /* SMPTE 240M */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x02,0x04,0x00,0x00 }, 14, AVCOL_SPC_YCGCO }, /* YCgCo */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x02,0x05,0x00,0x00 }, 14, AVCOL_SPC_RGB }, /* GBR */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0D,0x04,0x01,0x01,0x01,0x02,0x06,0x00,0x00 }, 14, AVCOL_SPC_BT2020_NCL }, /* ITU-R BT.2020 Non-Constant Luminance */
+    /* alternate mappings for encoding */
+    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x06,0x04,0x01,0x01,0x01,0x02,0x03,0x00,0x00 }, 14, AVCOL_SPC_SMPTE170M }, /* = AVCOL_SPC_SMPTE240M */
+
+    { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },  0, AVCOL_SPC_UNSPECIFIED },
+};
+
 static const struct {
     enum AVPixelFormat pix_fmt;
     const char data[16];
diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index f2fff2781e..fc587f19f0 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -91,6 +91,9 @@  extern const MXFCodecUL ff_mxf_data_definition_uls[];
 extern const MXFCodecUL ff_mxf_codec_uls[];
 extern const MXFCodecUL ff_mxf_pixel_format_uls[];
 extern const MXFCodecUL ff_mxf_codec_tag_uls[];
+extern const MXFCodecUL ff_mxf_color_primaries_uls[];
+extern const MXFCodecUL ff_mxf_color_trc_uls[];
+extern const MXFCodecUL ff_mxf_color_space_uls[];
 
 int ff_mxf_decode_pixel_layout(const char pixel_layout[16], enum AVPixelFormat *pix_fmt);
 int ff_mxf_get_content_package_rate(AVRational time_base);
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index f0975f409e..4b56984b77 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -207,6 +207,9 @@  typedef struct MXFDescriptor {
     uint8_t *extradata;
     int extradata_size;
     enum AVPixelFormat pix_fmt;
+    UID color_primaries_ul;
+    UID color_trc_ul;
+    UID color_space_ul;
 } MXFDescriptor;
 
 typedef struct MXFIndexTableSegment {
@@ -1201,9 +1204,18 @@  static int mxf_read_generic_descriptor(void *arg, AVIOContext *pb, int tag, int
         descriptor->aspect_ratio.num = avio_rb32(pb);
         descriptor->aspect_ratio.den = avio_rb32(pb);
         break;
+    case 0x3210:
+        avio_read(pb, descriptor->color_trc_ul, 16);
+        break;
     case 0x3212:
         descriptor->field_dominance = avio_r8(pb);
         break;
+    case 0x3219:
+        avio_read(pb, descriptor->color_primaries_ul, 16);
+        break;
+    case 0x321A:
+        avio_read(pb, descriptor->color_space_ul, 16);
+        break;
     case 0x3301:
         descriptor->component_depth = avio_rb32(pb);
         break;
@@ -2480,6 +2492,9 @@  static int mxf_parse_structural_metadata(MXFContext *mxf)
             }
             if (descriptor->aspect_ratio.num && descriptor->aspect_ratio.den)
                 st->display_aspect_ratio = descriptor->aspect_ratio;
+            st->codecpar->color_primaries = mxf_get_codec_ul(ff_mxf_color_primaries_uls, &descriptor->color_primaries_ul)->id;
+            st->codecpar->color_trc       = mxf_get_codec_ul(ff_mxf_color_trc_uls, &descriptor->color_trc_ul)->id;
+            st->codecpar->color_space     = mxf_get_codec_ul(ff_mxf_color_space_uls, &descriptor->color_space_ul)->id;
         } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
             container_ul = mxf_get_codec_ul(mxf_sound_essence_container_uls, essence_container_ul);
             /* Only overwrite existing codec ID if it is unset or A-law, which is the default according to SMPTE RP 224. */
diff --git a/tests/ref/fate/mxf-d10-user-comments b/tests/ref/fate/mxf-d10-user-comments
index e78765020c..de4f26c6f8 100644
--- a/tests/ref/fate/mxf-d10-user-comments
+++ b/tests/ref/fate/mxf-d10-user-comments
@@ -1 +1 @@ 
-b659c1204f8d04e2a5607af083590dca
+68f0fa62b6a676894afbbe4c34ebf70b
diff --git a/tests/ref/fate/mxf-opatom-user-comments b/tests/ref/fate/mxf-opatom-user-comments
index 1834b9e074..90e3fb229a 100644
--- a/tests/ref/fate/mxf-opatom-user-comments
+++ b/tests/ref/fate/mxf-opatom-user-comments
@@ -1 +1 @@ 
-892cf02e44bf7d61b6d6f01e41db9375
+f6760a9e710ba478bc3949f3e5c9b34a
diff --git a/tests/ref/fate/mxf-probe-d10 b/tests/ref/fate/mxf-probe-d10
index b59fe4b597..17f47639f3 100644
--- a/tests/ref/fate/mxf-probe-d10
+++ b/tests/ref/fate/mxf-probe-d10
@@ -18,7 +18,7 @@  pix_fmt=yuv422p
 level=5
 color_range=tv
 color_space=unknown
-color_transfer=unknown
+color_transfer=bt470m
 color_primaries=unknown
 chroma_location=topleft
 field_order=tt
diff --git a/tests/ref/fate/mxf-probe-dnxhd b/tests/ref/fate/mxf-probe-dnxhd
index 5d385e8a06..012d3ea1d9 100644
--- a/tests/ref/fate/mxf-probe-dnxhd
+++ b/tests/ref/fate/mxf-probe-dnxhd
@@ -126,8 +126,8 @@  pix_fmt=yuv422p
 level=-99
 color_range=unknown
 color_space=bt709
-color_transfer=unknown
-color_primaries=unknown
+color_transfer=bt709
+color_primaries=bt709
 chroma_location=unspecified
 field_order=progressive
 timecode=N/A
diff --git a/tests/ref/fate/mxf-probe-dv25 b/tests/ref/fate/mxf-probe-dv25
index d0fb308c19..810f145f41 100644
--- a/tests/ref/fate/mxf-probe-dv25
+++ b/tests/ref/fate/mxf-probe-dv25
@@ -18,7 +18,7 @@  pix_fmt=yuv420p
 level=-99
 color_range=unknown
 color_space=unknown
-color_transfer=unknown
+color_transfer=bt470m
 color_primaries=unknown
 chroma_location=topleft
 field_order=bb
diff --git a/tests/ref/fate/mxf-reel_name b/tests/ref/fate/mxf-reel_name
index cfe62dfdb7..16022b1789 100644
--- a/tests/ref/fate/mxf-reel_name
+++ b/tests/ref/fate/mxf-reel_name
@@ -1 +1 @@ 
-a788589c14f343dcc6d75aaaec0f0266
+73a891041b2fc836a893ffb49fff4fff
diff --git a/tests/ref/fate/mxf-user-comments b/tests/ref/fate/mxf-user-comments
index e91b23baa5..ddf51d939c 100644
--- a/tests/ref/fate/mxf-user-comments
+++ b/tests/ref/fate/mxf-user-comments
@@ -1 +1 @@ 
-c6469c0ae2aaee602eacbc009080ae8e
+1255faf854223a74d707553121e5eca3