diff mbox

[FFmpeg-devel,v3,2/2] libavformat/mxfenc: add missing dnxhr mxfcontainer essence ULs

Message ID 20180907080313.25744-3-jay@wizardofthenet.com
State Superseded
Headers show

Commit Message

jay@wizardofthenet.com Sept. 7, 2018, 8:03 a.m. UTC 
Add missing dnxhr mxf container essence ULs to the mxf encoder.
set proper mxf frame size for dnxhr using libavformat/dnxhd's get dnxhr size function.

This fixes dnxhr mxf files being quarantined by Avid Media Composer.

Signed-off-by: Jason Stevens <jay@wizardofthenet.com>
---
 libavformat/mxfenc.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

Comments

Tomas Härdin Sept. 7, 2018, 8:54 a.m. UTC | #1 
fre 2018-09-07 klockan 01:03 -0700 skrev Jason Stevens:
> Add missing dnxhr mxf container essence ULs to the mxf encoder.
> set proper mxf frame size for dnxhr using libavformat/dnxhd's get dnxhr size function.
> 
> This fixes dnxhr mxf files being quarantined by Avid Media Composer.
> 
> > Signed-off-by: Jason Stevens <jay@wizardofthenet.com>
> ---
>  libavformat/mxfenc.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 52 insertions(+), 1 deletion(-)
> 
> @@ -1959,7 +1989,11 @@ AVPacket *pkt)
>      header_cid = pkt->data + 0x28;
>      cid = header_cid[0] << 24 | header_cid[1] << 16 | header_cid[2] << 8 | header_cid[3];
>  
> -    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) < 0)
> +    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) == DNXHD_VARIABLE) {
> +        frame_size = avpriv_dnxhd_get_hr_frame_size(cid, st->codecpar->width, st->codecpar->height);
> +    }
> +
> +    if (frame_size < 0)
>          return -1;
>      if ((sc->interlaced = avpriv_dnxhd_get_interlaced(cid)) < 0)
>          return AVERROR_INVALIDDATA;
> @@ -1998,6 +2032,23 @@ AVPacket *pkt)
>      case 1253:
>          sc->index = INDEX_DNXHD_720p_8bit_LOW;
>          break;
> +    case 1274:
> +        sc->index = INDEX_DNXHR_LB;
> +        break;
> +    case 1273:
> +        sc->index = INDEX_DNXHR_SQ;
> +        break;
> +    case 1272:
> +        sc->index = INDEX_DNXHR_HQ;
> +        break;
> +    case 1271:
> +        sc->index = INDEX_DNXHR_HQX;
> +        sc->component_depth = 10;
> +        break;
> +    case 1270:
> +        sc->index = INDEX_DNXHR_444;
> +        sc->component_depth = 10;
> +        break;

Is there some spec one can reference to for these magic constants?

Perhaps this has been mentioned already, but mxf_parse_dnxhd_frame()
seems like something that could be useful for other muxers, assuming
DNxHR can be muxed into anything beyond MXF

/Tomas
jay@wizardofthenet.com Sept. 7, 2018, 1:42 p.m. UTC | #2 
I've not seen a smpte spec that contains the DNXHR mxf ULs.
The mxf UL definitions can be looked up at registry.smpte-ra.org
search for "VC-3", CIDs 1270-1274 are DNXHR.

DNXHR in already supported in mov and mkv.

-Jason

-----Original Message-----
From: "Tomas Härdin" <tjoppen@acc.umu.se>
Sent: Friday, September 7, 2018 1:54am
To: "FFmpeg development discussions and patches" <ffmpeg-devel@ffmpeg.org>
Cc: "Jason Stevens" <jay@wizardofthenet.com>
Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] libavformat/mxfenc: add missing dnxhr mxfcontainer essence ULs

