[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