From ff6f8ea4a18b1f12c3e4bdaf668b19f6ad98b559 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 12 Feb 2015 13:14:30 -0800 Subject: [PATCH 1/4] cleaner cleanup of AudioClient from RenderingClient --- gvr-interface/src/Client.h | 3 +++ gvr-interface/src/GVRInterface.cpp | 7 +++++++ gvr-interface/src/GVRInterface.h | 1 + gvr-interface/src/RenderingClient.cpp | 16 ++++++---------- gvr-interface/src/RenderingClient.h | 3 ++- libraries/audio-client/src/AudioClient.cpp | 3 ++- libraries/audio-client/src/AudioClient.h | 4 ++++ 7 files changed, 25 insertions(+), 12 deletions(-) diff --git a/gvr-interface/src/Client.h b/gvr-interface/src/Client.h index 6eb3913ea9..31bc8be3bd 100644 --- a/gvr-interface/src/Client.h +++ b/gvr-interface/src/Client.h @@ -22,7 +22,10 @@ class Client : public QObject { Q_OBJECT public: Client(QObject* parent = 0); + + virtual void cleanupBeforeQuit() = 0; protected: + void setupNetworking(); virtual void processVerifiedPacket(const HifiSockAddr& senderSockAddr, const QByteArray& incomingPacket); private slots: diff --git a/gvr-interface/src/GVRInterface.cpp b/gvr-interface/src/GVRInterface.cpp index 7476bfc764..3d58396322 100644 --- a/gvr-interface/src/GVRInterface.cpp +++ b/gvr-interface/src/GVRInterface.cpp @@ -86,6 +86,13 @@ GVRInterface::GVRInterface(int argc, char* argv[]) : QTimer* idleTimer = new QTimer(this); connect(idleTimer, &QTimer::timeout, this, &GVRInterface::idle); idleTimer->start(0); + + // call our quit handler before we go down + connect(this, &QCoreApplication::aboutToQuit, this, &GVRInterface::handleApplicationQuit); +} + +void GVRInterface::handleApplicationQuit() { + _client->cleanupBeforeQuit(); } void GVRInterface::idle() { diff --git a/gvr-interface/src/GVRInterface.h b/gvr-interface/src/GVRInterface.h index 926c44da15..9ffbd52909 100644 --- a/gvr-interface/src/GVRInterface.h +++ b/gvr-interface/src/GVRInterface.h @@ -53,6 +53,7 @@ private slots: void handleApplicationStateChange(Qt::ApplicationState state); void idle(); private: + void handleApplicationQuit(); void enterVRMode(); void leaveVRMode(); diff --git a/gvr-interface/src/RenderingClient.cpp b/gvr-interface/src/RenderingClient.cpp index 9737263eee..1abf4bbc95 100644 --- a/gvr-interface/src/RenderingClient.cpp +++ b/gvr-interface/src/RenderingClient.cpp @@ -37,7 +37,7 @@ RenderingClient::RenderingClient(QObject *parent, const QString& launchURLString DependencyManager::set(); // get our audio client setup on its own thread - QThread* audioThread = new QThread(this); + QThread* audioThread = new QThread(); auto audioClient = DependencyManager::set(); audioClient->setPositionGetter(getPositionForAudio); @@ -45,6 +45,8 @@ RenderingClient::RenderingClient(QObject *parent, const QString& launchURLString audioClient->moveToThread(audioThread); connect(audioThread, &QThread::started, audioClient.data(), &AudioClient::start); + connect(audioClient.data(), &AudioClient::destroyed, audioThread, &QThread::quit); + connect(audioThread, &QThread::finished, audioThread, &QThread::deleteLater); audioThread->start(); @@ -68,15 +70,9 @@ void RenderingClient::sendAvatarPacket() { _fakeAvatar.sendIdentityPacket(); } -RenderingClient::~RenderingClient() { - auto audioClient = DependencyManager::get(); - - // stop the audio client - QMetaObject::invokeMethod(audioClient.data(), "stop", Qt::BlockingQueuedConnection); - - // ask the audio thread to quit and wait until it is done - audioClient->thread()->quit(); - audioClient->thread()->wait(); +void RenderingClient::cleanupBeforeQuit() { + // destroy the AudioClient so it and it's thread will safely go down + DependencyManager::destroy(); } void RenderingClient::processVerifiedPacket(const HifiSockAddr& senderSockAddr, const QByteArray& incomingPacket) { diff --git a/gvr-interface/src/RenderingClient.h b/gvr-interface/src/RenderingClient.h index 870bde748f..c4724bc086 100644 --- a/gvr-interface/src/RenderingClient.h +++ b/gvr-interface/src/RenderingClient.h @@ -26,7 +26,6 @@ class RenderingClient : public Client { Q_OBJECT public: RenderingClient(QObject* parent = 0, const QString& launchURLString = QString()); - ~RenderingClient(); const glm::vec3& getPosition() const { return _position; } const glm::quat& getOrientation() const { return _orientation; } @@ -35,6 +34,8 @@ public: static glm::vec3 getPositionForAudio() { return _instance->getPosition(); } static glm::quat getOrientationForAudio() { return _instance->getOrientation(); } + virtual void cleanupBeforeQuit(); + private slots: void goToLocation(const glm::vec3& newPosition, bool hasOrientationChange, const glm::quat& newOrientation, diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 1919f2bbe5..a45fae94dc 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -136,6 +136,8 @@ AudioClient::AudioClient() : } AudioClient::~AudioClient() { + stop(); + if (_gverbLocal) { gverb_free(_gverbLocal); } @@ -489,7 +491,6 @@ void AudioClient::start() { } void AudioClient::stop() { - _inputFrameBuffer.finalize(); _inputGain.finalize(); _sourceGain.finalize(); diff --git a/libraries/audio-client/src/AudioClient.h b/libraries/audio-client/src/AudioClient.h index e55a116e06..4f16500103 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -188,6 +188,10 @@ protected: AudioClient(); ~AudioClient(); + virtual void customDeleter() { + deleteLater(); + } + private: void outputFormatChanged(); From 6c25205856cbace0494e306d22533db3c7efa145 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 12 Feb 2015 13:35:22 -0800 Subject: [PATCH 2/4] fix audio cleanup and close action in Application --- gvr-interface/src/RenderingClient.cpp | 4 +++ interface/src/Application.cpp | 35 +++++++++++-------- interface/src/Application.h | 2 ++ .../networking/src/UserActivityLogger.cpp | 10 +----- libraries/networking/src/UserActivityLogger.h | 2 +- 5 files changed, 28 insertions(+), 25 deletions(-) diff --git a/gvr-interface/src/RenderingClient.cpp b/gvr-interface/src/RenderingClient.cpp index 1abf4bbc95..8f7755a8c5 100644 --- a/gvr-interface/src/RenderingClient.cpp +++ b/gvr-interface/src/RenderingClient.cpp @@ -71,6 +71,10 @@ void RenderingClient::sendAvatarPacket() { } void RenderingClient::cleanupBeforeQuit() { + + QMetaObject::invokeMethod(DependencyManager::get().data(), + "stop", Qt::BlockingQueuedConnection); + // destroy the AudioClient so it and it's thread will safely go down DependencyManager::destroy(); } diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 474913fefb..aad1d51ac0 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -328,7 +328,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : connect(&nodeList->getNodeSocket(), SIGNAL(readyRead()), &_datagramProcessor, SLOT(processDatagrams())); // put the audio processing on a separate thread - QThread* audioThread = new QThread(this); + QThread* audioThread = new QThread(); audioThread->setObjectName("Audio Thread"); auto audioIO = DependencyManager::get(); @@ -338,7 +338,9 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : audioIO->moveToThread(audioThread); connect(audioThread, &QThread::started, audioIO.data(), &AudioClient::start); - connect(audioIO.data(), SIGNAL(muteToggled()), this, SLOT(audioMuteToggled())); + connect(audioIO.data(), &AudioClient::destroyed, audioThread, &QThread::quit); + connect(audioThread, &QThread::finished, audioThread, &QThread::deleteLater); + connect(audioIO.data(), &AudioClient::muteToggled, this, &Application::audioMuteToggled); audioThread->start(); @@ -516,21 +518,33 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : void Application::aboutToQuit() { _aboutToQuit = true; + + cleanupBeforeQuit(); } -Application::~Application() { +void Application::cleanupBeforeQuit() { QMetaObject::invokeMethod(&_settingsTimer, "stop", Qt::BlockingQueuedConnection); _settingsThread.quit(); saveSettings(); + // TODO: now that this is in cleanupBeforeQuit do we really need it to stop and force + // an event loop to send the packet? + UserActivityLogger::getInstance().close(); + + // stop the AudioClient + QMetaObject::invokeMethod(DependencyManager::get().data(), + "stop", Qt::BlockingQueuedConnection); + + // destroy the AudioClient so it and its thread have a chance to go down safely + DependencyManager::destroy(); +} + +Application::~Application() { _entities.getTree()->setSimulation(NULL); qInstallMessageHandler(NULL); _window->saveGeometry(); - int DELAY_TIME = 1000; - UserActivityLogger::getInstance().close(DELAY_TIME); - // make sure we don't call the idle timer any more delete idleTimer; @@ -541,15 +555,6 @@ Application::~Application() { _nodeThread->quit(); _nodeThread->wait(); - auto audioIO = DependencyManager::get(); - - // stop the audio process - QMetaObject::invokeMethod(audioIO.data(), "stop", Qt::BlockingQueuedConnection); - - // ask the audio thread to quit and wait until it is done - audioIO->thread()->quit(); - audioIO->thread()->wait(); - _octreeProcessor.terminate(); _entityEditSender.terminate(); diff --git a/interface/src/Application.h b/interface/src/Application.h index d015d09035..da29920ef9 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -410,6 +410,8 @@ private: void initDisplay(); void init(); + + void cleanupBeforeQuit(); void update(float deltaTime); diff --git a/libraries/networking/src/UserActivityLogger.cpp b/libraries/networking/src/UserActivityLogger.cpp index 4fc6905e3a..f2646369c1 100644 --- a/libraries/networking/src/UserActivityLogger.cpp +++ b/libraries/networking/src/UserActivityLogger.cpp @@ -86,17 +86,9 @@ void UserActivityLogger::launch(QString applicationVersion) { logAction(ACTION_NAME, actionDetails); } -void UserActivityLogger::close(int delayTime) { +void UserActivityLogger::close() { const QString ACTION_NAME = "close"; - - // In order to get the end of the session, we need to give the account manager enough time to send the packet. - QEventLoop loop; - QTimer timer; - connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); - // Now we can log it logAction(ACTION_NAME, QJsonObject()); - timer.start(delayTime); - loop.exec(); } void UserActivityLogger::changedDisplayName(QString displayName) { diff --git a/libraries/networking/src/UserActivityLogger.h b/libraries/networking/src/UserActivityLogger.h index 9b100461ee..295ad5ee8d 100644 --- a/libraries/networking/src/UserActivityLogger.h +++ b/libraries/networking/src/UserActivityLogger.h @@ -30,7 +30,7 @@ public slots: void logAction(QString action, QJsonObject details = QJsonObject(), JSONCallbackParameters params = JSONCallbackParameters()); void launch(QString applicationVersion); - void close(int delayTime); + void close(); void changedDisplayName(QString displayName); void changedModel(QString typeOfModel, QString modelURL); void changedDomain(QString domainURL); From 4e71e128f3d6f8bf1ce0be722d1aa0f4a61021dd Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 12 Feb 2015 13:42:36 -0800 Subject: [PATCH 3/4] remove QThread not currently used in client --- gvr-interface/src/Client.cpp | 2 -- gvr-interface/src/Client.h | 2 -- 2 files changed, 4 deletions(-) diff --git a/gvr-interface/src/Client.cpp b/gvr-interface/src/Client.cpp index e323ab96b5..65238ad784 100644 --- a/gvr-interface/src/Client.cpp +++ b/gvr-interface/src/Client.cpp @@ -9,8 +9,6 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#include - #include #include #include diff --git a/gvr-interface/src/Client.h b/gvr-interface/src/Client.h index 31bc8be3bd..6fbe40f165 100644 --- a/gvr-interface/src/Client.h +++ b/gvr-interface/src/Client.h @@ -16,8 +16,6 @@ #include -class QThread; - class Client : public QObject { Q_OBJECT public: From 30b371559dca50c963472d885b68befe4516bd53 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 12 Feb 2015 15:11:24 -0800 Subject: [PATCH 4/4] fix a typo in RenderingClient --- gvr-interface/src/RenderingClient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gvr-interface/src/RenderingClient.cpp b/gvr-interface/src/RenderingClient.cpp index 8f7755a8c5..e6d2f6b585 100644 --- a/gvr-interface/src/RenderingClient.cpp +++ b/gvr-interface/src/RenderingClient.cpp @@ -75,7 +75,7 @@ void RenderingClient::cleanupBeforeQuit() { QMetaObject::invokeMethod(DependencyManager::get().data(), "stop", Qt::BlockingQueuedConnection); - // destroy the AudioClient so it and it's thread will safely go down + // destroy the AudioClient so it and its thread will safely go down DependencyManager::destroy(); }