refactor(calls): make chatform sole owner of ToxCalls

anthonybilinski · anthonybilinski · commit 9a5d225922de · 2021-01-19T09:34:12.000-08:00
* Keep ToxCall creation on call invite, since it tracks call state changes
* Fix ending call in ChatForm after CoreAv ends call from remote
* Pass ToxCall up on invite instead of answer, so that ownership is the same
before or after answer
* Do nothing if CoreAv tries to cancel call for non-existent call, to avoid
loop of CoreAv ending call, causing ChatForm to reset, causing ToxCall to
cancel call with CoreAv
diff --git a/src/core/coreav.cpp b/src/core/coreav.cpp
@@ -92,6 +92,7 @@ CoreAV::CoreAV(std::unique_ptr<ToxAV, ToxAVDeleter> toxav, QMutex& toxCoreLock,
     connect(iterateTimer, &QTimer::timeout, this, &CoreAV::process);
     connect(coreavThread.get(), &QThread::finished, iterateTimer, &QTimer::stop);
     connect(coreavThread.get(), &QThread::started, this, &CoreAV::process);
+    qRegisterMetaType<ToxFriendCallPtr>("ToxFriendCallPtr");
 }
 
 void CoreAV::connectCallbacks(ToxAV& toxav)
@@ -201,7 +202,7 @@ bool CoreAV::isCallActive(const Group* g) const
     return it->second->isActive();
 }
 
-CoreAV::ToxFriendCallPtr CoreAV::answerCall(uint32_t friendNum, bool video)
+bool CoreAV::answerCall(uint32_t friendNum, bool video)
 {
     QWriteLocker locker{&callsLock};
     QMutexLocker coreLocker{&coreLock};
@@ -215,12 +216,11 @@ CoreAV::ToxFriendCallPtr CoreAV::answerCall(uint32_t friendNum, bool video)
     if (toxav_answer(toxav.get(), friendNum, audioSettings.getAudioBitrate(),
                      videoBitrate, &err)) {
         it->second->setActive(true);
-        return it->second;
+        return true;
     } else {
         qWarning() << "Failed to answer call with error" << err;
         toxav_call_control(toxav.get(), friendNum, TOXAV_CALL_CONTROL_CANCEL, nullptr);
-        calls.erase(it);
-        return {};
+        return false;
     }
 }
 
@@ -247,7 +247,7 @@ CoreAV::ToxFriendCallPtr CoreAV::startCall(uint32_t friendNum, bool video)
     // Call object must be owned by this thread or there will be locking problems with Audio
     call->moveToThread(this->thread());
     assert(call != nullptr);
-    calls.emplace(friendNum, call);
+    calls.emplace(friendNum, call.get());
     return call;
 }
 
@@ -258,7 +258,6 @@ bool CoreAV::cancelCall(uint32_t friendNum)
 
     auto it = calls.find(friendNum);
     if (it == calls.end()) {
-        qWarning() << QString("Can't cancel call with %1, it doesn't exist!").arg(friendNum);
         return false;
     }
 
@@ -533,7 +532,7 @@ void CoreAV::callCallback(ToxAV* toxav, uint32_t friendNum, bool audio, bool vid
     // Must explicitely unlock, because a deadlock can happen via ChatForm/Audio
     locker.unlock();
 
-    emit self->avInvite(friendNum, video);
+    emit self->avInvite(friendNum, video, call);
 }
 
 void CoreAV::stateCallback(ToxAV* toxav, uint32_t friendNum, uint32_t state, void* vSelf)
diff --git a/src/core/coreav.h b/src/core/coreav.h
@@ -23,6 +23,7 @@
 #include "src/core/toxcall.h"
 
 #include <QObject>
+#include <QMetaType>
 #include <QMutex>
 #include <QReadWriteLock>
 #include <atomic>
@@ -43,14 +44,17 @@ class VideoFrame;
 class Core;
 struct vpx_image;
 
