From 576bd0892c4a9f9570532ab3a9456240c6532360 Mon Sep 17 00:00:00 2001 From: Julien Wadel Date: Thu, 28 Mar 2024 14:23:50 +0100 Subject: [PATCH] Blocking connection for posting lambdas on model from GUI. Fix camera crash on deletion. Call crash + 1-1 AS --- Linphone/core/App.hpp | 9 ++ Linphone/core/camera/CameraGui.cpp | 101 ++++++++++-------- Linphone/core/camera/CameraGui.hpp | 7 +- .../participant/ParticipantDeviceList.cpp | 4 +- Linphone/model/call/CallModel.cpp | 2 +- Linphone/view/App/CallsWindow.qml | 4 +- Linphone/view/App/Layout/MainLayout.qml | 7 +- Linphone/view/Item/Call/WaitingRoom.qml | 3 +- Linphone/view/Item/Contact/Sticker.qml | 4 +- .../view/Layout/Call/ActiveSpeakerLayout.qml | 7 +- 10 files changed, 87 insertions(+), 61 deletions(-) diff --git a/Linphone/core/App.hpp b/Linphone/core/App.hpp index 13675a5d4..53ef8c0a0 100644 --- a/Linphone/core/App.hpp +++ b/Linphone/core/App.hpp @@ -86,6 +86,15 @@ public: } } + template + static auto postModelBlock(Func &&callable, Args &&...args) { + if (QThread::currentThread() != CoreModel::getInstance()->thread()) { + QMetaObject::invokeMethod(CoreModel::getInstance().get(), callable, args..., Qt::BlockingQueuedConnection); + } else { + QMetaObject::invokeMethod(CoreModel::getInstance().get(), callable, Qt::DirectConnection); + } + } + void clean(); void init(); void initCppInterfaces(); diff --git a/Linphone/core/camera/CameraGui.cpp b/Linphone/core/camera/CameraGui.cpp index c41ca81c4..4dd4bfc0d 100644 --- a/Linphone/core/camera/CameraGui.cpp +++ b/Linphone/core/camera/CameraGui.cpp @@ -33,8 +33,8 @@ DEFINE_ABSTRACT_OBJECT(CameraGui) -QMutex CameraGui::mPreviewCounterMutex; -int CameraGui::mPreviewCounter = 0; +QMutex CameraGui::gPreviewCounterMutex; +int CameraGui::gPreviewCounter = 0; // ============================================================================= CameraGui::CameraGui(QQuickItem *parent) : QQuickFramebufferObject(parent) { @@ -51,13 +51,15 @@ CameraGui::CameraGui(QQuickItem *parent) : QQuickFramebufferObject(parent) { CameraGui::~CameraGui() { mustBeInMainThread("~" + getClassName()); mRefreshTimer.stop(); - App::postModelSync([this]() { CoreModel::getInstance()->getCore()->enableVideoPreview(false); }); + mIsDeleting = true; + deactivatePreview(); + setWindowIdLocation(None); } QQuickFramebufferObject::Renderer *CameraGui::createRenderer() const { auto renderer = createRenderer(false); if (!renderer) { - qInfo() << "[Camera] (" << mQmlName << ") Setting Camera to Dummy, " << getSourceLocation(); + qInfo() << log().arg("(%1) Setting Camera to Dummy, %2").arg(mQmlName).arg(getSourceLocation()); QTimer::singleShot(1, this, &CameraGui::isNotReady); renderer = new CameraDummy(); // Used to fill a renderer to avoid pushing a NULL. QTimer::singleShot(1000, this, &CameraGui::requestNewRenderer); @@ -69,14 +71,13 @@ QQuickFramebufferObject::Renderer *CameraGui::createRenderer(bool resetWindowId) QQuickFramebufferObject::Renderer *renderer = NULL; // A renderer is mandatory, we cannot wait async. switch (getSourceLocation()) { - case CorePreview: - App::postModelSync([this, &renderer, resetWindowId]() { - qInfo() << "[Camera] (" << mQmlName << ") Setting Camera to Preview"; + case CorePreview: { + auto f = [qmlName = mQmlName, &renderer, resetWindowId]() { + qInfo() << "[Camera] (" << qmlName << ") Setting Camera to Preview"; auto coreModel = CoreModel::getInstance(); if (coreModel) { auto core = coreModel->getCore(); if (!core) return; - core->enableVideoPreview(true); if (resetWindowId) { renderer = (QQuickFramebufferObject::Renderer *)core->getNativePreviewWindowId(); if (renderer) core->setNativePreviewWindowId(NULL); @@ -85,13 +86,16 @@ QQuickFramebufferObject::Renderer *CameraGui::createRenderer(bool resetWindowId) if (renderer) core->setNativePreviewWindowId(renderer); } } - }); - break; - case Call: - App::postModelSync([this, &renderer, resetWindowId]() { - auto call = mCallGui->getCore()->getModel()->getMonitor(); + }; + if (mIsDeleting) { + App::postModelBlock(f); + } else App::postModelSync(f); + } break; + case Call: { + auto f = [qmlName = mQmlName, callGui = mCallGui, &renderer, resetWindowId]() { + auto call = callGui->getCore()->getModel()->getMonitor(); if (call) { - qInfo() << "[Camera] (" << mQmlName << ") Setting Camera to CallModel"; + qInfo() << "[Camera] (" << qmlName << ") Setting Camera to CallModel"; if (resetWindowId) { renderer = (QQuickFramebufferObject::Renderer *)call->getNativeVideoWindowId(); if (renderer) call->setNativeVideoWindowId(NULL); @@ -100,21 +104,27 @@ QQuickFramebufferObject::Renderer *CameraGui::createRenderer(bool resetWindowId) if (renderer) call->setNativeVideoWindowId(renderer); } } - }); - break; - case Device: - App::postModelSync([this, &renderer, resetWindowId]() { - auto device = mParticipantDeviceGui->getCore()->getModel()->getMonitor(); + }; + if (mIsDeleting) { + App::postModelBlock(f); + } else App::postModelSync(f); + } break; + case Device: { + auto f = [qmlName = mQmlName, participantDeviceGui = mParticipantDeviceGui, &renderer, resetWindowId]() { + auto device = participantDeviceGui->getCore()->getModel()->getMonitor(); if (device) { - qInfo() << "[Camera] (" << mQmlName << ") Setting Camera to ParticipantDeviceModel"; + qInfo() << "[Camera] (" << qmlName << ") Setting Camera to ParticipantDeviceModel"; if (resetWindowId) { } else { renderer = (QQuickFramebufferObject::Renderer *)device->createNativeVideoWindowId(); if (renderer) device->setNativeVideoWindowId(renderer); } } - }); - break; + }; + if (mIsDeleting) { + App::postModelBlock(f); + } else App::postModelSync(f); + } break; default: { } } @@ -169,9 +179,7 @@ bool CameraGui::getIsPreview() const { void CameraGui::setIsPreview(bool status) { if (mIsPreview != status) { mIsPreview = status; - if (mIsPreview) activatePreview(); - else deactivatePreview(); - // updateWindowIdLocation(); + updateWindowIdLocation(); update(); emit isPreviewChanged(status); @@ -198,7 +206,7 @@ ParticipantDeviceGui *CameraGui::getParticipantDeviceGui() const { void CameraGui::setParticipantDeviceGui(ParticipantDeviceGui *deviceGui) { if (mParticipantDeviceGui != deviceGui) { mParticipantDeviceGui = deviceGui; - qDebug() << "Set Device " << mParticipantDeviceGui; + qDebug() << log().arg("Set Device %1").arg((quint64)mParticipantDeviceGui); // setIsPreview(mParticipantDeviceGui->getCore()->isLocal()); emit participantDeviceGuiChanged(mParticipantDeviceGui); updateWindowIdLocation(); @@ -210,33 +218,37 @@ CameraGui::WindowIdLocation CameraGui::getSourceLocation() const { } void CameraGui::activatePreview() { - mPreviewCounterMutex.lock(); - setWindowIdLocation(WindowIdLocation::CorePreview); - if (++mPreviewCounter == 1) { - App::postModelSync([this]() { - auto coreModel = CoreModel::getInstance(); - coreModel->getCore()->enableVideoPreview(true); - }); + gPreviewCounterMutex.lock(); + if (++gPreviewCounter == 1) { + auto f = []() { CoreModel::getInstance()->getCore()->enableVideoPreview(true); }; + if (mIsDeleting) App::postModelBlock(f); + else App::postModelSync(f); } - mPreviewCounterMutex.unlock(); + gPreviewCounterMutex.unlock(); } void CameraGui::deactivatePreview() { - mPreviewCounterMutex.lock(); - setWindowIdLocation(WindowIdLocation::None); - if (--mPreviewCounter == 0) { - App::postModelSync([this]() { - auto coreModel = CoreModel::getInstance(); - coreModel->getCore()->enableVideoPreview(false); - }); - mPreviewCounterMutex.unlock(); + gPreviewCounterMutex.lock(); + if (getSourceLocation() == CorePreview) { + if (--gPreviewCounter == 0) { + auto f = []() { CoreModel::getInstance()->getCore()->enableVideoPreview(false); }; + if (mIsDeleting) App::postModelBlock(f); + else App::postModelSync(f); + } } + gPreviewCounterMutex.unlock(); } void CameraGui::setWindowIdLocation(const WindowIdLocation &location) { if (mWindowIdLocation != location) { - qDebug() << "Update Window Id location from " << mWindowIdLocation << " to " << location; + qDebug() << log() + .arg("( %1 ) Update Window Id location from %2 to %3") + .arg(mQmlName) + .arg(mWindowIdLocation) + .arg(location); + if (mWindowIdLocation == CorePreview) deactivatePreview(); resetWindowId(); // Location change: Reset old window ID. mWindowIdLocation = location; + if (mWindowIdLocation == CorePreview) activatePreview(); update(); // if (mWindowIdLocation == WindowIdLocation::CorePreview) { // mLastVideoDefinition = @@ -247,7 +259,8 @@ void CameraGui::setWindowIdLocation(const WindowIdLocation &location) { } void CameraGui::updateWindowIdLocation() { bool useDefaultWindow = true; - if (mCallGui) setWindowIdLocation(WindowIdLocation::Call); + if (mIsPreview) setWindowIdLocation(WindowIdLocation::CorePreview); + else if (mCallGui) setWindowIdLocation(WindowIdLocation::Call); else if (mParticipantDeviceGui && !mParticipantDeviceGui->getCore()->isLocal()) setWindowIdLocation(WindowIdLocation::Device); else setWindowIdLocation(WindowIdLocation::CorePreview); diff --git a/Linphone/core/camera/CameraGui.hpp b/Linphone/core/camera/CameraGui.hpp index 42e10bb68..764b90de8 100644 --- a/Linphone/core/camera/CameraGui.hpp +++ b/Linphone/core/camera/CameraGui.hpp @@ -42,7 +42,7 @@ class CameraGui : public QQuickFramebufferObject, public AbstractObject { Q_PROPERTY(bool isReady READ getIsReady WRITE setIsReady NOTIFY isReadyChanged) // Q_PROPERTY(SoundPlayer * linphonePlayer READ getLinphonePlayer WRITE setLinphonePlayer NOTIFY // linphonePlayerChanged) - Q_PROPERTY(QString qmlName READ getQmlName WRITE setQmlName NOTIFY qmlNameChanged) + Q_PROPERTY(QString qmlName READ getQmlName WRITE setQmlName NOTIFY qmlNameChanged REQUIRED) typedef enum { None = -1, CorePreview = 0, Call, Device, Player, Core } WindowIdLocation; @@ -55,8 +55,8 @@ public: Q_INVOKABLE void resetWindowId() const; // const to be used from createRenderer() void checkVideoDefinition(); - static QMutex mPreviewCounterMutex; - static int mPreviewCounter; + static QMutex gPreviewCounterMutex; + static int gPreviewCounter; bool getIsReady() const; void setIsReady(bool isReady); @@ -105,6 +105,7 @@ private: WindowIdLocation mWindowIdLocation = None; mutable bool mIsWindowIdSet = false; + bool mIsDeleting = false; DECLARE_ABSTRACT_OBJECT }; diff --git a/Linphone/core/participant/ParticipantDeviceList.cpp b/Linphone/core/participant/ParticipantDeviceList.cpp index c83ce6ab0..2d985d4ab 100644 --- a/Linphone/core/participant/ParticipantDeviceList.cpp +++ b/Linphone/core/participant/ParticipantDeviceList.cpp @@ -75,7 +75,7 @@ QSharedPointer ParticipantDeviceList::getMe() const { void ParticipantDeviceList::setDevices(QList> devices) { mustBeInMainThread(log().arg(Q_FUNC_INFO)); add(devices); - qDebug() << "[ParticipantDeviceList] : add " << devices.size() << " devices"; + qDebug() << log().arg("Add %1 devices").arg(devices.size()); } QSharedPointer ParticipantDeviceList::findDeviceByUniqueAddress(const QString &address) { @@ -93,7 +93,7 @@ QSharedPointer ParticipantDeviceList::findDeviceByUniqueA void ParticipantDeviceList::setConferenceModel(const std::shared_ptr &conferenceModel) { mustBeInMainThread(log().arg(Q_FUNC_INFO)); mConferenceModel = conferenceModel; - qDebug() << "[ParticipantDeviceList] : set Conference " << mConferenceModel.get(); + qDebug() << log().arg("Set Conference %1").arg((quint64)mConferenceModel.get()); if (mConferenceModelConnection->mCore.lock()) { // Unsure to get myself auto oldConnect = mConferenceModelConnection->mCore; // Setself rebuild safepointer setSelf(mConferenceModelConnection->mCore.mQData); // reset connections diff --git a/Linphone/model/call/CallModel.cpp b/Linphone/model/call/CallModel.cpp index d9aa61d34..2d988ec4b 100644 --- a/Linphone/model/call/CallModel.cpp +++ b/Linphone/model/call/CallModel.cpp @@ -72,8 +72,8 @@ void CallModel::accept(bool withVideo) { break; } } - mMonitor->acceptWithParams(params); + emit cameraEnabledChanged(withVideo); } void CallModel::decline() { diff --git a/Linphone/view/App/CallsWindow.qml b/Linphone/view/App/CallsWindow.qml index 79ef4ab24..b3c81a23b 100644 --- a/Linphone/view/App/CallsWindow.qml +++ b/Linphone/view/App/CallsWindow.qml @@ -358,7 +358,7 @@ Window { Layout.fillHeight: true Control.StackView { id: middleItemStackView - initialItem: waitingRoom + initialItem: mainWindow.call ? inCallItem : waitingRoom Layout.fillWidth: true Layout.fillHeight: true Layout.margins: 20 * DefaultStyle.dp @@ -597,7 +597,7 @@ Window { } onAddParticipantRequested: participantsStack.currentIndex = 1 } - AddParticipantLayout { + AddParticipantsLayout { conferenceInfoGui: mainWindow.conferenceInfo } } diff --git a/Linphone/view/App/Layout/MainLayout.qml b/Linphone/view/App/Layout/MainLayout.qml index 5c4a894e4..0b5acf75a 100644 --- a/Linphone/view/App/Layout/MainLayout.qml +++ b/Linphone/view/App/Layout/MainLayout.qml @@ -13,6 +13,7 @@ import UtilsCpp Item { id: mainItem + property var callObj signal addAccountRequest() @@ -220,7 +221,7 @@ Item { background: Item{} Layout.preferredWidth: 24 * DefaultStyle.dp Layout.preferredHeight: 24 * DefaultStyle.dp - property var callObj + contentItem: Image { anchors.fill: parent width: 24 * DefaultStyle.dp @@ -228,7 +229,7 @@ Item { source: AppIcons.phone } onClicked: { - callObj = UtilsCpp.createCall(sipAddr.text) + mainItem.callObj = UtilsCpp.createCall(sipAddr.text) } } Button { @@ -241,7 +242,7 @@ Item { height: 24 * DefaultStyle.dp source: AppIcons.videoCamera } - onClicked: callObj = UtilsCpp.createCall(sipAddr.text, true) + onClicked: mainItem.callObj = UtilsCpp.createCall(sipAddr.text, true) } } Button { diff --git a/Linphone/view/Item/Call/WaitingRoom.qml b/Linphone/view/Item/Call/WaitingRoom.qml index 3255208c2..691334c84 100644 --- a/Linphone/view/Item/Call/WaitingRoom.qml +++ b/Linphone/view/Item/Call/WaitingRoom.qml @@ -7,7 +7,7 @@ import UtilsCpp 1.0 RowLayout { id: mainItem - property bool cameraEnabled: true + property alias cameraEnabled: preview.cameraEnabled property bool microEnabled: true property bool settingsButtonChecked: settingsButton.checked property ConferenceInfoGui conferenceInfo @@ -24,6 +24,7 @@ RowLayout { id: preview Layout.preferredHeight: 330 * DefaultStyle.dp Layout.preferredWidth: 558 * DefaultStyle.dp + qmlName: "WP" AccountProxy{ id: accounts } diff --git a/Linphone/view/Item/Contact/Sticker.qml b/Linphone/view/Item/Contact/Sticker.qml index 1928ea48d..fc6ff5745 100644 --- a/Linphone/view/Item/Contact/Sticker.qml +++ b/Linphone/view/Item/Contact/Sticker.qml @@ -81,7 +81,7 @@ Item { onTriggered: {cameraLoader.active=false; cameraLoader.active=true;} } active: mainItem.visible && mainItem.cameraEnabled - onActiveChanged: console.log("camera active", active) + onActiveChanged: console.log("("+mainItem.qmlName+") Camera active " + active) sourceComponent: cameraComponent } Component{ @@ -94,10 +94,10 @@ Item { id: cameraItem anchors.fill: parent visible: false + qmlName: mainItem.qmlName call: mainItem.call participantDevice: mainItem.participantDevice isPreview: mainItem.previewEnabled - qmlName: mainItem.qmlName onRequestNewRenderer: { console.log("Request new renderer") resetTimer.restart() diff --git a/Linphone/view/Layout/Call/ActiveSpeakerLayout.qml b/Linphone/view/Layout/Call/ActiveSpeakerLayout.qml index 37e239dc0..e28ea9477 100644 --- a/Linphone/view/Layout/Call/ActiveSpeakerLayout.qml +++ b/Linphone/view/Layout/Call/ActiveSpeakerLayout.qml @@ -35,8 +35,7 @@ Item{ call: mainItem.call participantDevice: mainItem.conference && mainItem.conference.core.activeSpeaker property var address: participantDevice && participantDevice.core.address - onAddressChanged: console.log(address) - cameraEnabled: true + cameraEnabled: call && call.core.remoteVideoEnabled qmlName: 'AS' Timer { @@ -83,6 +82,8 @@ Item{ visible: allDevices.count > 2 spacing: 15 * DefaultStyle.dp model: allDevices + snapMode: ListView.SnapOneItem + clip: true delegate: Sticker { visible: mainItem.callState != LinphoneEnums.CallState.End && mainItem.callState != LinphoneEnums.CallState.Released @@ -109,7 +110,7 @@ Item{ anchors.rightMargin: 10 * DefaultStyle.dp anchors.bottomMargin: 10 * DefaultStyle.dp //participantDevice: allDevices.me - cameraEnabled: allDevices.count <= 2 + cameraEnabled: visible && mainItem.call && mainItem.call.core.cameraEnabled previewEnabled: true qmlName: 'P'