[vlc-devel] [PATCH] demux/mp4: (asking for pre-review) add support of fMP4

[Jean-Baptiste Kempf]

On Wed, Jan 25, 2012 at 03:55:18PM +0100, Frédéric Yhuel wrote :
> This is not a patch to be merged, I'm just seeking advices before
> continuing further.
Ok.

> This also to answer a "pix or didn't happen" on irc ;-)
Who could have said that? :)

> @@ -311,6 +311,10 @@ static int MP4_ReadBoxContainerRaw( stream_t *p_stream, MP4_Box_t *p_container )
>          if( !p_container->p_first ) p_container->p_first = p_box;
>          else p_container->p_last->p_next = p_box;
>          p_container->p_last = p_box;
> +#ifdef fMP4
> +        if( p_box->i_type == ATOM_moov )
> +            break;
> +#endif
Why is that needed?

> +MP4_Box_t *MP4_BoxGetNextChunk( stream_t *s, unsigned i_tk_id )
> +{
> +    /* p_chunk is a virtual root container for the moof and mdat boxes */
> +    MP4_Box_t *p_chunk;
> +    MP4_Box_t *p_moof;
> +    MP4_Box_t *p_mdat;
> +
> +    p_chunk = malloc( sizeof( MP4_Box_t ) );
> +    p_mdat = malloc( sizeof( MP4_Box_t ) );
> +    if( unlikely( p_chunk == NULL || p_mdat == NULL ) )
> +        return NULL;
> +
> +    p_chunk->i_type = ATOM_root;
> +    p_chunk->i_shortsize = 1;
> +
> +    p_chunk->data.p_data = NULL;
> +    p_chunk->p_father    = NULL;
> +    p_chunk->p_first     = NULL;
> +    p_chunk->p_last      = NULL;
> +    p_chunk->p_next      = NULL;

Why not calloc p_chunk ?

> +
> +    p_moof = MP4_ReadBox( s, p_chunk );
> +    /* we need to know the size of the mdat */
> +    MP4_ReadBoxCommon( s, p_mdat );
> +
> +    if( p_moof )
> +    {
> +        p_chunk->p_first = p_moof;
> +        p_moof->p_next = p_mdat;
> +        p_chunk->p_last = p_mdat;
> +    }
> +    else
> +    {
> +        msg_Warn( s, "no moof box found!");
> +        return NULL;

leaking here, right?

> +    if( p_tfhd == NULL)
> +    {
> +        msg_Warn( s, "no tfhd box found!");
> +        return NULL;

or here?

> +    p_sys->pi_sample_first = calloc( p_sys->i_tracks, sizeof( uint32_t ) );
> +    p_sys->pi_first_dts = calloc( p_sys->i_tracks, sizeof( uint64_t ) );
callocing for uints?

> +    for( int i=1; i < 256; i++)
> +        p_sys->map[i] = -1;

Memset ?
  
Best regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device