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

[Frederic YHUEL]

On Wed, May 23, 2012 at 3:43 PM, Jean-Baptiste Kempf <jb at videolan.org> wrote:
> 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?
>

Because with Smooth Streaming (and probably with DASH un-multiplexed
content), adaption is done for each track separately.

>> +    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?
>

Indeed the b_dash one could be removed :-)
(or the b_smooth 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?
>

No, it is not indeed, I did like that to get rid of a problem, but
this is not very clean...

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

FASTSEEK is not possible with DASH or Smooth Streaming, and I don't
think it is mandatory for a MP4 file, right?

>> -    /* 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.
>

Ok

>> +    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?
>

Maybe :-)

>> +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 ?
>

Oops

>> +    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.
>

Ok

>> +    /* temporary hack for DASH */
>
> Why ?
>
> You need more comments in those difficult functions, IMHO
>

Ok I will re-submit soon!


Best regards
Frédéric