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

[Jean-Baptiste Kempf]

On 22 May, Jonathan Thambidurai wrote :
>  modules/stream_filter/Makefile.am |    8 +
>  modules/stream_filter/hds/hds.c   | 1531 +++++++++++++++++++++++++++++++++++++
>  modules/stream_filter/hds/hds.h   |  132 ++++
>  3 files changed, 1671 insertions(+)
>  create mode 100644 modules/stream_filter/hds/hds.c
>  create mode 100644 modules/stream_filter/hds/hds.h

Missing NEWS, POTFILES.in and MODULES_LIST entry.

> +libhds_plugin_la_SOURCES = \
> +    stream_filter/hds/hds.c \
> +    stream_filter/hds/hds.h

Why do you need 2 files?

> +/*****************************************************************************
> + * hds.c: F4M stream filter

Better names, please.

> +static char* parse_asrt( vlc_object_t* p_this,
> +			hds_stream_t* s,
> +			char* data,
> +			char* data_end )

No tabs in source code, please.
No trailing spaces, too.

> +
> +typedef struct chunk_s
> +{
> +    int64_t     duration;   /* chunk duration in afrt timescale units */
> +    int64_t     timestamp;
> +    int32_t     frag_num;
> +    uint32_t    seg_num;
> +
> +    /* Used to speed things up in vod situations */
> +    uint32_t frun_entry;
> +
> +    uint32_t    mdat_pos;   /* position in the mdat */
> +    uint32_t    mdat_len;
> +    uint8_t     *mdat_data;
> +
> +    uint8_t     *data;
> +    uint32_t    data_len;
> +
> +    void    *next;
> +
> +    bool failed;
> +
> +    bool eof;
> +} chunk_t;

You should repack this. Probably by putting data+data_len above
frun_entry.

> +#define MAX_HDS_SERVERS 10
> +typedef struct hds_stream_s
> +{
> +    /* linked-list of chunks */
> +    volatile chunk_t        *chunks_head;
> +    volatile chunk_t        *chunks_livereadpos;
> +    volatile chunk_t        *chunks_downloadpos;
> +
> +    char*          quality_segment_modifier;
> +
> +    /* we can make this configurable */
> +    uint64_t       download_leadtime;
> +
> +    /* in timescale units */
> +    uint32_t       total_duration;
> +
> +    uint32_t       afrt_timescale;
> +
> +    /* these two values come from the abst */
> +    uint32_t       timescale;
> +    uint64_t       live_current_time;
> +
> +    vlc_mutex_t  abst_lock;
> +
> +    vlc_mutex_t  dl_lock;
> +    vlc_cond_t   dl_cond;
> +
> +    vlc_mutex_t  rd_lock;
> +    vlc_cond_t  rd_cond;
> +
> +    /* can be left as null */
> +    char* abst_url;
> +
> +    /* this comes from the manifest media section  */
> +    char*          url;
> +
> +    /* this comes from the bootstrap info */
> +    char*          movie_id;
> +
> +    char*          server_entries[MAX_HDS_SERVERS];
> +    uint8_t        server_entry_count;
> +
> +#define MAX_HDS_SEGMENT_RUNS 256
> +    segment_run_t  segment_runs[MAX_HDS_SEGMENT_RUNS];
> +    uint8_t        segment_run_count;
> +
> +#define MAX_HDS_FRAGMENT_RUNS 10000
> +    fragment_run_t fragment_runs[MAX_HDS_FRAGMENT_RUNS];
> +    uint32_t        fragment_run_count;
> +} hds_stream_t;

Why some defines above and some inside?
Are you sure it's wise to use a fragment_run_t[10000] same for
segment_run_t[256] ?

> +struct stream_sys_t
> +{
> +    char         *base_url;    /* URL common part for chunks */
> +    vlc_thread_t thread;       /* chunk download thread */
> +    bool         closed;
> +
> +    /* we pend on peek until some number of segments arrives; otherwise
> +     * the downstream system dies in case of playback */
> +    volatile uint64_t         chunk_count; 
> +
> +    uint32_t     flv_header_bytes_sent;
> +
> +    vlc_array_t  *hds_streams; /* available streams */
> +
> +    uint32_t duration_seconds;
> +    bool live;
> +};

Please repack this too. (closed at the end)?

With my kindest regards,

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