[vlc-devel] [PATCH 06/17] dash: segment added byterange and tochunk

Hugo Beauzée-Luyssen beauze.h at gmail.com
Thu Feb 2 10:56:31 CET 2012


2012/1/30 Christopher Müller <christopher.mueller at itec.uni-klu.ac.at>:
>> Von: vlc-devel-bounces at videolan.org [mailto:vlc-devel-
>> bounces at videolan.org] Im Auftrag von Hugo Beauzée-Luyssen
>> Gesendet: Montag, 30. Jänner 2012 17:46
>> An: Mailing list for VLC media player developers
>> Betreff: Re: [vlc-devel] [PATCH 06/17] dash: segment added byterange
>> and tochunk
>>
>> On Mon, Jan 30, 2012 at 2:48 PM,  <Christopher at mailsrv.uni-klu.ac.at>
>> wrote:
>> > From: Christopher Mueller <christopher.mueller at itec.aau.at>
>> >
>> > ---
>> >  modules/stream_filter/dash/mpd/Segment.cpp |   71
>> > +++++++++++++++++++++++++--
>> >  modules/stream_filter/dash/mpd/Segment.h   |   20 +++++++-
>> >  2 files changed, 82 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/modules/stream_filter/dash/mpd/Segment.cpp
>> > b/modules/stream_filter/dash/mpd/Segment.cpp
>> > index 5ac7f03..a6ab2d8 100644
>> > --- a/modules/stream_filter/dash/mpd/Segment.cpp
>> > +++ b/modules/stream_filter/dash/mpd/Segment.cpp
>> > @@ -28,24 +28,83 @@
>> >  #include "Segment.h"
>> >
>> >  using namespace dash::mpd;
>> > +using namespace dash::http;
>> >
>> > -std::string Segment::getSourceUrl() const
>> > +Segment::Segment    () :
>> > +         startByte  (-1),
>> > +         endByte    (-1)
>> > +{
>> > +
>> > +}
>> > +
>> > +std::string             Segment::getSourceUrl   () const
>> >  {
>> >     return this->sourceUrl;
>> >  }
>> >
>> > -void        Segment::setSourceUrl( const std::string &url )
>> > +void                    Segment::setSourceUrl   ( const std::string
>> > +&url )
>> >  {
>> >     if ( url.empty() == false )
>> >         this->sourceUrl = url;
>> >  }
>> > -
>> > -bool        Segment::isSingleShot() const
>> > +bool                    Segment::isSingleShot   () const
>> >  {
>> >     return true;
>> >  }
>> > -
>> > -void Segment::done()
>> > +void                    Segment::done           ()
>> >  {
>> >     //Only used for a SegmentTemplate.
>> >  }
>> > +void                    Segment::addBaseUrl     (BaseUrl *url) {
>> > +    this->baseUrls.push_back(url);
>> > +}
>> > +std::vector<BaseUrl *>  Segment::getBaseUrls    () const {
>> > +    return this->baseUrls;
>> > +}
>>
>> Since this returns a non-temporary value, it would be better not to
>> copy it using a (const) reference.
>
> Agree!
>
>>
>> > +void                    Segment::setByteRange   (int start, int end)
>> > +{
>> > +    this->startByte = start;
>> > +    this->endByte   = end;
>> > +}
>> > +int                     Segment::getStartByte   () const {
>> > +    return this->startByte;
>> > +}
>> > +int                     Segment::getEndByte     () const {
>> > +    return this->endByte;
>> > +}
>> > +dash::http::Chunk*      Segment::toChunk        () {
>> > +    Chunk *chunk = new Chunk();
>> > +
>> > +    if(this->startByte != -1 && this->endByte != -1)
>> > +    {
>> > +        chunk->setStartByte(this->startByte);
>> > +        chunk->setEndByte(this->endByte);
>> > +    }
>>
>> Can't a startByte or an endByte be defined without the other?
>
> The standard refers to RFC 2616, Clause 14.35.1 but the current implementation needs both values.
>
>>
>> > +
>> > +    if(this->baseUrls.size() > 0)
>> > +    {
>> > +        std::stringstream ss;
>> > +        ss << this->baseUrls.at(0)->getUrl() << this->sourceUrl;
>> > +        chunk->setUrl(ss.str());
>> > +        ss.clear();
>> > +
>> > +        for(size_t i = 1; i < this->baseUrls.size(); i++)
>> > +        {
>> > +            ss << this->baseUrls.at(i)->getUrl() << this->sourceUrl;
>> > +            chunk->addOptionalUrl(ss.str());
>> > +            ss.clear();
>> > +        }
>>
>> Is baseURL guarantied to be '/' terminated?
>
> I am not sure about that but every example in the standard has a baseurl which is terminated by a '/'. Anyway if baseurl does not end with '/' sourceurl has to start with it otherwise the mpd would be incorrect.
>
>>
>> > +
>> > +    }
>> > +    else
>> > +    {
>> > +        chunk->setUrl(this->sourceUrl);
>> > +    }
>> > +
>> > +    return chunk;
>> > +}
>> > diff --git a/modules/stream_filter/dash/mpd/Segment.h
>> > b/modules/stream_filter/dash/mpd/Segment.h
>> > index 9c2dd7e..a3b09a1 100644
>> > --- a/modules/stream_filter/dash/mpd/Segment.h
>> > +++ b/modules/stream_filter/dash/mpd/Segment.h
>> > @@ -26,6 +26,10 @@
>> >  #define SEGMENT_H_
>> >
>> >  #include <string>
>> > +#include <sstream>
>> > +#include <vector>
>> > +#include "mpd/BaseUrl.h"
>> > +#include "http/Chunk.h"
>> >
>> >  namespace dash
>> >  {
>> > @@ -34,6 +38,7 @@ namespace dash
>> >         class Segment
>> >         {
>> >             public:
>> > +                Segment();
>> >                 virtual ~Segment(){}
>> >                 virtual std::string getSourceUrl() const;
>> >                 virtual void        setSourceUrl( const std::string
>> > &url ); @@ -42,11 +47,20 @@ namespace dash
>> >                  *          That is basically true when using an Url,
>> > and false
>> >                  *          when using an UrlTemplate
>> >                  */
>> > -                virtual bool        isSingleShot() const;
>> > -                virtual void        done();
>> > +                virtual bool                        isSingleShot
>> > + () const;
>> > +                virtual void                        done
>> > + ();
>> > +                virtual void                        addBaseUrl
>> > + (BaseUrl *url);
>> > +                virtual std::vector<BaseUrl *>      getBaseUrls
>> > + () const;
>> > +                virtual void                        setByteRange
>> > + (int start, int end);
>> > +                virtual int                         getStartByte
>> > + () const;
>> > +                virtual int                         getEndByte
>> > + () const;
>> > +                virtual dash::http::Chunk*          toChunk
>> > + ();
>> >
>> >             protected:
>> > -                std::string         sourceUrl;
>> > +                std::string             sourceUrl;
>> > +                std::vector<BaseUrl *>  baseUrls;
>> > +                int                     startByte;
>> > +                int                     endByte;
>> >         };
>> >     }
>> >  }
>> > --
>> > 1.7.0.4
>> >

Since Segment and Chunk now share some common attributes, maybe you
should move these common attributes in another class, and make both
Segment and Chunk inherit from this new class.
Also, maybe it would be easier to have a pointer to the "parent"
Segment in each chunk, or another way to be able to query some basic
info such as a Segment duration from a chunk instance.

Regards,

-- 
Hugo Beauzée-Luyssen



More information about the vlc-devel mailing list