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

[Keary Griffin]

Ilkka Ollakka wrote:
> On Tue, Mar 30, 2010 at 11:00:32PM -0400, Keary Griffin wrote:
>
> Hi,
>
> few small marks, as I didn't spot anyone else yet mentioning them.
>
>   
>> +static ssize_t Read ( sout_access_out_t *, block_t * );
>>     
>
>   
>> +    p_access->pf_read  = Read;
>>     
>
> No need to define Read at all, as core checks if it's defined before
> using.
>
>   
OK
>> +#ifdef WIN32
>> +static int convert_path ( const char *restrict path, wchar_t *restrict wpath )
>> +static int win32_replace_rename ( const char *oldpath, const char *newpath )
>> +static int win32_retry_rename( const char *oldpath, const char *newpath )
>>     
>
> Maybe these should be in core instead access_output ? seems kinda
> generic functions. Didn't check if courmisch allready merged them in.
>
>   
I'll let other people chime in on these, as I'm not sure what is 
recommended for core vs modules, but here are my thoughts:
 
I think the way to do this would be to just merge the 
win32_replace_rename into vlc_rename (this is how I had it originally, 
albeit it within a private function in the livehttp module itself.  I 
was asked to split it so I could use core vlc_rename for non-win32).  
The main reason I didn't want to change vlc_rename in core was:
1)  I'm not familiar enough with windows to know whether or not the 
change would always be the right thing to do.
2)  With the change errno is not set correctly for renames under win32.  
(Although this could probably be fixed by writing some function to 
examine the win32 error codes and setting errno appropriately)

Is there an equivalent to _wrename under win32 that allows replacing 
existing files and sets errno correctly?

As for win32_retry_rename, it seems like such a hack I'm not sure if it 
should be moved to core or not...
>> +    if ( p_sys->i_numsegs == 0 )
>> +        i_firstseg = 1;
>> +    else
>> +        i_firstseg = ( p_sys->i_segment >= p_sys->i_numsegs ) ? ( ( p_sys->i_segment - p_sys->i_numsegs ) + 1 ) : 1;
>>     
>
> Wouldn't this be simpler if you just extend first if to check if
> i_segments < i_umsegs too?
>
>   
Yes it would ;-)
> ------------------------------------------------------------------------
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>   
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100510/8a0bbb7b/attachment.html>