[FFmpeg-devel] [PATCH] Fix decoding of DNxHD video in MXF container

[Joseph Artsimovich]

On 01/02/2012 20:13, Tomas Härdin wrote:
> On Tue, 2012-01-31 at 13:22 +0000, Joseph Artsimovich wrote:
>> I found two problems with decoding of DNxHD encoded videos in MXF
>> container.
>>
>> 1. The DNxHD codec signature in libavformat/mxf.c is wrong.
>> @@ -41,7 +41,7 @@ const MXFCodecUL ff_mxf_codec_uls[] = {
>>       { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x01,0x02,0x00 }, 13,    CODEC_ID_DVVIDEO }, /* DV25 IEC PAL */
>>       { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x07,0x04,0x01,0x02,0x02,0x03,0x01,0x01,0x00 }, 14,   CODEC_ID_JPEG2000 }, /* JPEG2000 Codestream */
>>       { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x01,0x7F,0x00,0x00,0x00 }, 13,   CODEC_ID_RAWVIDEO }, /* Uncompressed */
>> -    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x03,0x02,0x00,0x00 }, 14,      CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD */
>> +    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x71,0x00,0x00,0x00 }, 13,      CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD */
> Why not simply add the new one instead of risking old files breaking?
> Do we know what sample prompted the old value?
My data comes from SMPTE 2019-4-2009 and is consistent with samples I've 
got. In fact, the byte that differs, that is 0x71 *is* what identifies 
VC-3 encoding. This change can't break anything, because DNxHD in MXF 
never worked in the first place, because of the other bug I am addressing.

>> 2. Frame height is calculated incorrectly for interlaced videos.  What's
>> stored in PictureEssenceDescriptor in MXF is not a frame height but
>> field height.
>> This is a general problem that should affect other codecs.  I checked
>> how D-10 (IMX) in MXF container ends up being handled correctly, and it
>> turned out it's because of stream parsing.
> Does the DNxHD essence format not store resolution or does the decoder
> simply not parse it?
> This is obviously an issue for raw interlaced video though.
DNxHD resolution can be derived from compression id, which is stored in 
the compressed stream. DNxHD stream parsing is not implemented in 
FFMpeg. Anyway, the bug is in MXF parsing and is not limited to DNxHD. I 
don't have access to the MXF spec, but I've got "The MXF book" that 
confirms the value FFMpeg is treating as frame height is actually a 
field height.

>
>> @@ -1391,7 +1395,9 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
>>               if (st->codec->codec_id == CODEC_ID_NONE)
>>                   st->codec->codec_id = container_ul->id;
>>               st->codec->width = descriptor->width;
>> -            st->codec->height = descriptor->height;
>> +            st->codec->height = descriptor->height; /* field height, not frame height */
>> +            if (descriptor->frame_layout == 1) /* interlaced */
>> +                st->codec->height *= 2;
> What about frame_layout == 2 (I forget its name)?
> The number 1 is magic - consider adding an enum for all FrameLayout
> values.
"The MXF book" doesn't list the possible values and I can't find where I 
got it from. If you point me to the right direction, I'll be happy to 
make it an enum.

>> The patches fixing the decoding problems are attached.  Note that I
>> didn't look at MXF encoding, which might or might not be affected by
>> problem 2.
> This would need lavc to flag interlacing somehow, which only happens per
> frame ATM (I think).
Maybe. We aren't using FFMpeg for encoding MXFs though, so that's not up 
to me to implement.

-- 
Joseph Artsimovich
Senior C++ Applications Developer
MirriAd Ltd