[vlc-devel] [PATCH] Add Smooth Streaming module

[Jean-Baptiste Kempf]

On Wed, Apr 11, 2012 at 11:51:36AM +0200, Frédéric Yhuel wrote :
> +SOURCES_stream_filter_smooth = smooth.c

mp4/mp4.h too, no?

> +#include <vlc_xml.h>
> +#include <vlc_charset.h>
> +#include <vlc_threads.h>
> +#include <vlc_arrays.h>
> +#include <vlc_stream.h>
> +#include <vlc_url.h>
> +#include <vlc_memory.h>

You need all of those?

> +    add_bool( "smooth-disable-audio", false, DA_TEXT, DA_LONGTEXT, false )
> +        change_safe()

Sorry, but why ?

> +enum FourCC {
> +    NULL_CC=0,
> +    H264 = VLC_FOURCC( 'H', '2', '6', '4' ),
> +    AVC1 = VLC_FOURCC( 'A', 'V', 'C', '1' ),
> +    AACL = VLC_FOURCC( 'A', 'A', 'C', 'L' ),
> +    AACH = VLC_FOURCC( 'A', 'A', 'C', 'H' ),
> +    WVC1 = VLC_FOURCC( 'W', 'V', 'C', '1' ),
> +    WMAP = VLC_FOURCC( 'W', 'M', 'A', 'P' ),
> +    TTML = VLC_FOURCC( 'T', 'T', 'M', 'L' ),
> +};

Why not using VLC_CODEC_ID ?

> +    unsigned long   SamplingRate;
> +    unsigned long   Channels;
> +    unsigned long   BitsPerSample;

Why long?

> +#define SMS_GET4BYTES( dst ) do { \
> +    dst = U32_AT( slice ); \
> +    slice += 4; \
> +  } while(0)
> +
> +#define SMS_GET1BYTE( dst ) do { \
> +    dst = *slice; \
> +    slice += 1; \
> +  } while(0)
> +
> +#define SMS_GET3BYTES( dst ) do { \
> +    dst = Get24bBE( slice ); \
> +    slice += 3; \
> +  } while(0)
> +
> +#define SMS_GET8BYTES( dst ) do { \
> +    dst = U64_AT( slice ); \
> +    slice += 8; \
> +  } while(0)
> +
> +#define SMS_GET4or8BYTES( dst ) \
> +    if( (version) == 0 ) \
> +        SMS_GET4BYTES( dst ); \
> +    else \
> +        SMS_GET8BYTES( dst ); \
> +
> +#define SMS_GETFOURCC( dst ) do { \
> +    memcpy( &dst, slice, 4 ); \
> +    slice += 4; \
> +  } while(0)

Really look like mp4 ones, no?

> +quality_level_t *get_qlevel( sms_stream_t *, uint16_t );
> +sms_stream_t    *get_sms_stream( stream_t *, uint16_t );
> +uint16_t        set_track_id( chunk_t *, uint16_t, uint16_t );
> +static int      sms_TrackCreate( stream_t *, uint16_t, mp4_track_t * );
> +static int      sms_FmtCreate( stream_t *, uint16_t, uint16_t, es_format_t * );
> +int             build_raw_avcC( uint8_t **, const char * );
> +int             build_raw_esds( uint8_t **, const char * );
> +static int      chunk_index_add( uint32_t *, int32_t );
> +static          chunk_t *get_chunk( stream_t *, uint32_t, bool );
> +static int      sms_Download( stream_t *, chunk_t *, char *);
> +static int      Download( stream_t *, sms_stream_t *, uint64_t *);
> +static void     *sms_Thread( void *);
> +static void     *sms_Reload( void *);

No way to reorder to get a bit less of those?

> +    if(  i_size < 1 )
Are you sure 1 is enough?

> +    if( ql == NULL )
unlikely


Shouldn't sms_Free/new, ql_Free/New be in another file or so?

> +    if( !isSmoothStreaming( s ) )
> +    {
> +        msg_Dbg(p_this, "Smooth Streaming: this is not a valid manifest");

Are you sure this cannot happen when opening other valid streams? If so,
remove this.

> +    if( p_sys == NULL )
> +        return VLC_ENOMEM;
unlikely

> +    /* remove the last part of the url */
> +    char *base_url = strdup( uri );
> +    char *pos = strrchr( base_url, '/');
> +    *pos = '\0';
> +    p_sys->base_url = base_url;
Don't we have functions for that?

> +    bool disable_audio = var_CreateGetBool( s, "smooth-disable-audio" );
inherit

> +    /* */
> +    s->pf_read = Read;
> +    s->pf_peek = Peek;
> +    s->pf_control = Control;
Move that down.

> +    /* Parse SMS ismc content. */
> +    if( parse_Manifest( s, false ) != VLC_SUCCESS )
> +        return VLC_EGENERIC;

No memleak here?

> +    /* Choose first audio stream available (TO FIX) */
FIXME

> +/******************************************************************************
> + *  Download thread
> + *****************************************************************************/

Split it in another file? Maybe?

> +    qual = strtok( url_template, "{" );
> +    strtok( NULL, "}" );
> +    frag = strtok( NULL, "{" );
> +    strtok( NULL, "}" );
> +    end = strtok( NULL, "" );
> +    char *url = NULL;

strtok? I think strtok_r

> +static int sms_FmtCreate( stream_t *s, uint16_t tid, uint16_t qid,

> +/* Luc Saillard code */

Seriously?

Best regards,

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