[Ffmpeg-devel] Re: [PATCH] MXF demuxer improvements

[Baptiste Coudurier]

Hi Michael

Michael Niedermayer wrote:
> Hi
> 
> On Thu, Jul 27, 2006 at 10:54:16PM +0200, Baptiste Coudurier wrote:
> [...]
>> +            mxf->packages_refs = av_malloc(mxf->packages_count * sizeof(UID));
>> +            mxf->packages = av_mallocz(mxf->packages_count * sizeof(*mxf->packages));
>> +            url_fskip(pb, 4); /* useless size of objects, always 16 according to specs */
>> +            for (i = 0; i < mxf->packages_count; i++)
>> +                get_buffer(pb, mxf->packages_refs[i], 16);
> 
> why not get_buffer(pb, mxf->packages_refs, mxf->packages_count * sizeof(UID)); ?

Indeed. Good idea. Im learning day after day.

> [...]
> 
>> +static int mxf_read_metadata_track(MXFContext *mxf, KLVPacket *klv)
>> +{
> 
> this function seems to have been moved around which is a cosmetic change 
> and as such should be seperate from functional ones
> 
> [...]
> 
> ive no objections against the name changes, after all its your demuxer
> but please do them in a seperate commit/patch, that keeps svn-log more 
> readable (and patches easier to review)

Yes. I'll do seperate commits. I think Im bit afraid on working on my
own on svn atm hehe, it will change.

> [...]
>>  /* SMPTE RP224 http://www.smpte-ra.org/mdd/index.html */
>> -static const CodecTag mxf_sound_essence_labels[] = {
>> -    { CODEC_ID_PCM_S16LE, 0x01000000 },/* Uncompressed Sound Coding */
>> -    { CODEC_ID_PCM_S16LE, 0x017F0000 },/* Uncompressed Sound Coding */
>> -    { CODEC_ID_PCM_S16BE, 0x017E0000 },/* Uncompressed Sound Coding Big Endian*/
>> -    { CODEC_ID_PCM_ALAW,  0x02030101 },
>> -    { CODEC_ID_AC3,       0x02030201 },
>> -  //{ CODEC_ID_MP1,       0x02030104 },
>> -    { CODEC_ID_MP2,       0x02030105 },/* MP2 or MP3 */
>> -  //{ CODEC_ID_MP2,       0x02030106 },/* MPEG-2 Layer 1 */
>> -  //{ CODEC_ID_???,       0x0203010C },/* Dolby E */
>> -  //{ CODEC_ID_???,       0x02030301 },/* MPEG-2 AAC */
>> -    { 0, 0 },
>> -};
> 
> some of these codec ids seem to have dissapeared in the new code, i assume
> they never worked, or why where they removed?
> 

Well, finally I thought that poluting the demuxer with ids not used was
not a nice solution, I'll just add uls I encouter in files one after
one, is that solution worse ?

MXF is mainly a broadcast industry format and not many types will be
actually contained in it, even if SMPTE registers them, they got like 50
uls for MPEG-2 and 25 for DV, depending on flavors.

Thanks for reviewing.

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