From 7005452a235574f35af7e05a106d8fc0a1860c79 Mon Sep 17 00:00:00 2001
From: Emmanuel Lepage Vallee <emmanuel.lepage@savoirfairelinux.com>
Date: Fri, 10 Apr 2015 10:17:23 -0400
Subject: [PATCH] video: Fix DirectRenderer

Also restore the APIs that has been broken in the previous commit.
I made a minor change in the API to remove 2 setters for technically
read-only variables. Using them would have caused a SEGSIGV.

Refs #70057
---
 src/private/directrenderer.cpp       | 11 +++----
 src/private/shmrenderer.cpp          | 48 +++++++++++-----------------
 src/private/shmrenderer.h            |  4 ++-
 src/private/videorenderer_p.h        |  4 ++-
 src/private/videorenderermanager.cpp |  1 +
 src/private/videorenderermanager.h   |  3 +-
 src/video/renderer.cpp               | 25 ++++++---------
 src/video/renderer.h                 | 24 +++++++-------
 8 files changed, 54 insertions(+), 66 deletions(-)

diff --git a/src/private/directrenderer.cpp b/src/private/directrenderer.cpp
index a5a2d78d..8f66e798 100644
--- a/src/private/directrenderer.cpp
+++ b/src/private/directrenderer.cpp
@@ -69,26 +69,23 @@ void Video::DirectRenderer::startRendering()
 {
 
     qWarning() << "STARTING RENDERING";
-    setRendering(true);
+    Video::Renderer::d_ptr->m_isRendering = true;
     emit started();
 }
 void Video::DirectRenderer::stopRendering ()
 {
     qWarning() << "STOPPING RENDERING";
-    setRendering(false);
+    Video::Renderer::d_ptr->m_isRendering = false;
     emit stopped();
 }
 
 void Video::DirectRenderer::onNewFrame(const QByteArray& frame)
 {
-//    qDebug("Frame received by DirectRenderer, size: w %d, h %d", size().width(), size().height());
     if (!isRendering()) {
        return;
     }
-    if (static_cast<Video::Renderer*>(this)->d_ptr->otherFrame().size() != (size().height() * size().width()))
-    static_cast<Video::Renderer*>(this)->d_ptr->otherFrame().resize(size().height() * size().width()*4);
-    ::memcpy(static_cast<Video::Renderer*>(this)->d_ptr->otherFrame().data(),frame,static_cast<Video::Renderer*>(this)->d_ptr->otherFrame().size());
-    static_cast<Video::Renderer*>(this)->d_ptr->updateFrameIndex();
+
+    Video::Renderer::d_ptr->m_framePtr = const_cast<char*>(frame.data());
     emit frameUpdated();
 }
 
diff --git a/src/private/shmrenderer.cpp b/src/private/shmrenderer.cpp
index d386fbd3..1fd15e6a 100644
--- a/src/private/shmrenderer.cpp
+++ b/src/private/shmrenderer.cpp
@@ -82,7 +82,7 @@ class ShmRendererPrivate : public QObject
         int               m_fd         ;
         SHMHeader*        m_pShmArea   ;
         unsigned          m_ShmAreaLen ;
-        uint              m_FrameGen  ;
+        uint              m_FrameGen   ;
         QTimer*           m_pTimer     ;
         int               m_fpsC       ;
         int               m_Fps        ;
@@ -137,8 +137,7 @@ ShmRenderer::~ShmRenderer()
 }
 
 /// Wait new frame data from shared memory and save pointer
-bool
-ShmRendererPrivate::getNewFrame(bool wait)
+bool ShmRendererPrivate::getNewFrame(bool wait)
 {
     if (!shmLock())
         return false;
@@ -159,7 +158,7 @@ ShmRendererPrivate::getNewFrame(bool wait)
     }
 
     // valid frame to render (daemon may have stopped)?
-    if (not m_pShmArea->frameSize) {
+    if (! m_pShmArea->frameSize) {
         shmUnlock();
         return false;
     }
@@ -170,8 +169,9 @@ ShmRendererPrivate::getNewFrame(bool wait)
         return false;
     }
 
-    q_ptr->setFramePtr(m_pShmArea->data + m_pShmArea->readOffset);
+    q_ptr->Video::Renderer::d_ptr->m_framePtr = (char*)(m_pShmArea->data + m_pShmArea->readOffset);
     m_FrameGen = m_pShmArea->frameGen;
+    q_ptr->Video::Renderer::d_ptr->m_FrameSize = m_pShmArea->frameSize;
 
     shmUnlock();
 
@@ -195,8 +195,7 @@ ShmRendererPrivate::getNewFrame(bool wait)
 
 /// Remap the shared memory
 /// Shared memory in unlocked state if returns false (resize failed).
-bool
-ShmRendererPrivate::remapShm()
+bool ShmRendererPrivate::remapShm()
 {
     // This loop handles case where deamon resize shared memory
     // during time we unlock it for remapping.
@@ -226,8 +225,7 @@ ShmRendererPrivate::remapShm()
 }
 
 /// Connect to the shared memory
