[vlc-devel] [PATCH] libmp4: add TfrfBox and TfxdBox parsing

[Frederic YHUEL]

On Fri, Mar 23, 2012 at 9:37 AM, Jean-Baptiste Kempf <jb at videolan.org> wrote:
> On Fri, Mar 23, 2012 at 09:20:47AM +0100, Frederic YHUEL wrote :
>> On Thu, Mar 22, 2012 at 6:23 PM, Jean-Baptiste Kempf <jb at videolan.org> wrote:
>> > On Thu, Mar 22, 2012 at 05:41:36PM +0100, Frédéric Yhuel wrote :
>> >> +static inline int CmpUUID( const UUID_t *u1, const UUID_t *u2 )
>> >> +{
>> >> +    return memcmp( u1, u2, 16 );
>> >> +}
>> >
>> > guidcmp, if it is compatible... ( UUID_t vs GUID ?)
>> >
>>
>> If I understand well a GUID is MS implementation of UUID, and there
>> are several versions... boring...
>> It's probably possible to cast a UUID_t into a GUID, or to replace
>> completely UUID_t structs by GUID ones, but I'm not sure that the
>> first solution is clean, and the second solution is not worthwhile for
>> so little code duplication IMVHO. Am I right?
>
> Drop it. Use your old version code.
>

Ok, I'm gonna send a new patch with also a slightly improved debug

>> >> +static int MP4_ReadBox_uuid( stream_t *p_stream, MP4_Box_t *p_box )
>> >> +{
>> >> +    if( !CmpUUID( &p_box->i_uuid, &TfrfBoxUUID ) )
>> >> +        return MP4_ReadBox_tfrf( p_stream, p_box );
>> >> +    if( !CmpUUID( &p_box->i_uuid, &TfxdBoxUUID ) )
>> >> +        return MP4_ReadBox_tfxd( p_stream, p_box );
>> >> +
>> >> +    msg_Warn( p_stream, "Unknown uuid type box" );
>> >> +    return 1;
>> > Why 1 ?
>> >
>>
>> Good question, it seems to me that it is the error code when one
>> stumbles accross a unknown box or parameter, see MP4_ReadBox_default()
>> for example. Is there some doc about that somewhere?
>
> Probably not. Add it somewhere in the file;
>

Well in fact:

0 -> error
1 -> success
2 -> ? see MP4_NextBox()

I guess it should be replaced by VLC_EGENERIC etc... in a separate patch.

for my MP4_ReadBox_uuid(), returning '1' makes sense, because then the
unknown box will be skipped, and hopefully it doesn't matter. There
will be warning though.

BR,

-- 
Frédéric