[vlmc-devel] [PATCH 2/4] VmemRenderer: Use shared_ptr to manage m_snapshot

[Hugo Beauzée-Luyssen]

On 04/12/2016 01:10 PM, Yikai Lu wrote:
> ---
>   src/Backend/VLC/VLCVmemRenderer.cpp | 9 ++++-----
>   src/Backend/VLC/VLCVmemRenderer.h   | 4 +++-
>   2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/Backend/VLC/VLCVmemRenderer.cpp b/src/Backend/VLC/VLCVmemRenderer.cpp
> index d062bbe..65c3430 100644
> --- a/src/Backend/VLC/VLCVmemRenderer.cpp
> +++ b/src/Backend/VLC/VLCVmemRenderer.cpp
> @@ -59,7 +59,6 @@ VmemRenderer::~VmemRenderer()
>        * destroyed in a potentially locked state, while the vmem tries to lock/unlock.
>        */
>       stop();
> -    delete[] m_snapshot;
>   }
>
>   ::VLC::MediaPlayer&
> @@ -75,16 +74,16 @@ VmemRenderer::waitSnapshot()
>       m_snapshotRequired = true;
>       if ( m_waitCond.wait( &m_mutex, 3000 ) == false )
>           return nullptr;
> -    return m_snapshot;
> +    return m_snapshot.get();

If you use a shared_ptr, might as well return the shared_ptr here.
Returning the raw pointer loses the ability to convey the ownership 
information.
If the buffer should be owned solely by the VmemRenderer, then use a 
unique_ptr, and return the buffer preferably as a const uint8_t*

In any case, this class will be removed when the medialibrary is merged, 
so I wouldn't waste too much time on it.

>   }
>
>   void*
>   VmemRenderer::vmemLock( void **planes)
>   {
>       QMutexLocker lock( &m_mutex );
> -    m_snapshot = new uint8_t[320 * 180 * 4];
> -    *planes = m_snapshot;
> -    return m_snapshot;
> +    m_snapshot = std::shared_ptr<uint8_t>( new uint8_t[320 * 180 * 4] );
> +    *planes = m_snapshot.get();
> +    return nullptr;
>   }
>
>   void
> diff --git a/src/Backend/VLC/VLCVmemRenderer.h b/src/Backend/VLC/VLCVmemRenderer.h
> index 89b105d..de79410 100644
> --- a/src/Backend/VLC/VLCVmemRenderer.h
> +++ b/src/Backend/VLC/VLCVmemRenderer.h
> @@ -23,6 +23,7 @@
>   #ifndef VLCVMEMRENDERER_H
>   #define VLCVMEMRENDERER_H
>
> +#include <memory>
>   #include <QMutex>
>   #include <QWaitCondition>
>
> @@ -54,7 +55,8 @@ private:
>       void     vmemUnlock( void* picture );
>
>   private:
> -    uint8_t*        m_snapshot;
> +    std::shared_ptr<uint8_t>        m_snapshot;
> +
>       bool            m_snapshotRequired;
>       QMutex          m_mutex;
>       QWaitCondition  m_waitCond;
>