diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 1266c4a499..f56d31eea2 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1096,11 +1096,19 @@ void AudioClient::handleRecordedAudioInput(const QByteArray& audio) { handleAudioInput(audioBuffer); } -void AudioClient::prepareLocalAudioInjectors() { +void AudioClient::prepareLocalAudioInjectors(std::unique_ptr localAudioLock) { + bool doSynchronously = localAudioLock.operator bool(); + if (!localAudioLock) { + localAudioLock.reset(new Lock(_localAudioMutex)); + } + int samplesNeeded = std::numeric_limits::max(); while (samplesNeeded > 0) { - // unlock between every write to allow device switching - Lock lock(_localAudioMutex); + if (!doSynchronously) { + // unlock between every write to allow device switching + localAudioLock->unlock(); + localAudioLock->lock(); + } // in case of a device switch, consider bufferCapacity volatile across iterations if (_outputPeriod == 0) { @@ -1154,16 +1162,16 @@ void AudioClient::prepareLocalAudioInjectors() { } bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { - - QVector injectorsToRemove; - - // lock the injector vector - Lock lock(_injectorsMutex); - - if (_activeLocalAudioInjectors.size() == 0) { + // check the flag for injectors before attempting to lock + if (!_localInjectorsAvailable.load(std::memory_order_acquire)) { return false; } + // lock the injectors + Lock lock(_injectorsMutex); + + QVector injectorsToRemove; + memset(mixBuffer, 0, AudioConstants::NETWORK_FRAME_SAMPLES_STEREO * sizeof(float)); for (AudioInjector* injector : _activeLocalAudioInjectors) { @@ -1242,6 +1250,9 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { _activeLocalAudioInjectors.removeOne(injector); } + // update the flag + _localInjectorsAvailable.exchange(!_activeLocalAudioInjectors.empty(), std::memory_order_release); + return true; } @@ -1329,6 +1340,9 @@ bool AudioClient::outputLocalInjector(AudioInjector* injector) { // move local buffer to the LocalAudioThread to avoid dataraces with AudioInjector (like stop()) injectorBuffer->setParent(nullptr); injectorBuffer->moveToThread(&_localAudioThread); + + // update the flag now that injectors are available + _localInjectorsAvailable.exchange(true, std::memory_order_release); } else { qCDebug(audioclient) << "injector exists in active list already"; } @@ -1455,7 +1469,7 @@ void AudioClient::outputNotify() { bool AudioClient::switchOutputToAudioDevice(const QAudioDeviceInfo& outputDeviceInfo) { bool supportedFormat = false; - Lock lock(_localAudioMutex); + Lock localAudioLock(_localAudioMutex); _localSamplesAvailable.exchange(0, std::memory_order_release); // cleanup any previously initialized device @@ -1556,6 +1570,9 @@ bool AudioClient::switchOutputToAudioDevice(const QAudioDeviceInfo& outputDevice qCDebug(audioclient) << "local buffer (samples):" << localPeriod; disconnect(_audioOutput, &QAudioOutput::stateChanged, 0, 0); + + // unlock to avoid a deadlock with the device callback (which always succeeds this initialization) + localAudioLock.unlock(); } }); connect(_audioOutput, &QAudioOutput::notify, this, &AudioClient::outputNotify); @@ -1694,12 +1711,24 @@ qint64 AudioClient::AudioOutputIODevice::readData(char * data, qint64 maxSize) { int injectorSamplesPopped = 0; { bool append = networkSamplesPopped > 0; - // this does not require a lock as of the only two functions adding to _localSamplesAvailable (samples count): + // check the samples we have available locklessly; this is possible because only two functions add to the count: // - prepareLocalAudioInjectors will only increase samples count - // - switchOutputToAudioDevice will zero samples count - // stop the device, so that readData will exhaust the existing buffer or see a zeroed samples count - // and start the device, which can only see a zeroed samples count - samplesRequested = std::min(samplesRequested, _audio->_localSamplesAvailable.load(std::memory_order_acquire)); + // - switchOutputToAudioDevice will zero samples count, + // stop the device - so that readData will exhaust the existing buffer or see a zeroed samples count, + // and start the device - which can then only see a zeroed samples count + int samplesAvailable = _audio->_localSamplesAvailable.load(std::memory_order_acquire); + + // if we do not have enough samples buffered despite having injectors, buffer them synchronously + if (samplesAvailable < samplesRequested && _audio->_localInjectorsAvailable.load(std::memory_order_acquire)) { + // try_to_lock, in case the device is being shut down already + std::unique_ptr localAudioLock(new Lock(_audio->_localAudioMutex, std::try_to_lock)); + if (localAudioLock->owns_lock()) { + _audio->prepareLocalAudioInjectors(std::move(localAudioLock)); + samplesAvailable = _audio->_localSamplesAvailable.load(std::memory_order_acquire); + } + } + + samplesRequested = std::min(samplesRequested, samplesAvailable); if ((injectorSamplesPopped = _localInjectorsStream.appendSamples(mixBuffer, samplesRequested, append)) > 0) { _audio->_localSamplesAvailable.fetch_sub(injectorSamplesPopped, std::memory_order_release); qCDebug(audiostream, "Read %d samples from injectors (%d available, %d requested)", injectorSamplesPopped, _localInjectorsStream.samplesAvailable(), samplesRequested); diff --git a/libraries/audio-client/src/AudioClient.h b/libraries/audio-client/src/AudioClient.h index aaedee7456..3e9ba0fed9 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -158,7 +158,7 @@ public: Q_INVOKABLE void setAvatarBoundingBoxParameters(glm::vec3 corner, glm::vec3 scale); - void checkDevices(); + bool outputLocalInjector(AudioInjector* injector) override; static const float CALLBACK_ACCELERATOR_RATIO; @@ -198,8 +198,6 @@ public slots: int setOutputBufferSize(int numFrames, bool persist = true); - void prepareLocalAudioInjectors(); - bool outputLocalInjector(AudioInjector* injector) override; bool shouldLoopbackInjectors() override { return _shouldEchoToServer; } bool switchInputToAudioDevice(const QString& inputDeviceName); @@ -247,8 +245,13 @@ protected: } private: + friend class CheckDevicesThread; + friend class AudioInjectorsThread; + void outputFormatChanged(); void handleAudioInput(QByteArray& audioBuffer); + void checkDevices(); + void prepareLocalAudioInjectors(std::unique_ptr localAudioLock = nullptr); bool mixLocalAudioInjectors(float* mixBuffer); float azimuthForSource(const glm::vec3& relativePosition); float gainForSource(float distance, float volume); @@ -295,8 +298,9 @@ private: AudioRingBuffer _inputRingBuffer; LocalInjectorsStream _localInjectorsStream; // In order to use _localInjectorsStream as a lock-free pipe, - // use it with a single producer/consumer, and track available samples + // use it with a single producer/consumer, and track available samples and injectors std::atomic _localSamplesAvailable { 0 }; + std::atomic _localInjectorsAvailable { false }; MixedProcessedAudioStream _receivedAudioStream; bool _isStereoInput; diff --git a/libraries/audio/src/AbstractAudioInterface.h b/libraries/audio/src/AbstractAudioInterface.h index 2e4611cd4e..2e14b9956b 100644 --- a/libraries/audio/src/AbstractAudioInterface.h +++ b/libraries/audio/src/AbstractAudioInterface.h @@ -32,12 +32,12 @@ public: const Transform& transform, glm::vec3 avatarBoundingBoxCorner, glm::vec3 avatarBoundingBoxScale, PacketType packetType, QString codecName = QString("")); -public slots: // threadsafe // moves injector->getLocalBuffer() to another thread (so removes its parent) // take care to delete it when ~AudioInjector, as parenting Qt semantics will not work virtual bool outputLocalInjector(AudioInjector* injector) = 0; +public slots: virtual bool shouldLoopbackInjectors() { return false; } virtual void setIsStereoInput(bool stereo) = 0;