diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 1561eabf05..dae37ffc4b 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -1126,11 +1126,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) { @@ -1184,16 +1192,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) { @@ -1272,6 +1280,9 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) { _activeLocalAudioInjectors.removeOne(injector); } + // update the flag + _localInjectorsAvailable.exchange(!_activeLocalAudioInjectors.empty(), std::memory_order_release); + return true; } @@ -1359,10 +1370,13 @@ bool AudioClient::outputLocalInjector(AudioInjector* injector) { // move local buffer to the LocalAudioThread to avoid dataraces with AudioInjector (like stop()) injectorBuffer->setParent(nullptr); injectorBuffer->moveToThread(_localInjectorsThread); + + // update the flag + _localInjectorsAvailable.exchange(true, std::memory_order_release); } else { qCDebug(audioclient) << "injector exists in active list already"; } - + return true; } else { @@ -1485,7 +1499,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 @@ -1555,14 +1569,23 @@ bool AudioClient::switchOutputToAudioDevice(const QAudioDeviceInfo& outputDevice connect(_audioOutput, &QAudioOutput::stateChanged, [&, frameSize, requestedSize](QAudio::State state) { if (state == QAudio::ActiveState) { // restrict device callback to _outputPeriod samples - _outputPeriod = (_audioOutput->periodSize() / AudioConstants::SAMPLE_SIZE) * 2; + _outputPeriod = _audioOutput->periodSize() / AudioConstants::SAMPLE_SIZE; + // device callback may exceed reported period, so double it to avoid stutter + _outputPeriod *= 2; + _outputMixBuffer = new float[_outputPeriod]; _outputScratchBuffer = new int16_t[_outputPeriod]; // size local output mix buffer based on resampled network frame size int networkPeriod = _localToOutputResampler->getMaxOutput(AudioConstants::NETWORK_FRAME_SAMPLES_STEREO); _localOutputMixBuffer = new float[networkPeriod]; + + // local period should be at least twice the output period, + // in case two device reads happen before more data can be read (worst case) int localPeriod = _outputPeriod * 2; + // round up to an exact multiple of networkPeriod + localPeriod = ((localPeriod + networkPeriod - 1) / networkPeriod) * networkPeriod; + // this ensures lowest latency without stutter from underrun _localInjectorsStream.resizeForFrameSize(localPeriod); int bufferSize = _audioOutput->bufferSize(); @@ -1577,6 +1600,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); @@ -1715,12 +1741,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 4a9e51c16c..0e5363e0ff 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -145,7 +145,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; @@ -184,8 +184,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); @@ -231,8 +229,13 @@ protected: virtual void customDeleter() override; private: + friend class CheckDevicesThread; + friend class LocalInjectorsThread; + 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); @@ -279,8 +282,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;