[ #30520 ] Be even more carefull to prevent deaklock and races

src/call.cpp

@@ -881,6 +881,7 @@ void Call::nothing()
 void Call::error()
 {
    if (videoRenderer()) {
+      //Well, in this case we have no choice, it still doesn't belong here
       videoRenderer()->stopRendering();
    }
    throw QString("There was an error handling your call, please restart SFLPhone.Is you encounter this problem often, \
@@ -950,7 +951,7 @@ void Call::hangUp()
    m_pStopTimeStamp = curTime;
    qDebug() << "Hanging up call. callId : " << m_CallId << "ConfId:" << m_ConfId;
    bool ret;
-   if (videoRenderer()) {
+   if (videoRenderer()) { //TODO remove, cheap hack
       videoRenderer()->stopRendering();
    }
    if (!isConference())
@@ -1091,7 +1092,7 @@ void Call::startStop()
 void Call::stop()
 {
    qDebug() << "Stoping call. callId : " << m_CallId  << "ConfId:" << m_ConfId;
-   if (videoRenderer()) {
+   if (videoRenderer()) { //TODO remove, cheap hack
       videoRenderer()->stopRendering();
    }
    time_t curTime;

src/videomodel.cpp

@@ -28,7 +28,7 @@
 VideoModel* VideoModel::m_spInstance = nullptr;
 
 ///Constructor
-VideoModel::VideoModel():QObject(),m_BufferSize(0),m_ShmKey(0),m_SemKey(0),m_PreviewState(false),m_Thread(this)
+VideoModel::VideoModel():QThread(),m_BufferSize(0),m_ShmKey(0),m_SemKey(0),m_PreviewState(false)
 {
    VideoInterface& interface = DBus::VideoManager::instance();
    connect( &interface , SIGNAL(deviceEvent())                           , this, SLOT(deviceEvent())                           );
@@ -104,15 +104,15 @@ void VideoModel::startedDecoding(const QString& id, const QString& shmPath, int
 
    if (m_lRenderers[id] == nullptr ) {
       m_lRenderers[id] = new VideoRenderer(id,shmPath,Resolution(width,height));
-      m_lRenderers[id]->moveToThread(&m_Thread);
+      m_lRenderers[id]->moveToThread(this);
+      if (!isRunning())
+         start();
    }
    else {
       VideoRenderer* renderer = m_lRenderers[id];
       renderer->setShmPath(shmPath);
       renderer->setResolution(QSize(width,height));
    }
-   if (!m_Thread.isRunning())
-      m_Thread.start();
 
    m_lRenderers[id]->startRendering();
    if (id != "local") {
@@ -129,8 +129,13 @@ void VideoModel::stoppedDecoding(const QString& id, const QString& shmPath)
    if ( r ) {
       r->stopRendering();
    }
+   qDebug() << "Video stopped for call" << id <<  "Renderer found:" << (m_lRenderers[id] != nullptr);
    m_lRenderers[id] = nullptr;
    delete r;
-   qDebug() << "Video stopped for call" << id <<  "Renderer found:" << (m_lRenderers[id] != nullptr);
    emit videoStopped();
 }
+
+void VideoModel::run()
+{
+   exec();
+}

src/videomodel.h

@@ -31,7 +31,7 @@ class Call;
 struct SHMHeader;
 
 ///VideoModel: Video event dispatcher
-class LIB_EXPORT VideoModel : public QObject {
+class LIB_EXPORT VideoModel : public QThread {
    #pragma GCC diagnostic push
    #pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant"
    Q_OBJECT
@@ -48,6 +48,9 @@ public:
    //Setters
    void       setBufferSize(uint size);
 
+protected:
+   void run();
+
 private:
    //Constructor
    VideoModel();
@@ -60,7 +63,6 @@ private:
    uint           m_BufferSize  ;
    uint           m_ShmKey      ;
    uint           m_SemKey      ;
-   QThread        m_Thread      ;
    QHash<QString,VideoRenderer*> m_lRenderers;
 
 public Q_SLOTS:

src/videorenderer.cpp

@@ -71,15 +71,14 @@ VideoRenderer::~VideoRenderer()
 }
 
 ///Get the data from shared memory and transform it into a QByteArray
-QByteArray VideoRenderer::renderToBitmap(QByteArray& data,bool& ok)
+bool VideoRenderer::renderToBitmap(QByteArray& data)
 {
    if (!m_isRendering) {
-      return QByteArray();
+      return false;
    }
 
    if (!shmLock()) {
-      ok = false;
-      return QByteArray();
+      return false;
    }
 
    // wait for a new buffer
@@ -111,20 +110,17 @@ QByteArray VideoRenderer::renderToBitmap(QByteArray& data,bool& ok)
 //             break;
 //       }
       if (err < 0) {
-         ok = false;
-         return QByteArray();
+         return false;
       }
 
       if (!shmLock()) {
-         ok = false;
-         return QByteArray();
+         return false;
       }
    }
 
    if (!resizeShm()) {
       qDebug() << "Could not resize shared memory";
-      ok = false;
-      return QByteArray();
+      return false;
    }
 
    if (data.size() != m_pShmArea->m_BufferSize)
@@ -132,7 +128,8 @@ QByteArray VideoRenderer::renderToBitmap(QByteArray& data,bool& ok)
    memcpy(data.data(),m_pShmArea->m_Data,m_pShmArea->m_BufferSize);
    m_BufferGen = m_pShmArea->m_BufferGen;
    shmUnlock();
-   return data;
+//    return data;
+   return true;
 }
 
 ///Connect to the shared memory
@@ -243,9 +240,9 @@ timespec VideoRenderer::createTimeout()
 ///Update the buffer
 void VideoRenderer::timedEvents()
 {
-   bool ok = true;
    m_pMutex->lock();
-   renderToBitmap(m_Frame,ok);
+
+   bool ok = renderToBitmap(m_Frame);
    m_pMutex->unlock();
    if (ok == true) {
       emit frameUpdated();
@@ -294,7 +291,7 @@ void VideoRenderer::stopRendering()
 ///Get the raw bytes directly from the SHM, not recommended, but optimal
 const char* VideoRenderer::rawData()
 {
-   return m_isRendering?m_pShmArea->m_Data:nullptr;
+   return m_isRendering?m_Frame.data():nullptr;
 }
 
 ///Is this redenrer active

src/videorenderer.h

@@ -48,7 +48,6 @@ class LIB_EXPORT VideoRenderer : public QObject {
       bool startShm ();
 
       //Getters
-      QByteArray  renderToBitmap(QByteArray& data, bool& ok);
       const char* rawData         ();
       bool        isRendering     ();
       QByteArray  currentFrame    ();
@@ -82,6 +81,7 @@ class LIB_EXPORT VideoRenderer : public QObject {
       timespec createTimeout();
       bool     shmLock      ();
       void     shmUnlock    ();
+      bool     renderToBitmap(QByteArray& data);
 
    private Q_SLOTS:
       void timedEvents();