+Q_DECLARE_METATYPE(std::shared_ptr<ToxFriendCall>)
+
 class CoreAV : public QObject
 {
     Q_OBJECT
 
 public:
     using CoreAVPtr = std::unique_ptr<CoreAV>;
+    // must be shared for signal avInvites signal, even though ownership is not shared
     using ToxFriendCallPtr = std::shared_ptr<ToxFriendCall>;
-    using ToxGroupCallPtr = std::shared_ptr<ToxGroupCall>;
+    using ToxGroupCallPtr = std::unique_ptr<ToxGroupCall>;
 
     static CoreAVPtr makeCoreAV(Tox* core, QMutex& toxCoreLock,
                                 IAudioSettings& audioSettings, IGroupSettings& groupSettings);
@@ -78,12 +82,12 @@ class CoreAV : public QObject
 
 public slots:
     ToxFriendCallPtr startCall(uint32_t friendNum, bool video);
-    ToxFriendCallPtr answerCall(uint32_t friendNum, bool video);
+    bool answerCall(uint32_t friendNum, bool video);
     bool cancelCall(uint32_t friendNum);
     void start();
 
 signals:
-    void avInvite(uint32_t friendId, bool video);
+    void avInvite(uint32_t friendId, bool video, ToxFriendCallPtr call);
     void avStart(uint32_t friendId, bool video);
     void avEnd(uint32_t friendId, bool error = false);
 
@@ -129,13 +133,13 @@ private slots:
      * @brief Maps friend IDs to ToxFriendCall.
      * @note Need to use STL container here, because Qt containers need a copy constructor.
      */
-    std::map<uint32_t, ToxFriendCallPtr> calls;
+    std::map<uint32_t, ToxFriendCall*> calls;
 
     /**
      * @brief Maps group IDs to ToxGroupCalls.
      * @note Need to use STL container here, because Qt containers need a copy constructor.
      */
-    std::map<int, ToxGroupCallPtr> groupCalls;
+    std::map<int, ToxGroupCall*> groupCalls;
 
     // protect 'calls' and 'groupCalls'
     mutable QReadWriteLock callsLock{QReadWriteLock::Recursive};
diff --git a/src/widget/form/chatform.cpp b/src/widget/form/chatform.cpp
@@ -276,12 +276,19 @@ void ChatForm::onAttachClicked()
     }
 }
 
-void ChatForm::onAvInvite(uint32_t friendId, bool video)
+void ChatForm::onAvInvite(uint32_t friendId, bool video, std::shared_ptr<ToxFriendCall> call)
 {
     if (friendId != f->getId()) {
         return;
     }
 
+    if (this->call) {
+        qWarning() << "ChatForm invited to call when already in one. "
+            "This should have been caught by CoreAv!";
+    }
+
+    this->call = call;
+
     QString displayedName = f->getDisplayedName();
     insertChatMessage(ChatMessage::createChatInfoMessage(tr("%1 calling").arg(displayedName),
                                                          ChatMessage::INFO,
@@ -324,6 +331,7 @@ void ChatForm::onAvEnd(uint32_t friendId, bool error)
         return;
     }
 
+    call.reset();
     headWidget->removeCallConfirm();
     // Fixes an OS X bug with ending a call while in full screen
     if (netcam && netcam->isFullScreen()) {
@@ -354,14 +362,15 @@ void ChatForm::onAnswerCallTriggered(bool video)
     emit acceptCall(friendId);
 
     updateCallButtons();
-    if (call) {
-        qDebug() << "Stale call detected";
+    if (!call) {
+        qWarning() << "Trying to answer call without receiving an invite";
     }
 
     CoreAV* av = core.getAv();
-    call = av->answerCall(friendId, video);
-    if (!call) {
+    bool answered = av->answerCall(friendId, video);
+    if (!answered) {
         qDebug() << "Failed to answer call";
+        call.reset();
         updateCallButtons();
         stopCounter();
         hideNetcam();
diff --git a/src/widget/form/chatform.h b/src/widget/form/chatform.h
@@ -68,7 +68,7 @@ class ChatForm : public GenericChatForm
     void updateFriendActivity(Friend& frnd);
 
 public slots:
-    void onAvInvite(uint32_t friendId, bool video);
+    void onAvInvite(uint32_t friendId, bool video, std::shared_ptr<ToxFriendCall> call);
     void onAvStart(uint32_t friendId, bool video);
     void onAvEnd(uint32_t friendId, bool error);
     void onAvatarChanged(const ToxPk& friendPk, const QPixmap& pic);
diff --git a/src/widget/form/groupchatform.h b/src/widget/form/groupchatform.h
@@ -79,5 +79,5 @@ private slots:
     FlowLayout* namesListLayout;
     QLabel* nusersLabel;
     TabCompleter* tabber;
-    std::shared_ptr<ToxGroupCall> call;
+    std::unique_ptr<ToxGroupCall> call;
 };
