[vlc-devel] [PATCH 5/9] dash: added Persistent Connection

Hugo Beauzée-Luyssen beauze.h at gmail.com
Mon Mar 12 11:26:47 CET 2012


On Fri, Mar 9, 2012 at 7:05 PM,  <Christopher at mailsrv.uni-klu.ac.at> wrote:
> From: Christopher Mueller <christopher.mueller at itec.aau.at>
>
> ---
>  modules/stream_filter/dash/Modules.am              |    2 +
>  .../dash/http/PersistentConnection.cpp             |  196 ++++++++++++++++++++
>  .../stream_filter/dash/http/PersistentConnection.h |   63 +++++++
>  3 files changed, 261 insertions(+), 0 deletions(-)
>  create mode 100644 modules/stream_filter/dash/http/PersistentConnection.cpp
>  create mode 100644 modules/stream_filter/dash/http/PersistentConnection.h
>
> diff --git a/modules/stream_filter/dash/Modules.am b/modules/stream_filter/dash/Modules.am
> index 9f9b2e0..fe6d7d4 100644
> --- a/modules/stream_filter/dash/Modules.am
> +++ b/modules/stream_filter/dash/Modules.am
> @@ -20,6 +20,8 @@ SOURCES_stream_filter_dash = \
>     http/HTTPConnectionManager.cpp \
>     http/HTTPConnectionManager.h \
>     http/IHTTPConnection.h \
> +    http/PersistentConnection.cpp \
> +    http/PersistentConnection.h \
>     mpd/AdaptationSet.cpp \
>     mpd/AdaptationSet.h \
>     mpd/BaseUrl.h \
> diff --git a/modules/stream_filter/dash/http/PersistentConnection.cpp b/modules/stream_filter/dash/http/PersistentConnection.cpp
> new file mode 100644
> index 0000000..7d5af56
> --- /dev/null
> +++ b/modules/stream_filter/dash/http/PersistentConnection.cpp
> @@ -0,0 +1,196 @@
> +/*
> + * PersistentConnection.cpp
> + *****************************************************************************
> + * Copyright (C) 2010 - 2011 Klagenfurt University
> + *
> + * Created on: Aug 10, 2010
> + * Authors: Christopher Mueller <christopher.mueller at itec.uni-klu.ac.at>
> + *          Christian Timmerer  <christian.timmerer at itec.uni-klu.ac.at>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> + *****************************************************************************/
> +

It misses the #include "config.h"

