[vlmc-devel] IProducer::cut(), Media, Clip: Introduce unique_ptr to handle IProducer

yikei lu luyikei.qmltu at gmail.com
Mon Jun 27 15:20:21 CEST 2016


Hi,

Yeah, I think we can manage to remove some getter functions because
they are only used to connect to output ( formally known as consumer
:) ), but I still need more time to think a good way to do that.

Regards

2016-06-27 22:05 GMT+09:00 Hugo Beauzée-Luyssen <hugo at beauzee.fr>:
> On 06/27/2016 10:57 AM, Yikai Lu wrote:
>>
>> vlmc | branch: master | Yikai Lu <luyikei.qmltu at gmail.com> | Mon Jun 27
>> 17:09:10 2016 +0900| [b11c2aa9882a069f16571a80982495328116e5b4] | committer:
>> Yikai Lu
>>
>> IProducer::cut(), Media, Clip: Introduce unique_ptr to handle IProducer
>>
>>>
>>> https://code.videolan.org/videolan/vlmc/commit/b11c2aa9882a069f16571a80982495328116e5b4
>>
>> ---
>>
>>  src/Backend/IProducer.h         | 4 +++-
>>  src/Backend/MLT/MLTProducer.cpp | 4 ++--
>>  src/Backend/MLT/MLTProducer.h   | 2 +-
>>  src/Media/Clip.cpp              | 7 +++----
>>  src/Media/Clip.h                | 4 +++-
>>  src/Media/Media.cpp             | 8 +++-----
>>  src/Media/Media.h               | 5 ++++-
>>  7 files changed, 19 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/Backend/IProducer.h b/src/Backend/IProducer.h
>> index 408cee2..5e6d388 100644
>> --- a/src/Backend/IProducer.h
>> +++ b/src/Backend/IProducer.h
>> @@ -24,6 +24,8 @@
>>  #define IPRODUCER_H
>>
>>  #include <cstdint>
>> +#include <memory>
>> +
>>  #include "Backend/IService.h"
>>
>>  namespace Backend
>> @@ -59,7 +61,7 @@ namespace Backend
>>          virtual void            setBoundaries( int64_t begin, int64_t end
>> ) = 0;
>>
>>          // Absolute position in frames
>> -        virtual IProducer*      cut( int64_t begin  = 0, int64_t end  =
>> EndOfMedia ) = 0;
>> +        virtual std::unique_ptr<IProducer>      cut( int64_t begin  = 0,
>> int64_t end  = EndOfMedia ) = 0;
>>          virtual bool            isCut( ) const = 0 ;
>>
>>          virtual bool            sameClip( IProducer& that ) const = 0;
>> diff --git a/src/Backend/MLT/MLTProducer.cpp
>> b/src/Backend/MLT/MLTProducer.cpp
>> index 5466b11..9a21013 100644
>> --- a/src/Backend/MLT/MLTProducer.cpp
>> +++ b/src/Backend/MLT/MLTProducer.cpp
>> @@ -161,10 +161,10 @@ MLTProducer::setBoundaries( int64_t begin, int64_t
>> end )
>>      m_producer->set_in_and_out( begin, end );
>>  }
>>
>> -Backend::IProducer*
>> +std::unique_ptr<Backend::IProducer>
>>  MLTProducer::cut( int64_t begin, int64_t end )
>>  {
>> -    return new MLTProducer( m_producer->cut( begin, end ) );
>> +    return std::unique_ptr<IProducer>( new MLTProducer( m_producer->cut(
>> begin, end ) ) );
>>  }
>>
>>  bool
>> diff --git a/src/Backend/MLT/MLTProducer.h b/src/Backend/MLT/MLTProducer.h
>> index ff89052..87024cd 100644
>> --- a/src/Backend/MLT/MLTProducer.h
>> +++ b/src/Backend/MLT/MLTProducer.h
>> @@ -57,7 +57,7 @@ class MLTProducer : virtual public IProducer, public
>> MLTService
>>          virtual void            setEnd( int64_t end ) override;
>>          virtual void            setBoundaries( int64_t begin, int64_t end
>> ) override;
>>
>> -        virtual IProducer*      cut( int64_t begin = 0, int64_t end =
>> EndOfMedia ) override;
>> +        virtual std::unique_ptr<IProducer>      cut( int64_t begin = 0,
>> int64_t end = EndOfMedia ) override;
>>          virtual bool            isCut() const override;
>>
>>          virtual bool            sameClip( IProducer& that ) const
>> override;
>> diff --git a/src/Media/Clip.cpp b/src/Media/Clip.cpp
>> index 0be55c5..b2b40c1 100644
>> --- a/src/Media/Clip.cpp
>> +++ b/src/Media/Clip.cpp
>> @@ -37,7 +37,7 @@
>>  Clip::Clip( Media *media, qint64 begin /*= 0*/, qint64 end /*=
>> Backend::IProducer::EndOfMedia */, const QString& uuid /*= QString()*/ ) :
>>          Workflow::Helper( uuid ),
>>          m_media( media ),
>> -        m_producer( m_media->producer()->cut( begin, end ) ),
>> +        m_producer( std::move( m_media->producer()->cut( begin, end ) )
>> ),
>>          m_parent( media->baseClip() ),
>>          m_clipWorkflow( nullptr )
>>  {
>> @@ -69,7 +69,6 @@ Clip::~Clip()
>>  {
>>      emit unloaded( this );
>>      delete m_childs;
>> -    delete m_producer;
>>      if ( isRootClip() == true )
>>          delete m_media;
>>  }
>> @@ -261,7 +260,7 @@ Clip::toVariant() const
>>          h.insert( "begin", begin() );
>>          h.insert( "end", end() );
>>      }
>> -    h.insert( "filters", EffectHelper::toVariant( m_producer ) );
>> +    h.insert( "filters", EffectHelper::toVariant( m_producer.get() ) );
>>      return QVariant( h );
>>
>>  }
>> @@ -297,7 +296,7 @@ Clip::setFormats( Formats formats )
>>  Backend::IProducer*
>>  Clip::producer()
>>  {
>> -    return m_producer;
>> +    return m_producer.get();
>
>
> I'd return it as a reference to decrease the likeliness of someone storing
> this pointer.
>
>
>>  }
>>
>>  void
>> diff --git a/src/Media/Clip.h b/src/Media/Clip.h
>> index d50141f..5e384a6 100644
>> --- a/src/Media/Clip.h
>> +++ b/src/Media/Clip.h
>> @@ -35,6 +35,8 @@
>>  #include <QXmlStreamWriter>
>>  #include "Backend/IProducer.h"
>>
>> +#include <memory>
>> +
>>  class   MediaContainer;
>>  class   Media;
>>  class   ClipWorkflow;
>> @@ -139,7 +141,7 @@ class   Clip : public Workflow::Helper
>>
>>      private:
>>          Media*              m_media;
>> -        Backend::IProducer* m_producer;
>> +        std::unique_ptr<Backend::IProducer> m_producer;
>>
>>          QStringList         m_metaTags;
>>          QString             m_notes;
>> diff --git a/src/Media/Media.cpp b/src/Media/Media.cpp
>> index 8d9c6d6..e3af086 100644
>> --- a/src/Media/Media.cpp
>> +++ b/src/Media/Media.cpp
>> @@ -71,7 +71,6 @@ Media::Media(const QString &path )
>>
>>  Media::~Media()
>>  {
>> -    delete m_producer;
>>      delete m_fileInfo;
>>  }
>>
>> @@ -120,13 +119,13 @@ Media::toVariant() const
>>  Backend::IProducer*
>>  Media::producer()
>>  {
>> -    return m_producer;
>> +    return m_producer.get();
>
>
> Same as above
>
>>  }
>>
>>  const Backend::IProducer*
>>  Media::producer() const
>>  {
>> -    return m_producer;
>> +    return m_producer.get();
>
>
> And here as well
> Although we might want to get rid the the backend classes from within the
> media/clip classes. I don't think this is required out of the workflow
> classes (but I need to familiarize myself with the new architecture :) )
>
>
>>  }
>>
>>  void
>> @@ -138,8 +137,7 @@ Media::setFilePath( const QString &filePath )
>>      m_fileName = m_fileInfo->fileName();
>>      m_mrl = "file:///" + QUrl::toPercentEncoding( filePath, "/" );
>>
>> -    delete m_producer;
>> -    m_producer = new Backend::MLT::MLTProducer( qPrintable( filePath ) );
>> +    m_producer.reset( new Backend::MLT::MLTProducer( qPrintable( filePath
>> ) ) );
>>  }
>>
>>  #ifdef WITH_GUI
>> diff --git a/src/Media/Media.h b/src/Media/Media.h
>> index bc978c2..4018c2f 100644
>> --- a/src/Media/Media.h
>> +++ b/src/Media/Media.h
>> @@ -31,6 +31,9 @@
>>  #define MEDIA_H__
>>
>>  #include "config.h"
>> +
>> +#include <memory>
>> +
>>  #include <QString>
>>  #include <QObject>
>>  #include <QFileInfo>
>> @@ -110,7 +113,7 @@ public:
>>      QPixmap&                    snapshot();
>>  #endif
>>  protected:
>> -    Backend::IProducer*         m_producer;
>> +    std::unique_ptr<Backend::IProducer>         m_producer;
>>      QString                     m_mrl;
>>      QFileInfo*                  m_fileInfo;
>>      FileType                    m_fileType;
>>
>> _______________________________________________
>> Vlmc-devel mailing list
>> Vlmc-devel at videolan.org
>> https://mailman.videolan.org/listinfo/vlmc-devel
>>
>
> If we want to expose the Backend types from Clip & Media, I think shared_ptr
> is a better choice (since we de facto share the ownership of the backend
> class with the producer() method callers.
> If we manage to remove the dependency on the Backend classes from outside of
> the workflow classes, then unique_ptr is the probably the best choice
>
> Regards,
>
>
> _______________________________________________
> Vlmc-devel mailing list
> Vlmc-devel at videolan.org
> https://mailman.videolan.org/listinfo/vlmc-devel


More information about the Vlmc-devel mailing list