refactor(core): make construction independent of CoreAV

Allows to construct a Core object without also starting CoreAV.

src/core/core.cpp

@@ -19,8 +19,8 @@
 */
 
 #include "core.h"
+#include "coreav.h"
 #include "corefile.h"
-#include "src/core/coreav.h"
 #include "src/core/dhtserver.h"
 #include "src/core/icoresettings.h"
 #include "src/core/toxlogger.h"
@@ -495,7 +495,6 @@ Core::~Core()
     coreThread->exit(0);
     coreThread->wait();
 
-    av.reset();
     tox.reset();
 }
 
@@ -632,17 +631,6 @@ ToxCorePtr Core::makeToxCore(const QByteArray& savedata, const ICoreSettings* co
     // tox should be valid by now
     assert(core->tox != nullptr);
 
-    // toxcore is successfully created, create toxav
-    // TODO(sudden6): don't create CoreAv here, Core should be usable without CoreAV
-    core->av = CoreAV::makeCoreAV(core->tox.get(), core->coreLoopLock);
-    if (!core->av) {
-        qCritical() << "Toxav failed to start";
-        if (err) {
-            *err = ToxCoreErrors::FAILED_TO_START;
-        }
-        return {};
-    }
-
     // create CoreFile
     core->file = CoreFile::makeCoreFile(core.get(), core->tox.get(), core->coreLoopLock);
     if (!core->file) {
@@ -692,8 +680,6 @@ void Core::onStarted()
     loadGroups();
 
     process(); // starts its own timer
-    av->start();
-    emit avReady();
 }
 
 /**
@@ -706,12 +692,17 @@ void Core::start()
 
 const CoreAV* Core::getAv() const
 {
-    return av.get();
+    return av;
 }
 
 CoreAV* Core::getAv()
 {
-    return av.get();
+    return av;
+}
+
+void Core::setAv(CoreAV *coreAv)
+{
+    av = coreAv;
 }
 
 CoreFile* Core::getCoreFile() const
@@ -719,6 +710,16 @@ CoreFile* Core::getCoreFile() const
     return file.get();
 }
 
+Tox* Core::getTox() const
+{
+    return tox.get();
+}
+
+QMutex &Core::getCoreLoopLock() const
+{
+    return coreLoopLock;
+}
+
 /* Using the now commented out statements in checkConnection(), I watched how
  * many ticks disconnects-after-initial-connect lasted. Out of roughly 15 trials,
  * 5 disconnected; 4 were DCd for less than 20 ticks, while the 5th was ~50 ticks.
@@ -1151,7 +1152,15 @@ void Core::removeGroup(int groupId)
     tox_conference_delete(tox.get(), groupId, &error);
     if (PARSE_ERR(error)) {
         emit saveRequest();
-        av->leaveGroupCall(groupId);
+
+        /*
+         * TODO(sudden6): this is probably not (thread-)safe, but can be ignored for now since
+         * we don't change av at runtime.
+         */
+
+        if (av) {
+            av->leaveGroupCall(groupId);
+        }
     }
 }
 

src/core/core.h

@@ -73,7 +73,12 @@ public:
                                   IBootstrapListGenerator& bootstrapNodes, ToxCoreErrors* err = nullptr);
     const CoreAV* getAv() const;
     CoreAV* getAv();
+    void setAv(CoreAV* coreAv);
+
     CoreFile* getCoreFile() const;
+    Tox* getTox() const;
+    QMutex& getCoreLoopLock() const;
+
     ~Core();
 
     static const QString TOX_EXT;
@@ -150,8 +155,6 @@ signals:
     void failedToSetStatus(Status::Status status);
     void failedToSetTyping(bool typing);
 
-    void avReady();
-
     void saveRequest();
 
     /**
@@ -245,7 +248,7 @@ private:
     ToxPtr tox;
 
     std::unique_ptr<CoreFile> file;
-    std::unique_ptr<CoreAV> av;
+    CoreAV* av = nullptr;
     QTimer* toxTimer = nullptr;
     // recursive, since we might call our own functions
     mutable QMutex coreLoopLock{QMutex::Recursive};

src/core/coreav.cpp

@@ -19,10 +19,11 @@
 */
 
 #include "coreav.h"
+#include "audio/iaudiosettings.h"
 #include "core.h"
 #include "src/model/friend.h"
 #include "src/model/group.h"
-#include "src/persistence/settings.h"
+#include "src/persistence/igroupsettings.h"
 #include "src/video/corevideosource.h"
 #include "src/video/videoframe.h"
 #include <QCoreApplication>
@@ -68,12 +69,15 @@
  * deadlock.
  */
 