fre 2018-09-07 klockan 01:03 -0700 skrev Jason Stevens:
> Add missing dnxhr mxf container essence ULs to the mxf encoder.
> set proper mxf frame size for dnxhr using libavformat/dnxhd's get dnxhr size function.
> 
> This fixes dnxhr mxf files being quarantined by Avid Media Composer.
> 
> > Signed-off-by: Jason Stevens <jay@wizardofthenet.com>
> ---
>  libavformat/mxfenc.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 52 insertions(+), 1 deletion(-)
> 
> @@ -1959,7 +1989,11 @@ AVPacket *pkt)
>      header_cid = pkt->data + 0x28;
>      cid = header_cid[0] << 24 | header_cid[1] << 16 | header_cid[2] << 8 | header_cid[3];
>  
> -    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) < 0)
> +    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) == DNXHD_VARIABLE) {
> +        frame_size = avpriv_dnxhd_get_hr_frame_size(cid, st->codecpar->width, st->codecpar->height);
> +    }
> +
> +    if (frame_size < 0)
>          return -1;
>      if ((sc->interlaced = avpriv_dnxhd_get_interlaced(cid)) < 0)
>          return AVERROR_INVALIDDATA;
> @@ -1998,6 +2032,23 @@ AVPacket *pkt)
>      case 1253:
>          sc->index = INDEX_DNXHD_720p_8bit_LOW;
>          break;
> +    case 1274:
> +        sc->index = INDEX_DNXHR_LB;
> +        break;
> +    case 1273:
> +        sc->index = INDEX_DNXHR_SQ;
> +        break;
> +    case 1272:
> +        sc->index = INDEX_DNXHR_HQ;
> +        break;
> +    case 1271:
> +        sc->index = INDEX_DNXHR_HQX;
> +        sc->component_depth = 10;
> +        break;
> +    case 1270:
> +        sc->index = INDEX_DNXHR_444;
> +        sc->component_depth = 10;
> +        break;

Is there some spec one can reference to for these magic constants?

Perhaps this has been mentioned already, but mxf_parse_dnxhd_frame()
seems like something that could be useful for other muxers, assuming
DNxHR can be muxed into anything beyond MXF

/Tomas
Carl Eugen Hoyos Sept. 7, 2018, 2:40 p.m. UTC | #3 
2018-09-07 10:54 GMT+02:00, Tomas Härdin <tjoppen@acc.umu.se>:
> fre 2018-09-07 klockan 01:03 -0700 skrev Jason Stevens:
>> Add missing dnxhr mxf container essence ULs to the mxf encoder.
>> set proper mxf frame size for dnxhr using libavformat/dnxhd's get dnxhr
>> size function.
>>
>> This fixes dnxhr mxf files being quarantined by Avid Media Composer.
>>
>> > Signed-off-by: Jason Stevens <jay@wizardofthenet.com>
>> ---
>>  libavformat/mxfenc.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 52 insertions(+), 1 deletion(-)
>>
>> @@ -1959,7 +1989,11 @@ AVPacket *pkt)
>>      header_cid = pkt->data + 0x28;
>>      cid = header_cid[0] << 24 | header_cid[1] << 16 | header_cid[2] << 8
>> | header_cid[3];
>>
>> -    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) < 0)
>> +    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) ==
>> DNXHD_VARIABLE) {
>> +        frame_size = avpriv_dnxhd_get_hr_frame_size(cid,
>> st->codecpar->width, st->codecpar->height);
>> +    }
>> +
>> +    if (frame_size < 0)
>>          return -1;
>>      if ((sc->interlaced = avpriv_dnxhd_get_interlaced(cid)) < 0)
>>          return AVERROR_INVALIDDATA;
>> @@ -1998,6 +2032,23 @@ AVPacket *pkt)
>>      case 1253:
>>          sc->index = INDEX_DNXHD_720p_8bit_LOW;
>>          break;
>> +    case 1274:
>> +        sc->index = INDEX_DNXHR_LB;
>> +        break;
>> +    case 1273:
>> +        sc->index = INDEX_DNXHR_SQ;
>> +        break;
>> +    case 1272:
>> +        sc->index = INDEX_DNXHR_HQ;
>> +        break;
>> +    case 1271:
>> +        sc->index = INDEX_DNXHR_HQX;
>> +        sc->component_depth = 10;
>> +        break;
>> +    case 1270:
>> +        sc->index = INDEX_DNXHR_444;
>> +        sc->component_depth = 10;
>> +        break;
>
> Is there some spec one can reference to for these magic constants?

Which magic constants?

