[vlc-devel] [PATCH 3/3] Added livehttp access_out module to support HTTP Live Streaming

[Keary Griffin]

Thanks--  Quick questions below.

Rémi Denis-Courmont wrote:
> On Wednesday 31 March 2010 02:48:43 Keary Griffin, you wrote:
>   
>> ---
>>  modules/access_output/Modules.am |    2 +
>>  modules/access_output/livehttp.c |  563
>>  ++++++++++++++++++++++++++++++++++++++ 2 files changed, 565 insertions(+),
>>  0 deletions(-)
>>  create mode 100644 modules/access_output/livehttp.c
>>
>>     
>
> +static const char *psz_numberSignSet = "#";
>
> I am not sure why to give a name to this string. But given how shared objects 
> work, this would spare a global pointer and a relocation:
>
>  static const char psz_numberSignSet[] = "#";
>   
If I replace the static const char *psz_numberSignSet with
#define SEG_NUMBER_PLACEHOLDER    "#"

and simply replace the occurrences of psz_numberSignSet with 
SEG_NUMBER_PLACEHOLDER would that work (and be better?)
> +        char *psz_idxTmp = malloc( strlen( p_sys->psz_indexPath ) + strlen( 
> TMP_IDX_SUFFIX ) + 1 );
> +        if ( !psz_idxTmp )
> +            return -1;
> +        strcpy( psz_idxTmp, p_sys->psz_indexPath );
> +        strcpy( psz_idxTmp + strlen( psz_idxTmp ), TMP_IDX_SUFFIX );
>
> That's OK. But asprintf() would be more concise.
>   
Your right, will replace.
> +        fp = vlc_fopen( psz_idxTmp, "wt");
> +        if ( !fp )
> +        {
> +            msg_Err( p_access, "cannot open index file `%s'", psz_idxTmp );
> +            free( psz_idxTmp );
> +            return -1;
> +        }
> +       
> +        if ( fprintf( fp, "#EXTM3U\n#EXT-X-TARGETDURATION:%d\n#EXT-X-MEDIA-
> SEQUENCE:%"PRIu32"\n", (int)p_sys->i_seglen, i_firstseg ) < 0 )
>
> size_t is formatted with %zu. You shouldn't need to cast to (int).
>   
Will do-- Spent a good bit of time googling to try and figure out the 
right way to do this, but wasn't sure what the correct portable solution 
was and decided to be safe.
> +        {
> +            free( psz_idxTmp );
> +            fclose( fp );
> +            return -1;
> +        }
> +       
> +        char *psz_idxFormat = p_sys->psz_indexUrl ? p_sys->psz_indexUrl : 
> p_access->psz_path;
> +        for ( uint32_t i = i_firstseg; i <= p_sys->i_segment; i++ )
> +        {
> +            char *psz_name;
> +            if ( ! ( psz_name = formatSegmentPath( p_access, psz_idxFormat, 
> i, false ) ) )
> +            {
> +                free( psz_idxTmp );
> +                fclose( fp );
> +                return -1;
> +            }
> +            val = fprintf( fp, "#EXTINF:%d\n%s\n", (int)p_sys->i_seglen, 
> psz_name );
>
> Same as above.
>
> +            free( psz_name );
> +            if ( val < 0 ) 
> +            {
> +                free( psz_idxTmp );
> +                fclose( fp );
> +                return -1;
> +            }
> +        }
> +
> +        if ( b_isend )
> +        {
> +            if ( fprintf ( fp, STR_ENDLIST ) < 0)
>
> Please use fputs() -or fprintf("%s")- to print a plain string.
>
> Got it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100331/6ea4015e/attachment.html>