-CoreAV::CoreAV(std::unique_ptr<ToxAV, ToxAVDeleter> toxav, QMutex& toxCoreLock)
+CoreAV::CoreAV(std::unique_ptr<ToxAV, ToxAVDeleter> toxav, QMutex& toxCoreLock,
+               IAudioSettings& _audioSettings, IGroupSettings& _groupSettings)
     : audio{nullptr}
     , toxav{std::move(toxav)}
     , coreavThread{new QThread{this}}
     , iterateTimer{new QTimer{this}}
     , coreLock{toxCoreLock}
+    , audioSettings{_audioSettings}
+    , groupSettings{_groupSettings}
 {
     assert(coreavThread);
     assert(iterateTimer);
@@ -105,7 +109,8 @@ void CoreAV::connectCallbacks(ToxAV& toxav)
  * @param core pointer to the Tox instance
  * @return CoreAV instance on success, {} on failure
  */
-CoreAV::CoreAVPtr CoreAV::makeCoreAV(Tox* core, QMutex &toxCoreLock)
+CoreAV::CoreAVPtr CoreAV::makeCoreAV(Tox* core, QMutex& toxCoreLock,
+                                     IAudioSettings& audioSettings, IGroupSettings& groupSettings)
 {
     Toxav_Err_New err;
     std::unique_ptr<ToxAV, ToxAVDeleter> toxav{toxav_new(core, &err)};
@@ -125,7 +130,7 @@ CoreAV::CoreAVPtr CoreAV::makeCoreAV(Tox* core, QMutex &toxCoreLock)
 
     assert(toxav != nullptr);
 
-    return CoreAVPtr{new CoreAV{std::move(toxav), toxCoreLock}};
+    return CoreAVPtr{new CoreAV{std::move(toxav), toxCoreLock, audioSettings, groupSettings}};
 }
 
 /**
@@ -251,7 +256,7 @@ bool CoreAV::answerCall(uint32_t friendNum, bool video)
     Toxav_Err_Answer err;
 
     const uint32_t videoBitrate = video ? VIDEO_DEFAULT_BITRATE : 0;
-    if (toxav_answer(toxav.get(), friendNum, Settings::getInstance().getAudioBitrate(),
+    if (toxav_answer(toxav.get(), friendNum, audioSettings.getAudioBitrate(),
                      videoBitrate, &err)) {
         it->second->setActive(true);
         return true;
@@ -276,7 +281,7 @@ bool CoreAV::startCall(uint32_t friendNum, bool video)
     }
 
     uint32_t videoBitrate = video ? VIDEO_DEFAULT_BITRATE : 0;
-    if (!toxav_call(toxav.get(), friendNum, Settings::getInstance().getAudioBitrate(), videoBitrate,
+    if (!toxav_call(toxav.get(), friendNum, audioSettings.getAudioBitrate(), videoBitrate,
                     nullptr))
         return false;
 
@@ -476,9 +481,8 @@ void CoreAV::groupCallCallback(void* tox, uint32_t group, uint32_t peer, const i
     QReadLocker locker{&cav->callsLock};
 
     const ToxPk peerPk = c->getGroupPeerPk(group, peer);
-    const Settings& s = Settings::getInstance();
     // don't play the audio if it comes from a muted peer
-    if (s.getBlackList().contains(peerPk.toString())) {
+    if (cav->groupSettings.getBlackList().contains(peerPk.toString())) {
         return;
     }
 

src/core/coreav.h

@@ -21,6 +21,7 @@
 #pragma once
 
 #include "src/core/toxcall.h"
+
 #include <QObject>
 #include <QMutex>
 #include <QReadWriteLock>
@@ -31,6 +32,8 @@
 class Friend;
 class Group;
 class IAudioControl;
+class IAudioSettings;
+class IGroupSettings;
 class QThread;
 class QTimer;
 class CoreVideoSource;
@@ -46,7 +49,8 @@ class CoreAV : public QObject
 
 public:
     using CoreAVPtr = std::unique_ptr<CoreAV>;
-    static CoreAVPtr makeCoreAV(Tox* core, QMutex& coreLock);
+    static CoreAVPtr makeCoreAV(Tox* core, QMutex& toxCoreLock,
+                                IAudioSettings& audioSettings, IGroupSettings& groupSettings);
 
     void setAudio(IAudioControl& newAudio);
     IAudioControl* getAudio();
@@ -112,7 +116,8 @@ private:
         }
     };
 
-    CoreAV(std::unique_ptr<ToxAV, ToxAVDeleter> tox, QMutex &toxCoreLock);
+    CoreAV(std::unique_ptr<ToxAV, ToxAVDeleter> tox, QMutex &toxCoreLock,
+           IAudioSettings& _audioSettings, IGroupSettings& _groupSettings);
     void connectCallbacks(ToxAV& toxav);
 
     void process();
@@ -156,4 +161,7 @@ private:
      * @note This must be a recursive mutex as we're going to lock it in callbacks
      */
     QMutex& coreLock;
+
+    IAudioSettings& audioSettings;
+    IGroupSettings& groupSettings;
 };

