[vlmc-devel] [PATCH 4/4] VLCSource: Avoid deleting VmemRenderer too early

[Hugo Beauzée-Luyssen]

On 04/12/2016 01:10 PM, Yikai Lu wrote:
> If we declare in a function like "VmemRenderer renderer( some values )" just as we did before, it will be removed as soon as the function ends. Since the renderer don't know how much time should be needed to render a snapshot, we should keep it alive as long as possible.
> ---
>   src/Backend/VLC/VLCSource.cpp | 20 +++++++++++++-------
>   src/Backend/VLC/VLCSource.h   |  4 +++-
>   2 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/src/Backend/VLC/VLCSource.cpp b/src/Backend/VLC/VLCSource.cpp
> index d38d12f..3257781 100644
> --- a/src/Backend/VLC/VLCSource.cpp
> +++ b/src/Backend/VLC/VLCSource.cpp
> @@ -39,10 +39,16 @@ VLCSource::VLCSource( VLCBackend* backend, const QString& path )
>       , m_snapshot( nullptr )
>       , m_isParsed( false )
>       , m_nbFrames( 0 )
> +    , m_vmemRenderer( nullptr )
>   {
>       m_media = ::VLC::Media( backend->vlcInstance(), path.toStdString(), ::VLC::Media::FromPath );
>   }
>
> +VLCSource::~VLCSource()
> +{
> +    delete m_vmemRenderer;
> +}
> +
>   ::VLC::Media&
>   VLCSource::media()
>   {
> @@ -63,7 +69,6 @@ VLCSource::preparse()
>       Q_ASSERT( m_nbAudioTracks == 0 );
>       Q_ASSERT( m_nbVideoTracks == 0 );
>
> -    VmemRenderer renderer( m_backend, this, nullptr );
>       m_media.parse();
>       m_length = m_media.duration();
>       auto tracks = m_media.tracks();
> @@ -85,7 +90,7 @@ VLCSource::preparse()
>                   m_width = t.width();
>                   m_height = t.height();
>                   m_nbFrames = (int64_t)( (float)( m_length / 1000 ) * m_fps );
> -                computeSnapshot( renderer );
> +                computeSnapshot();
>               }
>           }
>           else if ( t.type() == ::VLC::MediaTrack::Type::Audio )
> @@ -102,25 +107,26 @@ VLCSource::isParsed() const
>   }
>
>   bool
> -VLCSource::computeSnapshot( VmemRenderer& renderer )
> +VLCSource::computeSnapshot()
>   {
>       Q_ASSERT( m_snapshot == nullptr );
> -    renderer.start();
> +    m_vmemRenderer = new VmemRenderer( m_backend, this, nullptr );
> +    m_vmemRenderer->start();
>       {
>           QMutex mutex;
>           QWaitCondition cond;
> -        auto em = renderer.mediaPlayer().eventManager();
> +        auto em = m_vmemRenderer->mediaPlayer().eventManager();
>           em.onPositionChanged([&mutex, &cond](float pos) {
>               QMutexLocker lock( &mutex );
>               if ( pos > 0.2 )
>                   cond.wakeAll();
>           });
>           QMutexLocker lock( &mutex );
> -        renderer.setPosition( 0.3 );
> +        m_vmemRenderer->setPosition( 0.3 );
>           if ( cond.wait( &mutex, 2000 ) == false )
>               return false;
>       }
> -    m_snapshot = renderer.waitSnapshot();
> +    m_snapshot = m_vmemRenderer->waitSnapshot();
>       return m_snapshot != nullptr;
>   }
>
> diff --git a/src/Backend/VLC/VLCSource.h b/src/Backend/VLC/VLCSource.h
> index 10a7d08..346adbe 100644
> --- a/src/Backend/VLC/VLCSource.h
> +++ b/src/Backend/VLC/VLCSource.h
> @@ -40,6 +40,7 @@ class VLCSource : public ISource
>   {
>   public:
>       VLCSource( VLCBackend* backend, const QString& path );
> +    virtual ~VLCSource();
>       virtual ISourceRenderer*    createRenderer( ISourceRendererEventCb* callback );
>       virtual bool                preparse();
>       virtual bool                isParsed() const;
> @@ -58,7 +59,7 @@ public:
>       ::VLC::Media&               media();
>
>   private:
> -    bool                        computeSnapshot( VmemRenderer& renderer );
> +    bool                        computeSnapshot();
>
>   private:
>       VLCBackend*                 m_backend;
> @@ -72,6 +73,7 @@ private:
>       uint8_t*                    m_snapshot;
>       bool                        m_isParsed;
>       int64_t                     m_nbFrames;
> +    VmemRenderer*               m_vmemRenderer;
>   };
>
>
>
The renderer will stop the VLC::MediaPlayer upon destruction, so that 
shouldn't be a problem. Moreover, the waitSnapshot method ensure we 
block until the snapshot is computed, so the timing shouldn't be an issue.
Did you encounter an issue related to this code?