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

[Frederic YHUEL]

Hi Ilkka,

Thanks for the comments!

On Mon, Mar 19, 2012 at 9:06 AM, Ilkka Ollakka <ileoo at videolan.org> wrote:
> On Fri, Mar 16, 2012 at 03:25:09PM +0100, Frédéric Yhuel wrote:
>> Not clean yet, I provide it for reviewers to test my fMP4 patch, and for
>> people interested in Smooth Streaming. Comments and patches welcome!
>
> Hi,
>
> Seems that VOD tries to download all the segments before it even starts
> to play anything. And actually for me that VOD-link didn't start playing
> even when it stopped loading segments.
>

Did you apply the MP4 patches and set "smooth-can-seek" option to 1?

>> ***** VOD *****
>> - http://demo.anevia.com:3130/html/player/smooth/vod/content1.ism/Manifest
>
>> +
>> +    /* remove the last part of the url */
>> +    char *inv = inverse_string( uri );
>> +    char *tmp = strtok( inv, "/" );
>> +    tmp = strtok( NULL, "" );
>> +    p_sys->base_url = inverse_string( tmp );
>
> strrchr and adding '\0' ?
>

Seems simpler indeed :-)

>> +        msg_Dbg( s, "pos is %"PRIu64", boffset is %"PRIu64"", pos, p_sys->playback.boffset );
>> +        assert( pos >= p_sys->playback.boffset );
>> +        int len = pos - p_sys->playback.boffset;
>> +        int skipped = Read( s, NULL, len);
>> +        if( skipped == len )
>> +            goto success;
>> +    }
>
> Couldn't you skip goto success just by placing that code before fail:
> and using break?
>

Indeed.

>> +
>> +fail:
>> +    msg_Warn( s, "segment_Seek failed, maybe not enough dld chunks");
>> +    return VLC_EGENERIC;
>> +
>> +success:
>> +    segment = get_segment( s, p_sys->playback.index, false );
>> +    if( segment == NULL )
>> +        return VLC_EGENERIC;
>> +    msg_Dbg( s, "Seek successful! We are now at segment %"PRIu32" quality \
>> +%"PRIu64", offset %"PRIu64" + %i", segment->sequence,
>> +segment->bandwidth, segment->offset, segment->read_pos );
>> +    msg_Info( s, "(segment size is %"PRIu64")", segment->size );
>> +    return VLC_SUCCESS;
>> +}
>
>> +static char *inverse_string( const char *str )
>> +{
>> +    size_t len = strlen( str );
>> +    char *ret = (char *)malloc( len+1 );
>> +    for( unsigned int i = 0; i < len; i++ )
>> +        ret[i] = str[len-i-1];
>> +    ret[len] = '\0';
>> +    return ret;
>> +}
>
> Don't think this function is actually needed
>

It is not.

Thanks again. I discovered other bugs / stupid code in the meantime, I
will send patches soon.

Best Regards,

-- 
Frédéric