Carl Eugen
Tomas Härdin Sept. 8, 2018, 2:43 p.m. UTC | #4 
fre 2018-09-07 klockan 16:40 +0200 skrev Carl Eugen Hoyos:
> > 2018-09-07 10:54 GMT+02:00, Tomas Härdin <tjoppen@acc.umu.se>:
> > fre 2018-09-07 klockan 01:03 -0700 skrev Jason Stevens:
> > > Add missing dnxhr mxf container essence ULs to the mxf encoder.
> > > set proper mxf frame size for dnxhr using libavformat/dnxhd's get dnxhr
> > > size function.
> > > 
> > > This fixes dnxhr mxf files being quarantined by Avid Media Composer.
> > > 
> > > > Signed-off-by: Jason Stevens <jay@wizardofthenet.com>
> > > 
> > > ---
> > >  libavformat/mxfenc.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 52 insertions(+), 1 deletion(-)
> > > 
> > > @@ -1959,7 +1989,11 @@ AVPacket *pkt)
> > >      header_cid = pkt->data + 0x28;
> > >      cid = header_cid[0] << 24 | header_cid[1] << 16 | header_cid[2] << 8
> > > > header_cid[3];
> > > 
> > > -    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) < 0)
> > > +    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) ==
> > > DNXHD_VARIABLE) {
> > > +        frame_size = avpriv_dnxhd_get_hr_frame_size(cid,
> > > st->codecpar->width, st->codecpar->height);
> > > +    }
> > > +
> > > +    if (frame_size < 0)
> > >          return -1;
> > >      if ((sc->interlaced = avpriv_dnxhd_get_interlaced(cid)) < 0)
> > >          return AVERROR_INVALIDDATA;
> > > @@ -1998,6 +2032,23 @@ AVPacket *pkt)
> > >      case 1253:
> > >          sc->index = INDEX_DNXHD_720p_8bit_LOW;
> > >          break;
> > > +    case 1274:
> > > +        sc->index = INDEX_DNXHR_LB;
> > > +        break;
> > > +    case 1273:
> > > +        sc->index = INDEX_DNXHR_SQ;
> > > +        break;
> > > +    case 1272:
> > > +        sc->index = INDEX_DNXHR_HQ;
> > > +        break;
> > > +    case 1271:
> > > +        sc->index = INDEX_DNXHR_HQX;
> > > +        sc->component_depth = 10;
> > > +        break;
> > > +    case 1270:
> > > +        sc->index = INDEX_DNXHR_444;
> > > +        sc->component_depth = 10;
> > > +        break;
> > 
> > Is there some spec one can reference to for these magic constants?
> 
> Which magic constants?

The "case" values. Not that I'm suggesting adding #defines for them or
anything, just a comment for where one might look up potential future
values

/Tomas
Paul B Mahol Sept. 8, 2018, 3:42 p.m. UTC | #5 
On 9/8/18, Tomas Haerdin <tjoppen@acc.umu.se> wrote:
> fre 2018-09-07 klockan 16:40 +0200 skrev Carl Eugen Hoyos:
>> > 2018-09-07 10:54 GMT+02:00, Tomas Haerdin <tjoppen@acc.umu.se>:
>> > fre 2018-09-07 klockan 01:03 -0700 skrev Jason Stevens:
>> > > Add missing dnxhr mxf container essence ULs to the mxf encoder.
>> > > set proper mxf frame size for dnxhr using libavformat/dnxhd's get
>> > > dnxhr
>> > > size function.
>> > >
>> > > This fixes dnxhr mxf files being quarantined by Avid Media Composer.
>> > >
>> > > > Signed-off-by: Jason Stevens <jay@wizardofthenet.com>
>> > >
>> > > ---
>> > >  libavformat/mxfenc.c | 53
>> > > +++++++++++++++++++++++++++++++++++++++++++-
>> > >  1 file changed, 52 insertions(+), 1 deletion(-)
>> > >
>> > > @@ -1959,7 +1989,11 @@ AVPacket *pkt)
>> > >      header_cid = pkt->data + 0x28;
>> > >      cid = header_cid[0] << 24 | header_cid[1] << 16 | header_cid[2]
>> > > << 8
>> > > > header_cid[3];
>> > >
>> > > -    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) < 0)
>> > > +    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) ==
>> > > DNXHD_VARIABLE) {
>> > > +        frame_size = avpriv_dnxhd_get_hr_frame_size(cid,
>> > > st->codecpar->width, st->codecpar->height);
>> > > +    }
>> > > +
>> > > +    if (frame_size < 0)
>> > >          return -1;
>> > >      if ((sc->interlaced = avpriv_dnxhd_get_interlaced(cid)) < 0)
>> > >          return AVERROR_INVALIDDATA;
>> > > @@ -1998,6 +2032,23 @@ AVPacket *pkt)
>> > >      case 1253:
>> > >          sc->index = INDEX_DNXHD_720p_8bit_LOW;
>> > >          break;
>> > > +    case 1274:
>> > > +        sc->index = INDEX_DNXHR_LB;
>> > > +        break;
>> > > +    case 1273:
>> > > +        sc->index = INDEX_DNXHR_SQ;
>> > > +        break;
>> > > +    case 1272:
>> > > +        sc->index = INDEX_DNXHR_HQ;
>> > > +        break;
>> > > +    case 1271:
>> > > +        sc->index = INDEX_DNXHR_HQX;
>> > > +        sc->component_depth = 10;
>> > > +        break;
>> > > +    case 1270:
>> > > +        sc->index = INDEX_DNXHR_444;
>> > > +        sc->component_depth = 10;
>> > > +        break;
>> >
>> > Is there some spec one can reference to for these magic constants?
>>
>> Which magic constants?
>
> The "case" values. Not that I'm suggesting adding #defines for them or
> anything, just a comment for where one might look up potential future
> values

