From 4909f3fc4a1e56952288e5c976e3460097b6375c Mon Sep 17 00:00:00 2001
From: Stepan Salenikovich <stepan.salenikovich@savoirfairelinux.com>
Date: Wed, 11 May 2016 17:55:59 -0400
Subject: [PATCH] videorenderermanager: fix renderer lifecycle

The decodingStopped signal is recevied before the call
is in the OVER state most of the time, since its possible
for decoding to start and stop. Thus, once decoding has
stopped, we wait until the call is over to destroy the
renderer.

Change-Id: I440f33a843f657cd9b70ee715a72cae7494a8921
Tuleap: #644
---
 src/private/videorenderermanager.cpp | 113 +++++++++++++++------------
 1 file changed, 64 insertions(+), 49 deletions(-)

diff --git a/src/private/videorenderermanager.cpp b/src/private/videorenderermanager.cpp
index 75e67de7..d1523d61 100644
--- a/src/private/videorenderermanager.cpp
+++ b/src/private/videorenderermanager.cpp
@@ -62,13 +62,13 @@ public:
    //Helper
    void removeRenderer(Video::Renderer* r);
 
-
 private:
    VideoRendererManager* q_ptr;
 
 public Q_SLOTS:
    void startedDecoding(const QString& id, const QString& shmPath, int width, int height);
    void stoppedDecoding(const QString& id, const QString& shmPath);
+   void callIsOver();
 
 };
 
@@ -227,9 +227,9 @@ void VideoRendererManagerPrivate::startedDecoding(const QString& id, const QStri
 
    }
    else {
-      r = m_hRenderers[rid];
+      r = m_hRenderers.value(rid);
 
-      QThread* t = m_hThreads[r];
+      QThread* t = m_hThreads.value(r);
 
       if (t && !t->isRunning())
          t->start();
@@ -280,72 +280,87 @@ void VideoRendererManagerPrivate::startedDecoding(const QString& id, const QStri
    }
 }
 
-/**
- * @warning This method can be called multiple time for the same renderer
- */
+/// Deletes the renderer and related resources
 void VideoRendererManagerPrivate::removeRenderer(Video::Renderer* r)
 {
-   if (!r || !m_hRenderers.contains(m_hRendererIds[r]))
-      return;
+    const auto id = m_hRendererIds.value(r);
+    auto t = m_hThreads.value(r);
 
-   const QByteArray id = m_hRendererIds[r];
+    m_hRendererIds.remove(r);
+    m_hRenderers.remove(id);
+    m_hThreads.remove(r);
 
-   //Quit if for some reasons the renderer is not found
-   if ( !r ) {
-      qWarning() << "Cannot stop rendering, renderer" << id << "not found";
-      return;
-   }
+    if (t) {
+        t->deleteLater();
+    }
 
-   Call* c = CallModel::instance().getCall(id);
+    r->deleteLater();
+}
 
-   if (c && c->lifeCycleState() == Call::LifeCycleState::FINISHED) {
-      c->d_ptr->removeRenderer(r);
-   }
+/**
+ * A video stopped being rendered
+ *
+ * @warning This method can be called multiple time for the same renderer
+ */
+void VideoRendererManagerPrivate::stoppedDecoding(const QString& id, const QString& shmPath)
+{
+    Q_UNUSED(shmPath)
 
-   r->stopRendering();
+    if (!m_hRenderers.contains(id.toLatin1()) || !m_hRenderers.contains(id.toLatin1())) {
+        qWarning() << "Cannot stop decoding, renderer" << id << "not found";
+        return; // nothing to do
+    }
 
-   qDebug() << "Video stopped for call" << id <<  "Renderer found:" << m_hRenderers.contains(id);
+    auto r = m_hRenderers.value(id.toLatin1());
 
-   Video::Device* dev = Video::DeviceModel::instance().getDevice(id);
+    Call* c = CallModel::instance().getCall(id);
 
-   if (dev)
-      emit dev->renderingStopped(r);
+    // TODO: the current implementeation of CallPrivate::removeRenderer() does nothing
+    if (c && c->lifeCycleState() == Call::LifeCycleState::FINISHED) {
+        c->d_ptr->removeRenderer(r);
+    }
 
-   if (id == PREVIEW_RENDERER_ID) {
-      m_PreviewState = false;
-      emit q_ptr->previewStateChanged(false);
-      emit q_ptr->previewStopped(r);
-   }
+    r->stopRendering();
 
-   QThread* t = m_hThreads[r];
+    qDebug() << "Video stopped for call" << id <<  "Renderer found:" << m_hRenderers.contains(id.toLatin1());
 
-   if (t) {
-       t->quit();
-       t->wait();
-   }
+    Video::Device* dev = Video::DeviceModel::instance().getDevice(id);
 
-   if (c && c->lifeCycleState() == Call::LifeCycleState::FINISHED) {
+    if (dev)
+        emit dev->renderingStopped(r);
 
-       m_hRendererIds.remove(r);
-       m_hRenderers.remove(id);
+    if (id == PREVIEW_RENDERER_ID) {
+        m_PreviewState = false;
+        emit q_ptr->previewStateChanged(false);
+        emit q_ptr->previewStopped(r);
+    }
 
-       m_hThreads[r] = nullptr;
-       if (t) {
-           t->deleteLater();
-       }
+    QThread* t = m_hThreads.value(r);
 
-       r->deleteLater();
-   }
+    if (t) {
+        t->quit();
+        t->wait();
+    }
+
+    // decoding stopped; remove the renderer, if/when call is over
+    if (c && c->lifeCycleState() == Call::LifeCycleState::FINISHED) {
+        removeRenderer(r);
+    } else if (c) {
+        connect(c, &Call::isOver, this, &VideoRendererManagerPrivate::callIsOver);
+    }
 }
 
-///A video stopped being rendered
-void VideoRendererManagerPrivate::stoppedDecoding(const QString& id, const QString& shmPath)
+void VideoRendererManagerPrivate::callIsOver()
 {
-   Q_UNUSED(shmPath)
-
-   if (m_hRenderers.contains(id.toLatin1())) {
-      removeRenderer(m_hRenderers[id.toLatin1()]);
-   }
+    if (auto call = qobject_cast<Call *>(sender())) {
+        if (auto r = m_hRenderers.value(call->dringId().toLatin1()))
+            removeRenderer(r);
+        else
+            qDebug() << "Could not delete renderer, it might have already been removed:" << call->dringId();
+
+        // remove the connection from this call to this
+        disconnect(call, &Call::isOver, this, 0);
+    }
 }
 
 void VideoRendererManager::switchDevice(const Video::Device* device) const
-- 
GitLab