src/persistence/profile.cpp

@@ -232,7 +232,7 @@ bool logCreateToxDataError(const CreateToxDataError& error, const QString& userN
 
 QStringList Profile::profiles;
 
-void Profile::initCore(const QByteArray& toxsave, const ICoreSettings& s, bool isNewProfile)
+void Profile::initCore(const QByteArray& toxsave, Settings& s, bool isNewProfile)
 {
     if (toxsave.isEmpty() && !isNewProfile) {
         qCritical() << "Existing toxsave is empty";
@@ -245,7 +245,7 @@ void Profile::initCore(const QByteArray& toxsave, const ICoreSettings& s, bool i
     }
 
     bootstrapNodes = std::unique_ptr<BootstrapNodeUpdater>(
-        new BootstrapNodeUpdater(Settings::getInstance().getProxy(), paths));
+        new BootstrapNodeUpdater(s.getProxy(), paths));
 
     Core::ToxCoreErrors err;
     core = Core::makeToxCore(toxsave, &s, *bootstrapNodes, &err);
@@ -265,6 +265,17 @@ void Profile::initCore(const QByteArray& toxsave, const ICoreSettings& s, bool i
         return;
     }
 
+    coreAv = CoreAV::makeCoreAV(core->getTox(), core->getCoreLoopLock(), s, s);
+    if (!coreAv) {
+        qDebug() << "Failed to start ToxAV";
+        emit failedToStart();
+        return;
+    }
+
+    // Tell Core that we run with AV before doing anything else
+    core->setAv(coreAv.get());
+    coreAv->start();
+
     if (isNewProfile) {
         core->setStatusMessage(tr("Toxing on qTox"));
         core->setUsername(name);
@@ -282,7 +293,7 @@ void Profile::initCore(const QByteArray& toxsave, const ICoreSettings& s, bool i
     avatarBroadcaster = std::unique_ptr<AvatarBroadcaster>(new AvatarBroadcaster(*core));
 }
 
-Profile::Profile(const QString& name, const QString& password, std::unique_ptr<ToxEncrypt> passkey, Paths& paths_)
+Profile::Profile(const QString& name, std::unique_ptr<ToxEncrypt> passkey, Paths& paths_)
     : name{name}
     , passkey{std::move(passkey)}
     , isRemoved{false}
@@ -320,7 +331,7 @@ Profile* Profile::loadProfile(const QString& name, const QString& password, Sett
         return nullptr;
     }
 
-    Profile* p = new Profile(name, password, std::move(tmpKey), settings.getPaths());
+    Profile* p = new Profile(name, std::move(tmpKey), settings.getPaths());
 
     // Core settings are saved per profile, need to load them before starting Core
     settings.updateProfileData(p, parser);
@@ -351,7 +362,7 @@ Profile* Profile::createProfile(const QString& name, const QString& password, Se
     }
 
     settings.createPersonal(name);
-    Profile* p = new Profile(name, password, std::move(tmpKey), settings.getPaths());
+    Profile* p = new Profile(name, std::move(tmpKey), settings.getPaths());
     settings.updateProfileData(p, parser);
 
     p->initCore(QByteArray(), settings, /*isNewProfile*/ true);

src/persistence/profile.h

@@ -104,15 +104,16 @@ private slots:
     void onAvatarOfferReceived(uint32_t friendId, uint32_t fileId, const QByteArray& avatarHash);
 
 private:
-    Profile(const QString& name, const QString& password, std::unique_ptr<ToxEncrypt> passkey, Paths& paths);
+    Profile(const QString& name, std::unique_ptr<ToxEncrypt> passkey, Paths& paths);
     static QStringList getFilesByExt(QString extension);
     QString avatarPath(const ToxPk& owner, bool forceUnencrypted = false);
     bool saveToxSave(QByteArray data);
-    void initCore(const QByteArray& toxsave, const ICoreSettings& s, bool isNewProfile);
+    void initCore(const QByteArray& toxsave, Settings &s, bool isNewProfile);
 
 private:
     std::unique_ptr<AvatarBroadcaster> avatarBroadcaster;
     std::unique_ptr<Core> core;
+    std::unique_ptr<CoreAV> coreAv;
     QString name;
     std::unique_ptr<ToxEncrypt> passkey;
     std::shared_ptr<RawDatabase> database;