[vlc-devel] [PATCH] Support for Http Dynamic Streaming

[Jean-Baptiste Kempf]

Thanks a lot for the update.

On 13 Jun, Jonathan Thambidurai wrote :
> +static chunk_t* chunk_new()
> +{
> +    chunk_t* chunk = malloc(sizeof(chunk_t));
> +    memset( chunk, 0, sizeof(chunk_t) );
> +    return chunk;

You should use calloc for this. And the result is almost an alias to
calloc.

Moreover, you MUST check all return instances of chunk_new()

> +            if( ! data_p )
> +            {
> +                msg_Err( p_this, "Couldn't find server entry" );
> +		return;

Careful about tabs! (and alignment in structs)

> +/* this only works with ANSI characters - this is ok
> +   for the bootstrapinfo field which this function is
> +   exclusively used for since it is merely a base64 encoding
> +*/
> +static bool is_whitespace( char c )
> +{
> +    return ( ' '  == c ||
> +             '\t' == c ||
> +             '\n' == c ||
> +             '\v' == c ||
> +             '\f' == c ||
> +             '\r' == c );
> +}

How is that different from isspace(c) ?

> +    msg_Info(s, "Downloading fragment %s",
> +             fragment_url );

You sometimes cut too much your lines.

> +    if( size > 50*1024*1024 )

Where does this magic number come from? If it's a value that you picked
up, use a define.

> +        mwait( last_dl_start_time + ( ((int64_t)hds_stream->fragment_runs[hds_stream->fragment_run_count-1].fragment_duration) * 1000000LL) / ((int64_t)hds_stream->afrt_timescale) );

No other way than using mwait?

> +#define MAX_BOOTSTRAP_INFO 10

Where does this number come from?

> +    bootstrap_info bootstraps[MAX_BOOTSTRAP_INFO];
> +    uint8_t bootstrap_idx = 0;
> +    memset( bootstraps, 0, sizeof(bootstrap_info) * MAX_BOOTSTRAP_INFO );
> +
> +#define MAX_MEDIA_ELEMENTS 10
idem?

> +    media_info medias[MAX_MEDIA_ELEMENTS];
> +    uint8_t media_idx = 0;
> +    memset( medias, 0, sizeof(media_info) * MAX_MEDIA_ELEMENTS );
> +
> +#define MAX_XML_DEPTH 256
idem?

The rest seems OK :)

With my kindest regards,

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