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

[Tomas Härdin]

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?

> 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.

> @@ -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 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).

/Tomas