In specification.
diff mbox

Patch

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 7f629dbe53..96e8a859de 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -146,6 +146,11 @@  enum ULIndex {
     INDEX_DNXHD_720p_8bit_HIGH,
     INDEX_DNXHD_720p_8bit_MEDIUM,
     INDEX_DNXHD_720p_8bit_LOW,
+    INDEX_DNXHR_LB,
+    INDEX_DNXHR_SQ,
+    INDEX_DNXHR_HQ,
+    INDEX_DNXHR_HQX,
+    INDEX_DNXHR_444,
     INDEX_JPEG2000,
     INDEX_H264,
 };
@@ -345,6 +350,31 @@  static const MXFContainerEssenceEntry mxf_essence_container_uls[] = {
       { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x05,0x00 },
       { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x13,0x00,0x00 },
       mxf_write_cdci_desc },
+    // DNxHR LB - CID 1274
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x05,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x28,0x00,0x00 },
+      mxf_write_cdci_desc },
+    // DNxHR SQ - CID 1273
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x05,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x27,0x00,0x00 },
+      mxf_write_cdci_desc },
+    // DNxHR HQ - CID 1272
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x05,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x26,0x00,0x00 },
+      mxf_write_cdci_desc },
+    // DNxHR HQX - CID 1271
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x05,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x25,0x00,0x00 },
+      mxf_write_cdci_desc },
+    // DNxHR 444 - CID 1270
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x05,0x00 },
+      { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x24,0x00,0x00 },
+      mxf_write_cdci_desc },
     // JPEG2000
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c,0x01,0x00 },
       { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x08,0x00 },
@@ -1959,7 +1989,11 @@  AVPacket *pkt)
     header_cid = pkt->data + 0x28;
     cid = header_cid[0] << 24 | header_cid[1] << 16 | header_cid[2] << 8 | header_cid[3];
 
-    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) < 0)
+    if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) == DNXHD_VARIABLE) {
+        frame_size = avpriv_dnxhd_get_hr_frame_size(cid, st->codecpar->width, st->codecpar->height);
+    }
+
+    if (frame_size < 0)
         return -1;
     if ((sc->interlaced = avpriv_dnxhd_get_interlaced(cid)) < 0)
         return AVERROR_INVALIDDATA;
@@ -1998,6 +2032,23 @@  AVPacket *pkt)
     case 1253:
         sc->index = INDEX_DNXHD_720p_8bit_LOW;
         break;
+    case 1274:
+        sc->index = INDEX_DNXHR_LB;
+        break;
+    case 1273:
+        sc->index = INDEX_DNXHR_SQ;
+        break;
+    case 1272:
+        sc->index = INDEX_DNXHR_HQ;
+        break;
+    case 1271:
+        sc->index = INDEX_DNXHR_HQX;
+        sc->component_depth = 10;
+        break;
+    case 1270:
+        sc->index = INDEX_DNXHR_444;
+        sc->component_depth = 10;
+        break;
     default:
         return -1;
     }