[vlc-devel] [PATCH 1/2] DASH: implement Seek()

[Frederic YHUEL]

On Mon, Mar 26, 2012 at 1:25 PM, Hugo Beauzée-Luyssen
<beauze.h at gmail.com> wrote:
> Hello,
>
> 2012/3/26 Frédéric Yhuel <fyhuel at viotech.net>:
>> This is needed for DASH module to work with (not yet ready) VLC MP4
>> demux. Forward seeking is implemented by calling Read() with a NULL
>> argument (= skipping), while backward seeking is limited to the current
>> block in the block_bytestream_t upon which is built the internal buffer.
>> This limitation is not really a problem because backward seeking will be
>> needed only for the parsing of the initialization fragment.
>> ---
>>  modules/stream_filter/dash/DASHManager.cpp        |    5 ++
>>  modules/stream_filter/dash/DASHManager.h          |    1 +
>>  modules/stream_filter/dash/buffer/BlockBuffer.cpp |   16 ++++++++
>>  modules/stream_filter/dash/buffer/BlockBuffer.h   |    1 +
>>  modules/stream_filter/dash/dash.cpp               |   43 ++++++++++++++++++++-
>>  5 files changed, 64 insertions(+), 2 deletions(-)
>>
>> diff --git a/modules/stream_filter/dash/DASHManager.cpp b/modules/stream_filter/dash/DASHManager.cpp
>> index f649015..8483c9d 100644
>> --- a/modules/stream_filter/dash/DASHManager.cpp
>> +++ b/modules/stream_filter/dash/DASHManager.cpp
>> @@ -74,6 +74,11 @@ int     DASHManager::read( void *p_buffer, size_t len )
>>     return this->buffer->get(p_buffer, len);
>>  }
>>
>> +int     DASHManager::seekBackwards( unsigned i_len )
> Defined with unsigned ...
>

Argh sorry.

>> +{
>> +    return this->buffer->seekBackwards( i_len );
>> +}
>> +
>>  int     DASHManager::peek( const uint8_t **pp_peek, size_t i_peek )
>>  {
>>     return this->buffer->peek(pp_peek, i_peek);
>> diff --git a/modules/stream_filter/dash/DASHManager.h b/modules/stream_filter/dash/DASHManager.h
>> index a09e98e..d46d59b 100644
>> --- a/modules/stream_filter/dash/DASHManager.h
>> +++ b/modules/stream_filter/dash/DASHManager.h
>> @@ -47,6 +47,7 @@ namespace dash
>>             bool    start   ();
>>             int     read    ( void *p_buffer, size_t len );
>>             int     peek    ( const uint8_t **pp_peek, size_t i_peek );
>> +            int     seekBackwards ( size_t len );
> ... Declared with size_t. It breaks the build here.

Sorry again. That being said, if I understand why I don't get a
warning for the previous mistake, I don't understand why I don't get
one here... ok size_t == unsigned int on my machine, but still...

>>
>>             const mpd::IMPDManager*         getMpdManager   () const;
>>             const logic::IAdaptationLogic*  getAdaptionLogic() const;
>> diff --git a/modules/stream_filter/dash/buffer/BlockBuffer.cpp b/modules/stream_filter/dash/buffer/BlockBuffer.cpp
>> index 056c481..1db9168 100644
>> --- a/modules/stream_filter/dash/buffer/BlockBuffer.cpp
>> +++ b/modules/stream_filter/dash/buffer/BlockBuffer.cpp
>> @@ -85,6 +85,22 @@ int     BlockBuffer::peek                 (const uint8_t **pp_peek, unsigned int
>>     vlc_mutex_unlock(&this->monitorMutex);
>>     return ret;
>>  }
>> +
>> +int     BlockBuffer::seekBackwards       (unsigned len)
>> +{
>> +    vlc_mutex_lock(&this->monitorMutex);
>> +    if( this->buffer.i_offset > len )
>> +    {
>> +        this->buffer.i_offset -= len;
>> +        this->sizeBytes += len;
>> +        vlc_mutex_unlock(&this->monitorMutex);
>> +        return VLC_SUCCESS;
>> +    }
>> +
>> +    vlc_mutex_unlock(&this->monitorMutex);
>> +    return VLC_EGENERIC;
>> +}
>> +
>>  int     BlockBuffer::get                  (void *p_data, unsigned int len)
>>  {
>>     vlc_mutex_lock(&this->monitorMutex);
>> diff --git a/modules/stream_filter/dash/buffer/BlockBuffer.h b/modules/stream_filter/dash/buffer/BlockBuffer.h
>> index 570861f..2c39db1 100644
>> --- a/modules/stream_filter/dash/buffer/BlockBuffer.h
>> +++ b/modules/stream_filter/dash/buffer/BlockBuffer.h
>> @@ -49,6 +49,7 @@ namespace dash
>>                 void    put         (block_t *block);
>>                 int     get         (void *p_data, unsigned int len);
>>                 int     peek        (const uint8_t **pp_peek, unsigned int i_peek);
>> +                int     seekBackwards (unsigned len);
>>                 void    setEOF      (bool value);
>>                 bool    getEOF      ();
>>                 mtime_t size        ();
>> diff --git a/modules/stream_filter/dash/dash.cpp b/modules/stream_filter/dash/dash.cpp
>> index b503f27..30113a4 100644
>> --- a/modules/stream_filter/dash/dash.cpp
>> +++ b/modules/stream_filter/dash/dash.cpp
>> @@ -77,7 +77,7 @@ struct stream_sys_t
>>         dash::DASHManager                   *p_dashManager;
>>         dash::http::HTTPConnectionManager   *p_conManager;
>>         dash::mpd::MPD                      *p_mpd;
>> -        int                                 position;
>> +        uint64_t                            position;
>>         bool                                isLive;
>>  };
>>
>> @@ -160,6 +160,36 @@ static void Close(vlc_object_t *p_obj)
>>  /*****************************************************************************
>>  * Callbacks:
>>  *****************************************************************************/
>> +static int  Seek            ( stream_t *p_stream, uint64_t pos )
>> +{
>> +    stream_sys_t        *p_sys          = (stream_sys_t *) p_stream->p_sys;
>> +    dash::DASHManager   *p_dashManager  = p_sys->p_dashManager;
>> +    int                 i_ret           = 0;
>> +    unsigned            i_len           = 0;
>> +    long                i_read          = 0;
>> +
>> +    if( pos < p_sys->position )
>> +    {
>> +        i_len = p_sys->position - pos;
> You should probably check for overflow here.
>

Ok

>> +        i_ret = p_dashManager->seekBackwards( i_len );
>> +        if( i_ret == VLC_EGENERIC )
>> +        {
>> +            msg_Err( p_stream, "Cannot seek backward outside the current block :-/" );
>> +            return VLC_EGENERIC;
>> +        }
>> +        else
>> +            return VLC_SUCCESS;
>> +    }
>> +
>> +    /* Seek forward */
>> +    i_len = pos - p_sys->position;
> This could also overflow. I'm not familiar enough with
> block_bytestream_t, but using isn't using only one of those a problem
> if we need an offset that's > "size_t max" ?
>

No the "i_offset" attribute of block_bytestream_t is the offset
*within* the current block of the chain. But maybe I did not
understand properly your comment?

-- 
Frédéric