[Ffmpeg-devel] Re: [PATCH] MXF unplayable file

[Baptiste Coudurier]

Hi

Reimar D?ffinger wrote:
> Hello,
> On Tue, Aug 01, 2006 at 11:11:09PM +0200, Baptiste Coudurier wrote:
>>> [...]
>>> @@ -695,8 +699,15 @@
>>>      { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x01,0x00 },        CODEC_ID_AC3 },
>>>      { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x05,0x00 },        CODEC_ID_MP2 }, /* MP2 or MP3 */
>>>    //{ { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02,0x1C,0x00 },    CODEC_ID_DOLBY_E }, /* Dolby-E */
>>> +    { { 0 }, CODEC_ID_NONE }
>>>  };
>> Right, UL value needs to be 16 bytes of 0x00 though to avoid problems
>> with memcmp I think.
> 
> That was a separate issue actually I must admit. You're doing
>> while (uls->id != CODEC_ID_NONE)
> 
> in mxf_get_codec_id, which with the current struct will read beyond its
> end I think.
> Also, since the check is against the codec id, the value if UL is
> irrelevant. Also the compiler must fill any not-specified parts with 0,
> so the only reason to write it out are those annoying warnings gcc 4.1
> spits out otherwise.

That may be right, I don't know if C specifies that.
Anyway it is safer to fill the key, MXFCodecUL is defined as UID which
is an array of 16 bytes. I'll fix it, thanks for notifying it.

> Oh, and the problem with the too-short tag is "only" a failed metadata
> parsing - though it results in the file not being playable.
> Btw. is there some official "tester" to "proof" this file is invalid?

File is not valid. IMHO, no reason to play it. MXF is already a bloated
mess and I see no reason to polute the code to handle non valid files,
or if we really want to, isolate the code and isolate broken files as
soon as possible.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312