[vlc-devel] [PATCH 4/8] dash: added blockbuffer

Christopher Müller christopher.mueller at itec.uni-klu.ac.at
Tue Feb 7 12:40:03 CET 2012


> Von: vlc-devel-bounces at videolan.org [mailto:vlc-devel-
> bounces at videolan.org] Im Auftrag von Hugo Beauzée-Luyssen
> Gesendet: Dienstag, 07. Februar 2012 12:28
> An: Mailing list for VLC media player developers
> Betreff: Re: [vlc-devel] [PATCH 4/8] dash: added blockbuffer
> 
> 2012/2/6 Christopher Müller <christopher.mueller at itec.uni-klu.ac.at>:
> > Fixed the leakage and added the include config.h.
> >
> >> -----Ursprüngliche Nachricht-----
> >> Von: vlc-devel-bounces at videolan.org [mailto:vlc-devel-
> >> bounces at videolan.org] Im Auftrag von Hugo Beauzée-Luyssen
> >> Gesendet: Sonntag, 05. Februar 2012 22:22
> >> An: Mailing list for VLC media player developers
> >> Betreff: Re: [vlc-devel] [PATCH 4/8] dash: added blockbuffer
> >>
> >> On Sun, Feb 5, 2012 at 10:34 AM, Christopher Mueller
> >> <christopher.mueller at itec.aau.at> wrote:
> >> > ---
> >> >  modules/stream_filter/dash/Modules.am              |    3 +
> >> >  modules/stream_filter/dash/buffer/BlockBuffer.cpp  |  196
> >> > ++++++++++++++++++++
> >> >  modules/stream_filter/dash/buffer/BlockBuffer.h    |   73
> ++++++++
> >> >  .../stream_filter/dash/buffer/IBufferObserver.h    |   42 ++++
> >> >  4 files changed, 314 insertions(+), 0 deletions(-)
> >> >  create mode 100644
> >> > modules/stream_filter/dash/buffer/BlockBuffer.cpp
> >> >  create mode 100644
> modules/stream_filter/dash/buffer/BlockBuffer.h
> >> >  create mode 100644
> >> > modules/stream_filter/dash/buffer/IBufferObserver.h
> >> >
> >> > diff --git a/modules/stream_filter/dash/Modules.am
> >> > b/modules/stream_filter/dash/Modules.am
> >> > index fdc1076..00fb071 100644
> >> > --- a/modules/stream_filter/dash/Modules.am
> >> > +++ b/modules/stream_filter/dash/Modules.am
> >> > @@ -9,6 +9,9 @@ SOURCES_stream_filter_dash = \
> >> >     adaptationlogic/IDownloadRateObserver.h \
> >> >     adaptationlogic/RateBasedAdaptationLogic.h \
> >> >     adaptationlogic/RateBasedAdaptationLogic.cpp \
> >> > +    buffer/BlockBuffer.cpp \
> >> > +    buffer/BlockBuffer.h \
> >> > +    buffer/IBufferObserver.h \
> >> >     exceptions/EOFException.h \
> >> >     http/Chunk.cpp \
> >> >     http/Chunk.h \
> >> > diff --git a/modules/stream_filter/dash/buffer/BlockBuffer.cpp
> >> > b/modules/stream_filter/dash/buffer/BlockBuffer.cpp
> >> > new file mode 100644
> >> > index 0000000..a3b28b4
> >> > --- /dev/null
> >> > +++ b/modules/stream_filter/dash/buffer/BlockBuffer.cpp
> >> > @@ -0,0 +1,196 @@
> >> > +/*
> >> > + * BlockBuffer.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.
> >> > +
> >> >
> >>
> +********************************************************************
> >> +*
> >> > +********/
> >> > +
> >> > +#include "buffer/BlockBuffer.h"
> >> > +
> >> > +using namespace dash::buffer;
> >> > +
> >> > +BlockBuffer::BlockBuffer    (stream_t *stream) :
> >> > +             sizeMilliSec   (0),
> >> > +             sizeBytes      (0),
> >> > +             stream         (stream),
> >> > +             isEOF          (false)
> >> > +
> >> > +{
> >> > +    this->capacityMilliSec  = var_InheritInteger(stream,
> >> > +"dash-bufferSize") * 1000;
> >> > +
> >>
> >> If this value is 0, BlockBuffer::notify will try to divide by zero.
> >> Also, you should probably check for inconsistant value, such as
> >> negative ones.
> >>
> >> > +    block_BytestreamInit(&this->buffer);
> >> > +    vlc_mutex_init(&this->monitorMutex);
> >> > +    vlc_cond_init(&this->empty);
> >> > +    vlc_cond_init(&this->full);
> >> > +}
> >> > +BlockBuffer::~BlockBuffer   ()
> >> > +{
> >> > +    std::cout << "Delete buffer" << std::endl;
> >> > +    block_BytestreamRelease(&this->buffer);
> >> > +    vlc_mutex_destroy(&this->monitorMutex);
> >> > +    vlc_cond_destroy(&this->empty);
> >> > +    vlc_cond_destroy(&this->full); }
> >> > +
> >> > +int     BlockBuffer::peek                 (const uint8_t
> >> > +**pp_peek, unsigned int len) {
> >> > +    vlc_mutex_lock(&this->monitorMutex);
> >> > +
> >> > +    while(this->sizeBytes == 0 && !this->isEOF)
> >> > +        vlc_cond_wait(&this->full, &this->monitorMutex);
> >> > +
> >> > +    if(this->sizeBytes == 0)
> >> > +    {
> >> > +        vlc_cond_signal(&this->empty);
> >> > +        vlc_mutex_unlock(&this->monitorMutex);
> >> > +        return 0;
> >> > +    }
> >> > +
> >> > +    int     ret     = len > this->sizeBytes ? this->sizeBytes :
> >> > + len;
> >> > +    uint8_t *peek   = new uint8_t[ret];
> >>
> >> This will leak. Also if you are allocating a value that is supposed
> >> to be freed by VLC, you must use malloc(), as vlc will use free()
> >>
> >> > +
> >> > +    std::cout << "Peek Bytes: " << ret << " from buffer length: "
> >> > + <<
> >> > + this->sizeBytes << std::endl;
> >> > +
> >> > +    block_PeekBytes(&this->buffer, peek, ret);
> >> > +    *pp_peek = peek;
> >> > +
> >> > +    std::cout << "Buffer length Sec: " << this->sizeMilliSec << "
> >> > +Bytes: " << this->sizeBytes<< std::endl;
> >> > +    vlc_mutex_unlock(&this->monitorMutex);
> >> > +    return ret;
> >> > +}
> >> > +int     BlockBuffer::get                  (void *p_data, unsigned
> >> int
> >> > +len) {
> >> > +    vlc_mutex_lock(&this->monitorMutex);
> >> > +
> >> > +    while(this->sizeBytes == 0 && !this->isEOF)
> >> > +        vlc_cond_wait(&this->full, &this->monitorMutex);
> >> > +
> >> > +    if(this->sizeBytes == 0)
> >> > +    {
> >> > +        vlc_cond_signal(&this->empty);
> >> > +        vlc_mutex_unlock(&this->monitorMutex);
> >> > +        return 0;
> >> > +    }
> >> > +
> >> > +    int ret = len > this->sizeBytes ? this->sizeBytes : len;
> >> > +
> >> > +    this->reduceBufferMilliSec(ret);
> >> > +
> >> > +    std::cout << "Get Bytes: " << ret << " from buffer length: "
> >> > + <<
> >> > + this->sizeBytes << std::endl;
> >> > +
> >> > +    block_GetBytes(&this->buffer, (uint8_t *)p_data, ret);
> >> > +
> >> > +    this->sizeBytes -= ret;
> >> > +
> >> > +    if(this->sizeBytes == 0)
> >> > +        this->sizeMilliSec = 0;
> >> > +
> >> > +    std::cout << "Buffer length: " << this->sizeMilliSec << "
> Bytes:
> >> > + " << this->sizeBytes << std::endl;
> >> > +
> >> > +    vlc_cond_signal(&this->empty);
> >> > +    vlc_mutex_unlock(&this->monitorMutex);
> >> > +    return ret;
> >> > +}
> >> > +void    BlockBuffer::put                  (block_t *block) {
> >> > +    vlc_mutex_lock(&this->monitorMutex);
> >> > +
> >> > +    while(this->sizeMilliSec >= this->capacityMilliSec &&
> >> > + !this->isEOF)
> >> > +        vlc_cond_wait(&this->empty, &this->monitorMutex);
> >> > +
> >> > +    if(this->isEOF)
> >> > +    {
> >> > +        vlc_cond_signal(&this->full);
> >> > +        vlc_mutex_unlock(&this->monitorMutex);
> >> > +        return;
> >> > +    }
> >> > +
> >> > +    std::cout << "Put MilliSec: " << block->i_length << " Bytes:
> "
> >> <<
> >> > + block->i_buffer << " into buffer" << std::endl;
> >> > +    this->sizeMilliSec   += block->i_length;
> >> > +    this->sizeBytes += block->i_buffer;
> >> > +
> >> > +    block_BytestreamPush(&this->buffer, block);
> >> > +
> >> > +    std::cout << "Buffer length: " << this->sizeMilliSec << "
> Bytes:
> >> > +" << this->sizeBytes << std::endl;
> >> > +    vlc_cond_signal(&this->full);
> >> > +    vlc_mutex_unlock(&this->monitorMutex);
> >> > +}
> >> > +void    BlockBuffer::setEOF               (bool value) {
> >> > +    vlc_mutex_lock(&this->monitorMutex);
> >> > +    this->isEOF = value;
> >> > +    vlc_cond_signal(&this->empty);
> >> > +    vlc_cond_signal(&this->full);
> >> > +    vlc_mutex_unlock(&this->monitorMutex);
> >> > +}
> >> > +bool    BlockBuffer::getEOF               () {
> >> > +    vlc_mutex_lock(&this->monitorMutex);
> >> > +    bool ret = this->isEOF;
> >> > +    vlc_mutex_unlock(&this->monitorMutex);
> >> > +    return ret;
> >> > +}
> >> > +void    BlockBuffer::attach               (IBufferObserver
> >> *observer)
> >> > +{
> >> > +    this->bufferObservers.push_back(observer);
> >> > +}
> >> > +void    BlockBuffer::notify               () {
> >> > +    for(size_t i = 0; i < this->bufferObservers.size(); i++)
> >> > +
> >> > +this->bufferObservers.at(i)->bufferLevelChanged(this-
> >sizeMilliSec
> >> > +this->, sizeMilliSec / this->capacityMilliSec); } void
> >> > +BlockBuffer::reduceBufferMilliSec (size_t bytes) {
> >> > +    size_t  pos = 0;
> >> > +    mtime_t sec = 0;
> >> > +
> >> > +    block_t *block = this->buffer.p_block;
> >> > +
> >> > +    if(bytes <= (block->i_buffer - this->buffer.i_offset))
> >> > +    {
> >> > +        pos = bytes;
> >> > +        sec = (block->i_length / block->i_buffer) * bytes;
> >> > +    }
> >> > +    else
> >> > +    {
> >> > +        pos = block->i_buffer;
> >> > +        sec = (block->i_length / block->i_buffer) *
> >> > + (block->i_buffer
> >> > + - this->buffer.i_offset);
> >> > +    }
> >> > +
> >> > +    while(pos < bytes)
> >> > +    {
> >> > +        block = this->buffer.p_block->p_next;
> >> > +        if((bytes - pos) <= (block->i_buffer -
> >> > + this->buffer.i_offset))
> >> > +        {
> >> > +            pos = bytes;
> >> > +            sec += (block->i_length/ block->i_buffer) * (bytes -
> >> > + pos);
> >> > +        }
> >> > +        else
> >> > +        {
> >> > +            pos += block->i_buffer;
> >> > +            sec += block->i_length;
> >> > +        }
> >> > +    }
> >> > +
> >> > +    this->sizeMilliSec -= sec;
> >> > +
> >> > +    if(this->sizeMilliSec < 0)
> >> > +        this->sizeMilliSec = 0;
> >> > +}
> >> > diff --git a/modules/stream_filter/dash/buffer/BlockBuffer.h
> >> > b/modules/stream_filter/dash/buffer/BlockBuffer.h
> >> > new file mode 100644
> >> > index 0000000..d1035e3
> >> > --- /dev/null
> >> > +++ b/modules/stream_filter/dash/buffer/BlockBuffer.h
> >> > @@ -0,0 +1,73 @@
> >> > +/*
> >> > + * BlockBuffer.h
> >> > +
> >> >
> >>
> +********************************************************************
> >> +*
> >> > +********
> >> > + * 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.
> >> > +
> >> >
> >>
> +********************************************************************
> >> +*
> >> > +********/
> >> > +
> >> > +#ifndef BLOCKBUFFER_H_
> >> > +#define BLOCKBUFFER_H_
> >> > +
> >> > +#include "buffer/IBufferObserver.h"
> >> > +
> >> > +#include <vlc_stream.h>
> >> > +#include <vlc_block_helper.h>
> >> > +#include <vector>
> >> > +
> >> > +#include <iostream>
> >> > +
> >> > +namespace dash
> >> > +{
> >> > +    namespace buffer
> >> > +    {
> >> > +        class BlockBuffer
> >> > +        {
> >> > +            public:
> >> > +                BlockBuffer           (stream_t *stream);
> >> > +                virtual ~BlockBuffer  ();
> >> > +
> >> > +                void    put         (block_t *block);
> >> > +                int     get         (void *p_data, unsigned int
> >> len);
> >> > +                int     peek        (const uint8_t **pp_peek,
> >> > + unsigned int i_peek);
> >> > +                void    setEOF      (bool value);
> >> > +                bool    getEOF      ();
> >> > +                mtime_t sizeLeft    ();
> >> > +                void    attach      (IBufferObserver *observer);
> >> > +                void    notify      ();
> >> > +
> >> > +            private:
> >> > +                mtime_t             capacityMilliSec;
> >> > +                mtime_t             sizeMilliSec;
> >> > +                size_t              sizeBytes;
> >> > +                vlc_mutex_t         monitorMutex;
> >> > +                vlc_cond_t          empty;
> >> > +                vlc_cond_t          full;
> >> > +                stream_t            *stream;
> >> > +                bool                isEOF;
> >> > +                block_bytestream_t  buffer;
> >> > +
> >> > +                std::vector<IBufferObserver *> bufferObservers;
> >> > +
> >> > +                void reduceBufferMilliSec(size_t bytes);
> >> > +        };
> >> > +    }
> >> > +}
> >> > +
> >> > +#endif /* BLOCKBUFFER_H_ */
> >> > diff --git a/modules/stream_filter/dash/buffer/IBufferObserver.h
> >> > b/modules/stream_filter/dash/buffer/IBufferObserver.h
> >> > new file mode 100644
> >> > index 0000000..b2a6315
> >> > --- /dev/null
> >> > +++ b/modules/stream_filter/dash/buffer/IBufferObserver.h
> >> > @@ -0,0 +1,42 @@
> >> > +/*
> >> > + * IBufferObserver.h
> >> > +
> >> >
> >>
> +********************************************************************
> >> +*
> >> > +********
> >> > + * 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.
> >> > +
> >> >
> >>
> +********************************************************************
> >> +*
> >> > +********/
> >> > +
> >> > +#ifndef IBUFFEROBSERVER_H_
> >> > +#define IBUFFEROBSERVER_H_
> >> > +
> >> > +#include <vlc_common.h>
> >> > +
> >> > +namespace dash
> >> > +{
> >> > +    namespace buffer
> >> > +    {
> >> > +        class IBufferObserver
> >> > +        {
> >> > +            public:
> >>
> >> This interface is missing a virtual destructor.
> >>
> >> > +                virtual void bufferLevelChanged(mtime_t
> >> > +bufferedSeconds, int bufferedPercent) = 0;
> >> > +        };
> >> > +    }
> >> > +}
> >> > +
> >> > +#endif /* IBUFFEROBSERVER_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
> 
> I must have expressed myself badly, there were two problems :
> - You are allocating a buffer that will no be freed by VLC, therefore,
> each time peek() will be called, you will leak.

So what is the correct way. Is the memory allocated? If not when do I know that I can delete the memory?

> - My guess was that you assumed the buffer would be freed by VLC, in
> which case, you would have to use malloc, not new.

That is what I have assumed ;)

BR
Chris




More information about the vlc-devel mailing list