From cd2665fc55e2dc7a39856e90ae643258e3c92555 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 24 Apr 2017 18:10:50 -0400 Subject: [PATCH 1/6] standardize Audio bg thread joins --- interface/src/Application.cpp | 11 +-- libraries/audio-client/src/AudioClient.cpp | 89 +++++++++++++--------- libraries/audio-client/src/AudioClient.h | 38 +++------ 3 files changed, 69 insertions(+), 69 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index fbf28a8d99..c94b1bbd2f 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1683,7 +1683,6 @@ void Application::updateHeartbeat() const { void Application::aboutToQuit() { emit beforeAboutToQuit(); - DependencyManager::get()->beforeAboutToQuit(); foreach(auto inputPlugin, PluginManager::getInstance()->getInputPlugins()) { if (inputPlugin->isActive()) { @@ -1784,14 +1783,12 @@ void Application::cleanupBeforeQuit() { _snapshotSoundInjector->stop(); } - // stop audio after QML, as there are unexplained audio crashes originating in qtwebengine - - // stop the AudioClient, synchronously - QMetaObject::invokeMethod(DependencyManager::get().data(), - "stop", Qt::BlockingQueuedConnection); - + // FIXME: something else is holding a reference to AudioClient, + // so it must be explicitly synchronously stopped here + QMetaObject::invokeMethod(DependencyManager::get().data(), "stop", Qt::BlockingQueuedConnection); // destroy Audio so it and its threads have a chance to go down safely + // this must happen after QML, as there are unexplained audio crashes originating in qtwebengine DependencyManager::destroy(); DependencyManager::destroy(); diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 4a2de0a64b..6107fef516 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -76,42 +76,58 @@ using Mutex = std::mutex; using Lock = std::unique_lock; static Mutex _deviceMutex; -// background thread that continuously polls for device changes -class CheckDevicesThread : public QThread { +class BackgroundThread : public QThread { public: - const unsigned long DEVICE_CHECK_INTERVAL_MSECS = 2 * 1000; + BackgroundThread(AudioClient* client) : QThread((QObject*)client), _client(client) {} + virtual void join() = 0; +protected: + AudioClient* _client; +}; - CheckDevicesThread(AudioClient* audioClient) - : _audioClient(audioClient) { - } +// background thread continuously polling device changes +class CheckDevicesThread : public BackgroundThread { +public: + CheckDevicesThread(AudioClient* client) : BackgroundThread(client) {} - void beforeAboutToQuit() { - Lock lock(_checkDevicesMutex); + void join() override { _quit = true; + std::unique_lock lock(mutex); + cv.wait(lock, [&]{ return !running; }); } +protected: void run() override { - while (true) { - { - Lock lock(_checkDevicesMutex); - if (_quit) { - break; - } - _audioClient->checkDevices(); - } + while (!_quit) { + _client->checkDevices(); + + const unsigned long DEVICE_CHECK_INTERVAL_MSECS = 2 * 1000; QThread::msleep(DEVICE_CHECK_INTERVAL_MSECS); } + std::lock_guard lock(mutex); + running = false; + cv.notify_one(); } private: - AudioClient* _audioClient { nullptr }; - Mutex _checkDevicesMutex; - bool _quit { false }; + std::atomic _quit { false }; + bool running { true }; + std::mutex mutex; + std::condition_variable cv; }; -void AudioInjectorsThread::prepare() { - _audio->prepareLocalAudioInjectors(); -} +// background thread buffering local injectors +class LocalInjectorsThread : public BackgroundThread { + Q_OBJECT +public: + LocalInjectorsThread(AudioClient* client) : BackgroundThread(client) {} + + void join() override { return; } + +private slots: + void prepare() { _client->prepareLocalAudioInjectors(); } +}; + +#include "AudioClient.moc" static void channelUpmix(int16_t* source, int16_t* dest, int numSamples, int numExtraChannels) { for (int i = 0; i < numSamples/2; i++) { @@ -179,7 +195,6 @@ AudioClient::AudioClient() : _inputToNetworkResampler(NULL), _networkToOutputResampler(NULL), _localToOutputResampler(NULL), - _localAudioThread(this), _audioLimiter(AudioConstants::SAMPLE_RATE, OUTPUT_CHANNEL_COUNT), _outgoingAvatarAudioSequenceNumber(0), _audioOutputIODevice(_localInjectorsStream, _receivedAudioStream, this), @@ -210,13 +225,14 @@ AudioClient::AudioClient() : // start a thread to detect any device changes _checkDevicesThread = new CheckDevicesThread(this); - _checkDevicesThread->setObjectName("CheckDevices Thread"); + _checkDevicesThread->setObjectName("AudioClient CheckDevices Thread"); _checkDevicesThread->setPriority(QThread::LowPriority); _checkDevicesThread->start(); // start a thread to process local injectors - _localAudioThread.setObjectName("LocalAudio Thread"); - _localAudioThread.start(); + _localInjectorsThread = new LocalInjectorsThread(this); + _localInjectorsThread->setObjectName("AudioClient LocalInjectors Thread"); + _localInjectorsThread->start(); configureReverb(); @@ -231,18 +247,23 @@ AudioClient::AudioClient() : } AudioClient::~AudioClient() { - delete _checkDevicesThread; - stop(); if (_codec && _encoder) { _codec->releaseEncoder(_encoder); _encoder = nullptr; } } -void AudioClient::beforeAboutToQuit() { - static_cast(_checkDevicesThread)->beforeAboutToQuit(); -} +void AudioClient::customDeleter() { + stop(); // synchronously + static_cast(_checkDevicesThread)->join(); + delete _checkDevicesThread; + + static_cast(_localInjectorsThread)->join(); + delete _localInjectorsThread; + + deleteLater(); // asynchronously +} void AudioClient::handleMismatchAudioFormat(SharedNodePointer node, const QString& currentCodec, const QString& recievedCodec) { qCDebug(audioclient) << __FUNCTION__ << "sendingNode:" << *node << "currentCodec:" << currentCodec << "recievedCodec:" << recievedCodec; @@ -1527,8 +1548,8 @@ bool AudioClient::switchOutputToAudioDevice(const QAudioDeviceInfo& outputDevice _outputScratchBuffer = new int16_t[_outputPeriod]; // size local output mix buffer based on resampled network frame size - _networkPeriod = _localToOutputResampler->getMaxOutput(AudioConstants::NETWORK_FRAME_SAMPLES_STEREO); - _localOutputMixBuffer = new float[_networkPeriod]; + int networkPeriod = _localToOutputResampler->getMaxOutput(AudioConstants::NETWORK_FRAME_SAMPLES_STEREO); + _localOutputMixBuffer = new float[networkPeriod]; int localPeriod = _outputPeriod * 2; _localInjectorsStream.resizeForFrameSize(localPeriod); @@ -1691,7 +1712,7 @@ qint64 AudioClient::AudioOutputIODevice::readData(char * data, qint64 maxSize) { } // prepare injectors for the next callback - QMetaObject::invokeMethod(&_audio->_localAudioThread, "prepare", Qt::QueuedConnection); + QMetaObject::invokeMethod(_audio->_localInjectorsThread, "prepare", Qt::QueuedConnection); int samplesPopped = std::max(networkSamplesPopped, injectorSamplesPopped); int framesPopped = samplesPopped / AudioConstants::STEREO; diff --git a/libraries/audio-client/src/AudioClient.h b/libraries/audio-client/src/AudioClient.h index 139749e8e8..6e69915f0a 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -71,19 +71,6 @@ class QIODevice; class Transform; class NLPacket; -class AudioInjectorsThread : public QThread { - Q_OBJECT - -public: - AudioInjectorsThread(AudioClient* audio) : _audio(audio) {} - -public slots : - void prepare(); - -private: - AudioClient* _audio; -}; - class AudioClient : public AbstractAudioInterface, public Dependency { Q_OBJECT SINGLETON_DEPENDENCY @@ -186,8 +173,6 @@ public slots: void audioMixerKilled(); void toggleMute(); - void beforeAboutToQuit(); - virtual void setIsStereoInput(bool stereo) override; void toggleAudioNoiseReduction() { _isNoiseGateEnabled = !_isNoiseGateEnabled; } @@ -244,9 +229,7 @@ protected: AudioClient(); ~AudioClient(); - virtual void customDeleter() override { - deleteLater(); - } + virtual void customDeleter() override; private: void outputFormatChanged(); @@ -339,19 +322,17 @@ private: // for network audio (used by network audio thread) int16_t _networkScratchBuffer[AudioConstants::NETWORK_FRAME_SAMPLES_AMBISONIC]; - // for local audio (used by audio injectors thread) - int _networkPeriod { 0 }; - float _localMixBuffer[AudioConstants::NETWORK_FRAME_SAMPLES_STEREO]; - int16_t _localScratchBuffer[AudioConstants::NETWORK_FRAME_SAMPLES_AMBISONIC]; - float* _localOutputMixBuffer { NULL }; - AudioInjectorsThread _localAudioThread; - RecursiveMutex _localAudioMutex; - // for output audio (used by this thread) int _outputPeriod { 0 }; float* _outputMixBuffer { NULL }; int16_t* _outputScratchBuffer { NULL }; + // for local audio (used by audio injectors thread) + float _localMixBuffer[AudioConstants::NETWORK_FRAME_SAMPLES_STEREO]; + int16_t _localScratchBuffer[AudioConstants::NETWORK_FRAME_SAMPLES_AMBISONIC]; + float* _localOutputMixBuffer { NULL }; + RecursiveMutex _localAudioMutex; + AudioLimiter _audioLimiter; // Adds Reverb @@ -394,12 +375,13 @@ private: QString _selectedCodecName; Encoder* _encoder { nullptr }; // for outbound mic stream - QThread* _checkDevicesThread { nullptr }; - RateCounter<> _silentOutbound; RateCounter<> _audioOutbound; RateCounter<> _silentInbound; RateCounter<> _audioInbound; + + QThread* _checkDevicesThread { nullptr }; + QThread* _localInjectorsThread { nullptr }; }; From 4a26785eda43c42879d5758c5e1110d6005cf46e Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 24 Apr 2017 18:21:27 -0400 Subject: [PATCH 2/6] cleanup Audio bg threads in cleanupBeforeQuit --- interface/src/Application.cpp | 3 ++- libraries/audio-client/src/AudioClient.cpp | 21 +++++++++++++++------ libraries/audio-client/src/AudioClient.h | 1 + 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index c94b1bbd2f..ff6f85c8b2 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1785,7 +1785,8 @@ void Application::cleanupBeforeQuit() { // FIXME: something else is holding a reference to AudioClient, // so it must be explicitly synchronously stopped here - QMetaObject::invokeMethod(DependencyManager::get().data(), "stop", Qt::BlockingQueuedConnection); + QMetaObject::invokeMethod(DependencyManager::get().data(), + "cleanupBeforeQuit", Qt::BlockingQueuedConnection); // destroy Audio so it and its threads have a chance to go down safely // this must happen after QML, as there are unexplained audio crashes originating in qtwebengine diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 6107fef516..fbb578f560 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -254,15 +254,24 @@ AudioClient::~AudioClient() { } void AudioClient::customDeleter() { - stop(); // synchronously + deleteLater(); +} - static_cast(_checkDevicesThread)->join(); - delete _checkDevicesThread; +void AudioClient::cleanupBeforeQuit() { + // FIXME: this should be put in customDeleter, but there is still a reference to this when it is called, + // so this must be explicitly, synchronously stopped - static_cast(_localInjectorsThread)->join(); - delete _localInjectorsThread; + stop(); - deleteLater(); // asynchronously + if (_checkDevicesThread) { + static_cast(_checkDevicesThread)->join(); + delete _checkDevicesThread; + } + + if (_localInjectorsThread) { + static_cast(_localInjectorsThread)->join(); + delete _localInjectorsThread; + } } void AudioClient::handleMismatchAudioFormat(SharedNodePointer node, const QString& currentCodec, const QString& recievedCodec) { diff --git a/libraries/audio-client/src/AudioClient.h b/libraries/audio-client/src/AudioClient.h index 6e69915f0a..f3038b4d62 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -158,6 +158,7 @@ public: public slots: void start(); void stop(); + void cleanupBeforeQuit(); void handleAudioEnvironmentDataPacket(QSharedPointer message); void handleAudioDataPacket(QSharedPointer message); From c27dd446ebfdfd2654f0993fe970b1ff437988ce Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 25 Apr 2017 14:59:30 -0400 Subject: [PATCH 3/6] rename to standard --- libraries/audio-client/src/AudioClient.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index fbb578f560..9e1195e460 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -90,29 +90,29 @@ public: CheckDevicesThread(AudioClient* client) : BackgroundThread(client) {} void join() override { - _quit = true; - std::unique_lock lock(mutex); - cv.wait(lock, [&]{ return !running; }); + _shouldQuit = true; + std::unique_lock lock(_joinMutex); + _joinCondition.wait(lock, [&]{ return !_isRunning; }); } protected: void run() override { - while (!_quit) { + while (!_shouldQuit) { _client->checkDevices(); const unsigned long DEVICE_CHECK_INTERVAL_MSECS = 2 * 1000; QThread::msleep(DEVICE_CHECK_INTERVAL_MSECS); } - std::lock_guard lock(mutex); - running = false; - cv.notify_one(); + std::lock_guard lock(_joinCondition); + _isRunning = false; + _joinCondition.notify_one(); } private: - std::atomic _quit { false }; - bool running { true }; - std::mutex mutex; - std::condition_variable cv; + std::atomic _shouldQuit { false }; + bool _isRunning { true }; + std::mutex _joinMutex; + std::condition_variable _joinCondition; }; // background thread buffering local injectors From e9c5b8600367be234126f979088fd60ffe7789a5 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 25 Apr 2017 16:02:43 -0400 Subject: [PATCH 4/6] use mutex, not cv --- libraries/audio-client/src/AudioClient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 9e1195e460..2c78fbceca 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -103,7 +103,7 @@ protected: const unsigned long DEVICE_CHECK_INTERVAL_MSECS = 2 * 1000; QThread::msleep(DEVICE_CHECK_INTERVAL_MSECS); } - std::lock_guard lock(_joinCondition); + std::lock_guard lock(_joinMutex); _isRunning = false; _joinCondition.notify_one(); } From 73a3bf413bc3bdbd00cfb0f12768b0503803fa1a Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 8 May 2017 20:14:09 -0400 Subject: [PATCH 5/6] update local audio thread name --- libraries/audio-client/src/AudioClient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 72cb438eab..1561eabf05 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1358,7 +1358,7 @@ bool AudioClient::outputLocalInjector(AudioInjector* injector) { // move local buffer to the LocalAudioThread to avoid dataraces with AudioInjector (like stop()) injectorBuffer->setParent(nullptr); - injectorBuffer->moveToThread(&_localAudioThread); + injectorBuffer->moveToThread(_localInjectorsThread); } else { qCDebug(audioclient) << "injector exists in active list already"; } From 53e254152da25fc43362715c03b41e466e9238cf Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Tue, 9 May 2017 22:30:29 +0100 Subject: [PATCH 6/6] fixed puck location --- plugins/openvr/src/ViveControllerManager.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/openvr/src/ViveControllerManager.cpp b/plugins/openvr/src/ViveControllerManager.cpp index 8fedf926dc..db12d25e0d 100644 --- a/plugins/openvr/src/ViveControllerManager.cpp +++ b/plugins/openvr/src/ViveControllerManager.cpp @@ -41,7 +41,7 @@ void releaseOpenVrSystem(); static const char* CONTROLLER_MODEL_STRING = "vr_controller_05_wireless_b"; -const quint64 CALIBRATION_TIMELAPSE = 3 * USECS_PER_SECOND; +const quint64 CALIBRATION_TIMELAPSE = 2 * USECS_PER_SECOND; static const char* MENU_PARENT = "Avatar"; static const char* MENU_NAME = "Vive Controllers"; @@ -297,12 +297,12 @@ void ViveControllerManager::InputDevice::calibrate(const controller::InputCalibr // done } else if (_config == Config::FeetAndHips) { _jointToPuckMap[controller::HIPS] = _validTrackedObjects[HIP].first; - _pucksOffset[_validTrackedObjects[2].first] = computeOffset(defaultToReferenceMat, inputCalibration.defaultHips, _validTrackedObjects[2].second); + _pucksOffset[_validTrackedObjects[HIP].first] = computeOffset(defaultToReferenceMat, inputCalibration.defaultHips, _validTrackedObjects[HIP].second); } else if (_config == Config::FeetHipsAndChest) { _jointToPuckMap[controller::HIPS] = _validTrackedObjects[HIP].first; - _pucksOffset[_validTrackedObjects[2].first] = computeOffset(defaultToReferenceMat, inputCalibration.defaultHips, _validTrackedObjects[2].second); + _pucksOffset[_validTrackedObjects[HIP].first] = computeOffset(defaultToReferenceMat, inputCalibration.defaultHips, _validTrackedObjects[HIP].second); _jointToPuckMap[controller::SPINE2] = _validTrackedObjects[CHEST].first; - _pucksOffset[_validTrackedObjects[3].first] = computeOffset(defaultToReferenceMat, inputCalibration.defaultSpine2, _validTrackedObjects[3].second); + _pucksOffset[_validTrackedObjects[CHEST].first] = computeOffset(defaultToReferenceMat, inputCalibration.defaultSpine2, _validTrackedObjects[CHEST].second); } _calibrated = true; }