[vlc-devel] [PATCH] demux/mp4: add parsing of mfra atom

[Frederic YHUEL]

Hi j-b, thanks for your review!

On Wed, Nov 30, 2011 at 1:03 AM, Jean-Baptiste Kempf <jb at videolan.org> wrote:
> On Tue, Nov 29, 2011 at 05:14:41PM +0100, fyhuel at viotech.net wrote :
>> +static int MP4_ReadBox_mfro( stream_t *p_stream, MP4_Box_t *p_box )
> Ok.
>
>> +static int MP4_ReadBox_tfra( stream_t *p_stream, MP4_Box_t *p_box )
>
>> +    uint8_t length_size_of_traf_num = p_tfra->i_lengths>>4;
>> +    uint8_t length_size_of_trun_num = ( p_tfra->i_lengths & 0x0c )>>2;
>
> spaces around ">>", please.
>

Ooops... sorry!

>> +    for( uint32_t i=0; i < i_number_of_entries; i++ )
>
> Spec says i = 1, why?
>

Because C arrays are zero-indexed :-)
Besides, Laurent Aimar did like that elsewhere in the code, see "elst"
and "stsz" atoms for examples.

I'll send a new patch.

Maybe I should store length_size_of_[traf, trun, sample]_num in the
traf struct, so that we will not have to do the computation again.

And maybe I should wait a little before submitting a new patch, to be
sure to not forget to think about something like that :-)

-- 
Frédéric