refactor(coreav): move CoreAV to the factory pattern too

- clean up error handling during construction of the Core
- prevent leaks by using unique_ptr

src/core/core.cpp

@@ -209,8 +209,8 @@ ToxCorePtr Core::makeToxCore(const QByteArray& savedata, const ICoreSettings* co
     assert(core->tox != nullptr);
 
     // toxcore is successfully created, create toxav
-    core->av = std::unique_ptr<CoreAV>(new CoreAV(core->tox.get()));
-    if (!core->av || !core->av->getToxAv()) {
+    core->av = CoreAV::makeCoreAV(core->tox.get());
+    if (!core->av) {
         qCritical() << "Toxav failed to start";
         if (err) {
             *err = ToxCoreErrors::FAILED_TO_START;
@@ -478,15 +478,15 @@ void Core::onGroupMessage(Tox*, uint32_t groupId, uint32_t peerId, Tox_Message_T
     emit core->groupMessageReceived(groupId, peerId, message, isAction);
 }
 
-void Core::onGroupPeerListChange(Tox*, uint32_t groupId, void* core)
+void Core::onGroupPeerListChange(Tox*, uint32_t groupId, void* vCore)
 {
-    const auto coreAv = static_cast<Core*>(core)->getAv();
-    if (coreAv->isGroupAvEnabled(groupId)) {
+    const auto core = static_cast<Core*>(vCore);
+    if (core->getGroupAvEnabled(groupId)) {
         CoreAV::invalidateGroupCallSources(groupId);
     }
 
     qDebug() << QString("Group %1 peerlist changed").arg(groupId);
-    emit static_cast<Core*>(core)->groupPeerlistChanged(groupId);
+    emit core->groupPeerlistChanged(groupId);
 }
 
 void Core::onGroupPeerNameChange(Tox*, uint32_t groupId, uint32_t peerId, const uint8_t* name,
@@ -1170,6 +1170,30 @@ QStringList Core::getGroupPeerNames(int groupId) const
     return names;
 }
 
+/**
+ * @brief Check, that group has audio or video stream
+ * @param groupId Id of group to check
+ * @return True for AV groups, false for text-only groups
+ */
+bool Core::getGroupAvEnabled(int groupId) const
+{
+    QMutexLocker ml{coreLoopLock.get()};
+    TOX_ERR_CONFERENCE_GET_TYPE error;
+    TOX_CONFERENCE_TYPE type = tox_conference_get_type(tox.get(), groupId, &error);
+    switch (error) {
+    case TOX_ERR_CONFERENCE_GET_TYPE_OK:
+        break;
+    case TOX_ERR_CONFERENCE_GET_TYPE_CONFERENCE_NOT_FOUND:
+        qWarning() << "Conference not found";
+        break;
+    default:
+        qWarning() << "Unknown error code:" << QString::number(error);
+        break;
+    }
+
+    return type == TOX_CONFERENCE_TYPE_AV;
+}
+
 /**
  * @brief Print in console text of error.
  * @param error Error to handle.
@@ -1429,7 +1453,7 @@ QString Core::getPeerName(const ToxPk& id) const
  */
 bool Core::isReady() const
 {
-    return av && av->getToxAv() && tox;
+    return av && tox;
 }
 
 /**

src/core/core.h

@@ -81,6 +81,7 @@ public:
     QString getGroupPeerName(int groupId, int peerId) const;
     ToxPk getGroupPeerPk(int groupId, int peerId) const;
     QStringList getGroupPeerNames(int groupId) const;
+    bool getGroupAvEnabled(int groupId) const;
     ToxPk getFriendPublicKey(uint32_t friendNumber) const;
     QString getFriendUsername(uint32_t friendNumber) const;
 

src/core/coreav.cpp

@@ -81,8 +81,9 @@ std::map<uint32_t, ToxFriendCall> CoreAV::calls;
  */
 std::map<int, ToxGroupCall> CoreAV::groupCalls;
 
-CoreAV::CoreAV(Tox* tox)
-    : coreavThread{new QThread{this}}
+CoreAV::CoreAV(std::unique_ptr<ToxAV, ToxAVDeleter> toxav)
+    : toxav{std::move(toxav)}
+    , coreavThread{new QThread{this}}
     , iterateTimer{new QTimer{this}}
     , threadSwitchLock{false}
 {
@@ -92,21 +93,51 @@ CoreAV::CoreAV(Tox* tox)
     coreavThread->setObjectName("qTox CoreAV");
     moveToThread(coreavThread.get());
 
+    connectCallbacks(*this->toxav);
+
     iterateTimer->setSingleShot(true);
 
     connect(iterateTimer, &QTimer::timeout, this, &CoreAV::process);
     connect(coreavThread.get(), &QThread::finished, iterateTimer, &QTimer::stop);
 
-    toxav = toxav_new(tox, nullptr);
+    coreavThread->start();
+}
 
-    toxav_callback_call(toxav, CoreAV::callCallback, this);
-    toxav_callback_call_state(toxav, CoreAV::stateCallback, this);
-    toxav_callback_audio_bit_rate(toxav, CoreAV::audioBitrateCallback, this);
-    toxav_callback_video_bit_rate(toxav, CoreAV::videoBitrateCallback, this);
-    toxav_callback_audio_receive_frame(toxav, CoreAV::audioFrameCallback, this);
-    toxav_callback_video_receive_frame(toxav, CoreAV::videoFrameCallback, this);
+void CoreAV::connectCallbacks(ToxAV& toxav) {
+    toxav_callback_call(&toxav, CoreAV::callCallback, this);
+    toxav_callback_call_state(&toxav, CoreAV::stateCallback, this);
+    toxav_callback_audio_bit_rate(&toxav, CoreAV::audioBitrateCallback, this);
+    toxav_callback_video_bit_rate(&toxav, CoreAV::videoBitrateCallback, this);
+    toxav_callback_audio_receive_frame(&toxav, CoreAV::audioFrameCallback, this);
+    toxav_callback_video_receive_frame(&toxav, CoreAV::videoFrameCallback, this);
+}
 
-    coreavThread->start();
+/**
+ * @brief Factory method for CoreAV
+ * @param core pointer to the Tox instance
+ * @return CoreAV instance on success, {} on failure
+ */
+CoreAV::CoreAVPtr CoreAV::makeCoreAV(Tox* core)
+{
+    TOXAV_ERR_NEW err;
+    std::unique_ptr<ToxAV, ToxAVDeleter> toxav{toxav_new(core, &err)};
+    switch (err) {
+    case TOXAV_ERR_NEW_OK:
+        break;
+    case TOXAV_ERR_NEW_MALLOC:
+        qCritical() << "Failed to allocate ressources for ToxAV";
+        return {};
+    case TOXAV_ERR_NEW_MULTIPLE:
+        qCritical() << "Attempted to create multiple ToxAV instances";
+        return {};
+    case TOXAV_ERR_NEW_NULL:
+        qCritical() << "Unexpected NULL parameter";
+        return {};
+    }
+
+    assert(toxav != nullptr);
+
+    return CoreAVPtr{new CoreAV{std::move(toxav)}};
 }
 
 CoreAV::~CoreAV()
@@ -117,12 +148,6 @@ CoreAV::~CoreAV()
 
     coreavThread->exit(0);
     coreavThread->wait();
-    toxav_kill(toxav);
-}
-
-const ToxAV* CoreAV::getToxAv() const
-{
-    return toxav;
 }
 
 /**
@@ -138,8 +163,8 @@ void CoreAV::start()
 
 void CoreAV::process()
 {
-    toxav_iterate(toxav);
-    iterateTimer->start(toxav_iteration_interval(toxav));
+    toxav_iterate(toxav.get());
+    iterateTimer->start(toxav_iteration_interval(toxav.get()));
 }
 
 /**
@@ -229,12 +254,12 @@ 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, friendNum, Settings::getInstance().getAudioBitrate(), videoBitrate, &err)) {
+    if (toxav_answer(toxav.get(), friendNum, Settings::getInstance().getAudioBitrate(), videoBitrate, &err)) {
         it->second.setActive(true);
         return true;
     } else {
         qWarning() << "Failed to answer call with error" << err;
-        toxav_call_control(toxav, friendNum, TOXAV_CALL_CONTROL_CANCEL, nullptr);
+        toxav_call_control(toxav.get(), friendNum, TOXAV_CALL_CONTROL_CANCEL, nullptr);
         calls.erase(it);
         return false;
     }
@@ -265,7 +290,7 @@ bool CoreAV::startCall(uint32_t friendNum, bool video)
     }
 
     uint32_t videoBitrate = video ? VIDEO_DEFAULT_BITRATE : 0;
-    if (!toxav_call(toxav, friendNum, Settings::getInstance().getAudioBitrate(), videoBitrate, nullptr))
+    if (!toxav_call(toxav.get(), friendNum, Settings::getInstance().getAudioBitrate(), videoBitrate, nullptr))
         return false;
 
     auto ret = calls.insert(std::make_pair(friendNum, ToxFriendCall(friendNum, video, *this)));
@@ -290,7 +315,7 @@ bool CoreAV::cancelCall(uint32_t friendNum)
     }
 
     qDebug() << QString("Cancelling call with %1").arg(friendNum);
-    if (!toxav_call_control(toxav, friendNum, TOXAV_CALL_CONTROL_CANCEL, nullptr)) {
+    if (!toxav_call_control(toxav.get(), friendNum, TOXAV_CALL_CONTROL_CANCEL, nullptr)) {
         qWarning() << QString("Failed to cancel call with %1").arg(friendNum);
         return false;
     }
@@ -345,7 +370,7 @@ bool CoreAV::sendCallAudio(uint32_t callId, const int16_t* pcm, size_t samples,
     TOXAV_ERR_SEND_FRAME err;
     int retries = 0;
     do {
-        if (!toxav_audio_send_frame(toxav, callId, pcm, samples, chans, rate, &err)) {
+        if (!toxav_audio_send_frame(toxav.get(), callId, pcm, samples, chans, rate, &err)) {
             if (err == TOXAV_ERR_SEND_FRAME_SYNC) {
                 ++retries;
                 QThread::usleep(500);
@@ -379,7 +404,7 @@ void CoreAV::sendCallVideo(uint32_t callId, std::shared_ptr<VideoFrame> vframe)
 
     if (call.getNullVideoBitrate()) {
         qDebug() << "Restarting video stream to friend" << callId;
-        toxav_video_set_bit_rate(toxav, callId, VIDEO_DEFAULT_BITRATE, nullptr);
+        toxav_video_set_bit_rate(toxav.get(), callId, VIDEO_DEFAULT_BITRATE, nullptr);
         call.setNullVideoBitrate(false);
     }
 
@@ -394,7 +419,7 @@ void CoreAV::sendCallVideo(uint32_t callId, std::shared_ptr<VideoFrame> vframe)
     TOXAV_ERR_SEND_FRAME err;
     int retries = 0;
     do {
-        if (!toxav_video_send_frame(toxav, callId, frame.width, frame.height, frame.y, frame.u,
+        if (!toxav_video_send_frame(toxav.get(), callId, frame.width, frame.height, frame.y, frame.u,
                                     frame.v, &err)) {
             if (err == TOXAV_ERR_SEND_FRAME_SYNC) {
                 ++retries;
@@ -565,7 +590,7 @@ bool CoreAV::sendGroupCallAudio(int groupId, const int16_t* pcm, size_t samples,
         return true;
     }
 
-    if (toxav_group_send_audio(toxav_get_tox(toxav), groupId, pcm, samples, chans, rate) != 0)
+    if (toxav_group_send_audio(toxav_get_tox(toxav.get()), groupId, pcm, samples, chans, rate) != 0)
         qDebug() << "toxav_group_send_audio error";
 
     return true;
@@ -629,29 +654,6 @@ bool CoreAV::isGroupCallOutputMuted(const Group* g) const
     return (it != groupCalls.end()) && it->second.getMuteVol();
 }
 
-/**
- * @brief Check, that group has audio or video stream
- * @param groupId Id of group to check
- * @return True for AV groups, false for text-only groups
- */
-bool CoreAV::isGroupAvEnabled(int groupId) const
-{
-    Tox* tox = Core::getInstance()->tox.get();
-    Tox_Err_Conference_Get_Type error;
-    Tox_Conference_Type type = tox_conference_get_type(tox, groupId, &error);
-    switch (error) {
-    case TOX_ERR_CONFERENCE_GET_TYPE_OK:
-        break;
-    case TOX_ERR_CONFERENCE_GET_TYPE_CONFERENCE_NOT_FOUND:
-        qCritical() << "Conference not found";
-        break;
-    default:
-        break;
-    }
-
-    return type == TOX_CONFERENCE_TYPE_AV;
-}
-
 /**
  * @brief Returns the calls input (microphone) mute state.
  * @param f The friend to check
@@ -707,7 +709,7 @@ void CoreAV::sendNoVideo()
     qDebug() << "CoreAV: Signaling end of video sending";
     for (auto& kv : calls) {
         ToxFriendCall& call = kv.second;
-        toxav_video_set_bit_rate(toxav, kv.first, 0, nullptr);
+        toxav_video_set_bit_rate(toxav.get(), kv.first, 0, nullptr);
         call.setNullVideoBitrate(true);
     }
 }

src/core/coreav.h

@@ -35,7 +35,7 @@ class CoreVideoSource;
 class CameraSource;
 class VideoSource;
 class VideoFrame;
-class CoreAV;
+class Core;
 struct vpx_image;
 
 class CoreAV : public QObject
@@ -43,10 +43,11 @@ class CoreAV : public QObject
     Q_OBJECT
 
 public:
-    explicit CoreAV(Tox* tox);
-    ~CoreAV();
 
-    const ToxAV* getToxAv() const;
+    using CoreAVPtr = std::unique_ptr<CoreAV>;
+    static CoreAVPtr makeCoreAV(Tox* core);
+
+    ~CoreAV();
 
     bool anyActiveCalls() const;
     bool isCallStarted(const Friend* f) const;
@@ -70,7 +71,6 @@ public:
     void muteCallOutput(const Group* g, bool mute);
     bool isGroupCallInputMuted(const Group* g) const;
     bool isGroupCallOutputMuted(const Group* g) const;
-    bool isGroupAvEnabled(int groupNum) const;
 
     bool isCallInputMuted(const Friend* f) const;
     bool isCallOutputMuted(const Friend* f) const;
@@ -103,6 +103,17 @@ private slots:
     static void videoBitrateCallback(ToxAV* toxAV, uint32_t friendNum, uint32_t rate, void* self);
 
 private:
+    struct ToxAVDeleter
+    {
+        void operator()(ToxAV* tox)
+        {
+            toxav_kill(tox);
+        }
+    };
+
+    explicit CoreAV(std::unique_ptr<ToxAV, ToxAVDeleter> tox);
+    void connectCallbacks(ToxAV& toxav);
+
     void process();
     static void audioFrameCallback(ToxAV* toxAV, uint32_t friendNum, const int16_t* pcm,
                                    size_t sampleCount, uint8_t channels, uint32_t samplingRate,
@@ -115,7 +126,8 @@ private:
     static constexpr uint32_t VIDEO_DEFAULT_BITRATE = 2500;
 
 private:
-    ToxAV* toxav;
+
+    std::unique_ptr<ToxAV, ToxAVDeleter> toxav;
     std::unique_ptr<QThread> coreavThread;
     QTimer* iterateTimer = nullptr;
     static std::map<uint32_t, ToxFriendCall> calls;

src/widget/widget.cpp

@@ -1875,9 +1875,8 @@ Group* Widget::createGroup(int groupId)
 
     const auto groupName = tr("Groupchat #%1").arg(groupId);
     Core* core = Nexus::getCore();
-    CoreAV* coreAv = core->getAv();
 
-    bool enabled = coreAv->isGroupAvEnabled(groupId);
+    bool enabled = core->getGroupAvEnabled(groupId);
     Group* newgroup = GroupList::addGroup(groupId, groupName, enabled, core->getUsername());
     std::shared_ptr<GroupChatroom> chatroom(new GroupChatroom(newgroup));
     const auto compact = Settings::getInstance().getCompactLayout();