-bool
-ShmRenderer::startShm()
+bool ShmRenderer::startShm()
 {
    if (d_ptr->m_fd != -1) {
       qDebug() << "fd must be -1";
@@ -256,8 +254,7 @@ ShmRenderer::startShm()
 }
 
 /// Disconnect from the shared memory
-void
-ShmRenderer::stopShm()
+void ShmRenderer::stopShm()
 {
    if (d_ptr->m_fd < 0)
        return;
@@ -274,15 +271,13 @@ ShmRenderer::stopShm()
 }
 
 /// Lock the memory while the copy is being made
-bool
-ShmRendererPrivate::shmLock()
+bool ShmRendererPrivate::shmLock()
 {
     return ::sem_wait(&m_pShmArea->mutex) >= 0;
 }
 
 /// Remove the lock, allow a new frame to be drawn
-void
-ShmRendererPrivate::shmUnlock()
+void ShmRendererPrivate::shmUnlock()
 {
     ::sem_post(&m_pShmArea->mutex);
 }
@@ -294,24 +289,22 @@ ShmRendererPrivate::shmUnlock()
  ****************************************************************************/
 
 /// Start the rendering loop
-void
-ShmRenderer::startRendering()
+void ShmRenderer::startRendering()
 {
     QMutexLocker locker {mutex()};
 
     if (!startShm())
         return;
 
-    setRendering(true);
+    Video::Renderer::d_ptr->m_isRendering = true;
     emit started();
 }
 
 /// Stop the rendering loop
-void
-ShmRenderer::stopRendering()
+void ShmRenderer::stopRendering()
 {
     QMutexLocker locker {mutex()};
-    setRendering(false);
+    Video::Renderer::d_ptr->m_isRendering = false;
 
     emit stopped();
     stopShm();
@@ -324,22 +317,20 @@ ShmRenderer::stopRendering()
  ****************************************************************************/
 
 /// Get the current frame rate of this renderer
-int
-ShmRenderer::fps() const
+int ShmRenderer::fps() const
 {
     return d_ptr->m_Fps;
 }
 
 /// Get frame data pointer from shared memory
-void*
-ShmRenderer::getFramePtr() const
+const QByteArray& ShmRenderer::currentFrame() const
 {
     if (!isRendering())
-        return nullptr;
+        return {};
 
     QMutexLocker lk {mutex()};
     d_ptr->getNewFrame(false);
-    return Renderer::getFramePtr();
+    return Renderer::currentFrame();
 }
 
 /*****************************************************************************
@@ -348,8 +339,7 @@ ShmRenderer::getFramePtr() const
  *                                                                           *
  ****************************************************************************/
 
-void
-ShmRenderer::setShmPath(const QString& path)
+void ShmRenderer::setShmPath(const QString& path)
 {
     d_ptr->m_ShmPath = path;
 }
diff --git a/src/private/shmrenderer.h b/src/private/shmrenderer.h
index e6074d27..6064f090 100644
--- a/src/private/shmrenderer.h
+++ b/src/private/shmrenderer.h
@@ -42,6 +42,8 @@ class LIB_EXPORT ShmRenderer : public Renderer {
    Q_OBJECT
    #pragma GCC diagnostic pop
 
+   friend class VideoRendererManagerPrivate ;
+
    public:
       //Constructor
       ShmRenderer (const QByteArray& id, const QString& shmPath, const QSize& res);
@@ -53,7 +55,7 @@ class LIB_EXPORT ShmRenderer : public Renderer {
 
       //Getters
       virtual int   fps() const;
-      virtual void* getFramePtr() const;
+      virtual const QByteArray& currentFrame() const override;
 
       //Setters
       void setShmPath(const QString& path);
diff --git a/src/private/videorenderer_p.h b/src/private/videorenderer_p.h
index dc88330b..a18d09df 100644
--- a/src/private/videorenderer_p.h
+++ b/src/private/videorenderer_p.h
@@ -41,7 +41,9 @@ public:
    QMutex*           m_pMutex     ;
    QString           m_Id         ;
    QSize             m_pSize      ;
-   void*             m_framePtr   ;
+   char*             m_framePtr   ;
+   QByteArray        m_Content    ;
+   unsigned int      m_FrameSize  ;
 
 private:
    Video::Renderer* q_ptr;
diff --git a/src/private/videorenderermanager.cpp b/src/private/videorenderermanager.cpp
index 602404cf..309beb52 100644
--- a/src/private/videorenderermanager.cpp
+++ b/src/private/videorenderermanager.cpp
@@ -36,6 +36,7 @@
 #include <video/resolution.h>
 #include "private/videorate_p.h"
 #include "private/call_p.h"
+#include "private/videorenderer_p.h"
 
 #ifdef ENABLE_LIBWRAP
  #include "private/directrenderer.h"
diff --git a/src/private/videorenderermanager.h b/src/private/videorenderermanager.h
index 9e0edb32..2cf14aed 100644
--- a/src/private/videorenderermanager.h
+++ b/src/private/videorenderermanager.h
@@ -37,7 +37,8 @@ struct SHMHeader;
 class VideoRendererManagerPrivate;
 
 ///VideoModel: Video event dispatcher
-class VideoRendererManager : public QObject {
+class VideoRendererManager : public QObject
+{
    #pragma GCC diagnostic push
    #pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant"
    Q_OBJECT
diff --git a/src/video/renderer.cpp b/src/video/renderer.cpp
index 575caa87..31ac5a8c 100644
--- a/src/video/renderer.cpp
+++ b/src/video/renderer.cpp
@@ -25,19 +25,22 @@
 
 Video::RendererPrivate::RendererPrivate(Video::Renderer* parent)
     : QObject(parent), q_ptr(parent)
-    , m_pMutex(new QMutex())
+    , m_pMutex(new QMutex()),m_framePtr(nullptr),m_FrameSize(0)
 {
 }
 
 Video::Renderer::Renderer(const QByteArray& id, const QSize& res) : d_ptr(new RendererPrivate(this))
 {
    setObjectName("Renderer:"+id);
+   d_ptr->m_FrameSize = res.width() * res.height() * 4;
    d_ptr->m_pSize = res;
    d_ptr->m_Id = id;
 }
 
 Video::Renderer::~Renderer()
-{}
+{
+   delete d_ptr;
+}
 
 /*****************************************************************************
 *                                                                           *
@@ -45,6 +48,7 @@ Video::Renderer::~Renderer()
 *                                                                           *
 ****************************************************************************/
 
+///Return if the rendering is currently active or not
 bool Video::Renderer::isRendering() const
 {
   return d_ptr->m_isRendering;
@@ -62,9 +66,11 @@ QSize Video::Renderer::size() const
   return d_ptr->m_pSize;
 }
 
-void* Video::Renderer::getFramePtr() const
+const QByteArray& Video::Renderer::currentFrame() const
 {
-    return d_ptr->m_framePtr;
+   if (d_ptr->m_framePtr && d_ptr->m_FrameSize)
+      d_ptr->m_Content.setRawData(d_ptr->m_framePtr,d_ptr->m_FrameSize);
+   return d_ptr->m_Content;
 }
 
 /*****************************************************************************
@@ -73,20 +79,9 @@ void* Video::Renderer::getFramePtr() const
  *                                                                           *
  ****************************************************************************/
 
-///Return the current resolution
-void Video::Renderer::setRendering(bool rendering) const
-{
-  d_ptr->m_isRendering = rendering;
-}
-
 void Video::Renderer::setSize(const QSize& size) const
 {
   d_ptr->m_pSize = size;
 }
 
-void Video::Renderer::setFramePtr(void* ptr) const
-{
-  d_ptr->m_framePtr = ptr;
-}
-
 #include <renderer.moc>
diff --git a/src/video/renderer.h b/src/video/renderer.h
index cd38e0ab..9541d334 100644
--- a/src/video/renderer.h
+++ b/src/video/renderer.h
@@ -33,6 +33,7 @@ namespace Video {
 
 class RendererPrivate;
 class ShmRendererPrivate;
+class ShmRenderer;
 class DirectRendererPrivate;
 class DirectRenderer;
 
@@ -43,15 +44,18 @@ class DirectRenderer;
  *
  * Each platform transparently provide its own implementation.
  */
-class LIB_EXPORT Renderer : public QObject {
+class LIB_EXPORT Renderer : public QObject
+{
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant"
-Q_OBJECT
+   Q_OBJECT
 #pragma GCC diagnostic pop
 
-friend class Video::ShmRendererPrivate;
-friend class Video::DirectRendererPrivate;
-friend class Video::DirectRenderer;
+   friend class Video::ShmRendererPrivate   ;
+   friend class Video::ShmRenderer          ;
+   friend class Video::DirectRendererPrivate;
+   friend class Video::DirectRenderer       ;
+   friend class VideoRendererManagerPrivate ;
 
 public:
    //Constructor
@@ -60,14 +64,11 @@ public:
 
    //Getters
    virtual bool              isRendering     () const;
+   virtual const QByteArray& currentFrame    () const;
    virtual QSize             size            () const;
    virtual QMutex*           mutex           () const;
-   virtual void*             getFramePtr     () const;
 
-   //Setters
-   void setRendering(bool rendering)            const;
-   void setSize(const QSize& size)              const;
-   void setFramePtr(void* ptr)                  const;
+   void setSize(const QSize& size) const;
 
 Q_SIGNALS:
    void frameUpdated(); // Emitted when a new frame is ready
@@ -78,9 +79,8 @@ public Q_SLOTS:
    virtual void startRendering() = 0;
    virtual void stopRendering () = 0;
 
-
 private:
-   QScopedPointer<RendererPrivate> d_ptr;
+   RendererPrivate* d_ptr;
    Q_DECLARE_PRIVATE(Renderer)
 
 };
-- 
GitLab