[vlc-devel] [PATCH 4/5] demux/mp4: Add fragmented MP4 support

[Jean-Baptiste Kempf]

On Tue, May 22, 2012 at 05:11:11PM +0200, Frédéric Yhuel wrote :
> +    bool b_adaption;

Why is it at the track level?

> +    bool         b_smooth;       /* Smooth Streaming => no moov box */
> +    bool         b_dash;         /* DASH */
> +    bool         b_fragmented;   /* fMP4 */

Do we need this here?
Especially the Dash one?

>      unsigned int i_index = 0;
> +    if( p_track->i_sample < chunk.i_sample_first )
> +    {
> +        msg_Err( p_demux, "tk->i_sample is %u and ck->i_sample_first is %u",
> +                p_track->i_sample, chunk.i_sample_first );
> +    }
> +    assert( p_track->i_sample >= chunk.i_sample_first );

Is this fragmented related?

> -    stream_Control( p_demux->s, STREAM_CAN_FASTSEEK, &b_seekable );
> +    stream_Control( p_demux->s, STREAM_CAN_SEEK, &b_seekable );

?

> -    /* Now load all boxes ( except raw data ) */
> -    if( ( p_sys->p_root = MP4_BoxGetRoot( p_demux->s ) ) == NULL )
> +    /* Is it smooth streaming or DASH ? */
> +    char *parent_name = NULL;
> +    if( p_demux->s->p_source && p_demux->s->p_source->p_module )
> +        parent_name = (char *)module_get_name( p_demux->s->p_source->p_module, false );
> +    if( parent_name && !strcmp( parent_name, "Smooth Streaming" ) )
>      {
> -        msg_Warn( p_demux, "MP4 plugin discarded (not a valid file)" );
> -        goto error;
> +        p_sys->b_smooth = true;
> +        p_sys->b_fragmented = true;
> +    }
> +    if( parent_name && !strcmp( parent_name, "DASH" ) )
> +    {
> +        p_sys->b_dash = true;
> +        p_sys->b_fragmented = true;
> +    }
> +    if( p_sys->b_fragmented )
> +        p_demux->pf_demux = DemuxFrg;
> +
> +    if( p_sys->b_dash )
> +    {
> +        /* Now load init segment */
> +        if( ( p_sys->p_root = MP4_BoxGetInitFrag( p_demux->s ) ) == NULL )
> +        {
> +            msg_Warn( p_demux,
> +                    "MP4 plugin discarded (not a valid initilization fragment)" );
> +            goto error;
> +        }
> +    }
> +    else if( p_sys->b_smooth )
> +    {
> +        /* Now load init segment */
> +        if( ( p_sys->p_root = MP4_BoxGetSmooBox( p_demux->s ) ) == NULL )
> +        {
> +            msg_Warn( p_demux,
> +                    "MP4 plugin discarded (not a valid 'smoo' box)" );
> +            goto error;
> +        }
> +        else
> +        {
> +            MP4_Box_t *p_smoo = MP4_BoxGet( p_sys->p_root, "uuid" );
> +            if( !p_smoo || CmpUUID( &p_smoo->i_uuid, &SmooBoxUUID ) )
> +                goto error;
> +            /* Get number of tracks */
> +            p_sys->i_tracks = 0;
> +            for( int i = 0; i < 3; i++ )
> +            {
> +                MP4_Box_t *p_stra = MP4_BoxGet( p_smoo, "uuid[%d]", i );
> +                if( p_stra && p_stra->data.p_stra->i_track_ID )
> +                    p_sys->i_tracks++;
> +                /* Get timescale of the video track; */
> +                if( i == 0 )
> +                    p_sys->i_timescale = p_stra->data.p_stra->i_timescale;
> +            }
> +
> +            goto allocate_memory;
> +        }
> +    }
> +    else
> +    {
> +        /* Now load all boxes ( except raw data ) */
> +        if( ( p_sys->p_root = MP4_BoxGetRoot( p_demux->s ) ) == NULL )
> +        {
> +            msg_Warn( p_demux, "MP4 plugin discarded (not a valid file)" );
> +            goto error;
> +        }
>      }

This part should be in another func, IMVHO.

> +    if( p_sys->b_fragmented )
> +    {
> +        mp4_track_t *p_track;
> +        for( uint16_t i = 0; i < p_sys->i_tracks; i++ )
> +        {
> +            p_track = &p_sys->track[i];
> +            p_track->cchunk = calloc( 1, sizeof( mp4_chunk_t ) );

No mem issue?

> +static mp4_track_t *fMP4_GetTrack( demux_t *p_demux, uint16_t tid )
> +{
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +
> +    mp4_track_t *ret = NULL;
> +    for( unsigned i = 0; i < p_sys->i_tracks; i++ )
> +    {
> +        ret = &p_sys->track[i];

Could it be NULL ?

> +    uint32_t i_track_ID = p_tfhd->data.p_tfhd->i_track_ID;
> +    *i_tk_id = i_track_ID;
> +    assert( i_track_ID > 0 );
> +    msg_Dbg( p_demux, "GetChunk: track ID is %"PRIu32"", i_track_ID );

Extra line please.

> +    /* temporary hack for DASH */

Why ?

You need more comments in those difficult functions, IMHO

Best regards,

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