> +#include "PersistentConnection.h"
> +
> +using namespace dash::http;
> +
> +
> +PersistentConnection::PersistentConnection  (stream_t *stream) :
> +                      HTTPConnection        (stream),
> +                      isInit                (false)
> +{
> +}
> +PersistentConnection::~PersistentConnection ()
> +{
If the chunk should be deleted from here, you should ensure chunkQueue is empty.

> +}
> +
> +int                 PersistentConnection::read              (void *p_buffer, unsigned int len)
> +{
> +    Chunk *readChunk = this->chunkQueue.front();
This will raise an exception if the deque is empty.
> +
> +    if(readChunk == NULL)
> +        return -1;
This can't happen, as it would currently segfault when adding a NULL
chunk. Although, this shouldn't happen if you refuse to append a NULL
Chunk.

> +
> +    if(readChunk->getBytesRead() == 0)
> +    {
> +        if(!this->initChunk(readChunk))
> +        {
> +            this->chunkQueue.pop_front();
> +            return -1;
> +        }
> +    }
> +
> +    if(readChunk->getBytesToRead() == 0)
> +    {
> +        this->chunkQueue.pop_front();
> +        return 0;
> +    }
> +
> +    int ret = 0;
> +    if(len > readChunk->getBytesToRead())
> +        ret = HTTPConnection::read(p_buffer, readChunk->getBytesToRead());
> +    else
> +        ret = HTTPConnection::read(p_buffer, len);
> +
> +    if(ret <= 0)
> +    {
> +        readChunk->setStartByte(readChunk->getStartByte() + readChunk->getBytesRead());
> +        readChunk->setBytesRead(0);
> +        if(!this->reconnect(readChunk))
> +        {
> +            this->chunkQueue.pop_front();
> +            return -1;
> +        }
> +
> +        return this->read(p_buffer, len);
> +    }
> +
> +    readChunk->setBytesRead(readChunk->getBytesRead() + ret);
> +
I don't quite remember how Chunks are handled, but don't they need to
be deleted from here?

> +    return ret;
> +}
> +std::string         PersistentConnection::prepareRequest    (Chunk *chunk)
> +{
> +    std::string request;
> +    if(!chunk->useByteRange())
> +    {
> +        request = "GET "    + chunk->getPath()     + " HTTP/1.1" + "\r\n" +
> +                  "Host: "  + chunk->getHostname() + "\r\n\r\n";
> +    }
> +    else
> +    {
> +        std::stringstream req;
> +        req << "GET " << chunk->getPath() << " HTTP/1.1\r\n" <<
> +               "Host: " << chunk->getHostname() << "\r\n" <<
> +               "Range: bytes=" << chunk->getStartByte() << "-" << chunk->getEndByte() << "\r\n\r\n";
> +
> +        request = req.str();
> +    }
> +    return request;
> +}
> +bool                PersistentConnection::init              (Chunk *chunk)
> +{
> +    if(this->isInit)
> +        return true;
> +
Chunk isn't checked for NULL

> +    if(!chunk->hasHostname())
> +        if(!this->setUrlRelative(chunk))
> +            return false;
> +
> +    this->httpSocket = net_ConnectTCP(this->stream, chunk->getHostname().c_str(), chunk->getPort());
> +
> +    if(!this->httpSocket)
The error value is -1

> +        return false;
> +
> +    if(this->sendData(this->prepareRequest(chunk)))
> +        this->isInit = true;
> +
> +    this->chunkQueue.push_back(chunk);
> +    this->hostname = chunk->getHostname();
> +
> +    return this->isInit;
> +}
> +bool                PersistentConnection::addChunk          (Chunk *chunk)
> +{
This will crash if chunk is NULL (which will happen if there's no more
segment to read)

> +    if(!this->isInit)
> +        return this->init(chunk);
> +
> +    if(!chunk->hasHostname())
> +        if(!this->setUrlRelative(chunk))
> +            return false;
> +
> +    if(chunk->getHostname().compare(this->hostname))
> +        return false;
> +
> +    if(this->sendData(this->prepareRequest(chunk)))
> +    {
> +        this->chunkQueue.push_back(chunk);
> +        return true;
> +    }
> +
> +    return false;
> +}
> +bool                PersistentConnection::initChunk         (Chunk *chunk)
> +{
> +    if(this->parseHeader())
> +    {
> +        chunk->setLength(this->contentLength);
> +        return true;
> +    }
> +
> +    if(!this->reconnect(chunk))
> +        return false;
> +
> +    if(this->parseHeader())
> +    {
> +        chunk->setLength(this->contentLength);
> +        return true;
> +    }
> +
> +    return false;
> +}
> +bool                PersistentConnection::reconnect         (Chunk *chunk)
> +{
> +    int         retry   = 0;
> +    std::string request = this->prepareRequest(chunk);
> +
> +    while(retry < RETRY)
> +    {
> +        this->httpSocket = net_ConnectTCP(this->stream, chunk->getHostname().c_str(), chunk->getPort());
> +        if(this->httpSocket)
Again, this error check is invalid.

> +            if(this->resendAllRequests())
> +                return true;
> +
> +        retry++;
> +    }
> +
> +    return false;
> +}
> +const std::string&  PersistentConnection::getHostname       ()
> +{
> +    return this->hostname;
> +}
> +bool                PersistentConnection::isConnected       ()
> +{
> +    return this->isInit;
> +}
> +bool                PersistentConnection::resendAllRequests ()
> +{
> +    for(size_t i = 0; i < this->chunkQueue.size(); i++)
> +        if(!this->sendData((this->prepareRequest(this->chunkQueue.at(i)))))
> +            return false;
> +
> +    return true;
> +}
> diff --git a/modules/stream_filter/dash/http/PersistentConnection.h b/modules/stream_filter/dash/http/PersistentConnection.h
> new file mode 100644
> index 0000000..c62ba33
> --- /dev/null
> +++ b/modules/stream_filter/dash/http/PersistentConnection.h
> @@ -0,0 +1,63 @@
> +/*
> + * PersistentConnection.h
> + *****************************************************************************
> + * Copyright (C) 2010 - 2011 Klagenfurt University
Happy new year! :D

> + *
> + * Created on: Aug 10, 2010
> + * Authors: Christopher Mueller <christopher.mueller at itec.uni-klu.ac.at>
> + *          Christian Timmerer  <christian.timmerer at itec.uni-klu.ac.at>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> + *****************************************************************************/
> +
> +#ifndef PERSISTENTCONNECTION_H_
> +#define PERSISTENTCONNECTION_H_
> +
> +#include "HTTPConnection.h"
> +#include <deque>
> +
> +#define RETRY 5
I'd go for a static const member instead of a define.

> +
> +namespace dash
> +{
> +    namespace http
> +    {
> +        class PersistentConnection : public HTTPConnection
> +        {
> +            public:
> +                PersistentConnection            (stream_t *stream);
> +                virtual ~PersistentConnection   ();
> +
> +                virtual int         read        (void *p_buffer, unsigned int len);
IHttpConnection declares this method as (void*, size_t), I kinda fear
some broken compiler may see unsigned int as a different type.
Moreover, IIRC the standard doesn't state size_t == unsigned int
You should change the type to ensure this will overwrite read() method.

> +                virtual bool        init        (Chunk *chunk);
> +                bool                addChunk    (Chunk *chunk);
> +                const std::string&  getHostname ();
> +                bool                isConnected ();
Getters should be const methods.

> +
> +            private:
> +                std::deque<Chunk *>  chunkQueue;
> +                bool                isInit;
> +                std::string         hostname;
> +
> +            protected:
> +                virtual std::string prepareRequest      (Chunk *chunk);
> +                bool                initChunk           (Chunk *chunk);
> +                bool                reconnect           (Chunk *chunk);
> +                bool                resendAllRequests   ();
> +        };
> +    }
> +}
> +
> +#endif /* PERSISTENTCONNECTION_H_ */
> --
> 1.7.0.4
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel



-- 
Hugo Beauzée-Luyssen



More information about the vlc-devel mailing list