From 061668cba41bb76a77d0147cec536a724ba960ef Mon Sep 17 00:00:00 2001
From: Zach Pomerantz <zmp@umich.edu>
Date: Sat, 21 Jan 2017 19:10:34 -0500
Subject: [PATCH] use lock-free pipe for local audio in device callback

---
 libraries/audio-client/src/AudioClient.cpp | 13 +++++++++++--
 libraries/audio-client/src/AudioClient.h   |  3 +++
 libraries/audio/src/AudioRingBuffer.h      |  6 ++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp
index b12c48ef82..2e532d67bf 100644
--- a/libraries/audio-client/src/AudioClient.cpp
+++ b/libraries/audio-client/src/AudioClient.cpp
@@ -190,6 +190,9 @@ AudioClient::AudioClient() :
     _inputGate(),
     _positionGetter(DEFAULT_POSITION_GETTER),
     _orientationGetter(DEFAULT_ORIENTATION_GETTER) {
+    // avoid putting a lock in the device callback
+    assert(_localSamplesAvailable.is_lock_free());
+
     // deprecate legacy settings
     {
         Setting::Handle<int>::Deprecated("maxFramesOverDesired", InboundAudioStream::MAX_FRAMES_OVER_DESIRED);
@@ -1105,7 +1108,8 @@ void AudioClient::prepareLocalAudioInjectors() {
 
     int bufferCapacity = _localInjectorsStream.getSampleCapacity();
     if (_localToOutputResampler) {
-        // avoid overwriting the buffer
+        // avoid overwriting the buffer,
+        // instead of failing on writes because the buffer is used as a lock-free pipe
         bufferCapacity -=
             _localToOutputResampler->getMaxOutput(AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL) *
             AudioConstants::STEREO;
@@ -1115,9 +1119,10 @@ void AudioClient::prepareLocalAudioInjectors() {
     int samplesNeeded = std::numeric_limits<int>::max();
     while (samplesNeeded > 0) {
         // lock for every write to avoid locking out the device callback
+        // this lock is intentional - the buffer is only lock-free in its use in the device callback
         Lock lock(_localAudioMutex);
 
-        samplesNeeded = bufferCapacity - _localInjectorsStream.samplesAvailable();
+        samplesNeeded = bufferCapacity - _localSamplesAvailable.load(std::memory_order_relaxed);
         if (samplesNeeded <= 0) {
             break;
         }
@@ -1149,6 +1154,7 @@ void AudioClient::prepareLocalAudioInjectors() {
                 AudioConstants::NETWORK_FRAME_SAMPLES_STEREO);
         }
 
+        _localSamplesAvailable.fetch_add(samples, std::memory_order_release);
         samplesNeeded -= samples;
     }
 }
@@ -1452,6 +1458,7 @@ bool AudioClient::switchOutputToAudioDevice(const QAudioDeviceInfo& outputDevice
     bool supportedFormat = false;
 
     Lock lock(_localAudioMutex);
+    _localSamplesAvailable.exchange(0, std::memory_order_release);
 
     // cleanup any previously initialized device
     if (_audioOutput) {
@@ -1666,7 +1673,9 @@ qint64 AudioClient::AudioOutputIODevice::readData(char * data, qint64 maxSize) {
     {
         Lock lock(_audio->_localAudioMutex);
         bool append = networkSamplesPopped > 0;
+        samplesRequested = std::min(samplesRequested, _audio->_localSamplesAvailable.load(std::memory_order_acquire));
         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 b0eaa2dd34..699ba71ef7 100644
--- a/libraries/audio-client/src/AudioClient.h
+++ b/libraries/audio-client/src/AudioClient.h
@@ -280,6 +280,9 @@ private:
     QIODevice* _loopbackOutputDevice;
     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
+    std::atomic<int> _localSamplesAvailable { 0 };
     MixedProcessedAudioStream _receivedAudioStream;
     bool _isStereoInput;
 
diff --git a/libraries/audio/src/AudioRingBuffer.h b/libraries/audio/src/AudioRingBuffer.h
index e43fd22548..bb32df19a2 100644
--- a/libraries/audio/src/AudioRingBuffer.h
+++ b/libraries/audio/src/AudioRingBuffer.h
@@ -45,6 +45,12 @@ public:
     // FIXME: discards any data in the buffer
     void resizeForFrameSize(int numFrameSamples);
 
+    // Reading and writing to the buffer uses minimal shared data, such that
+    // in cases that avoid overwriting the buffer, a single producer/consumer
+    // may use this as a lock-free pipe (see audio-client/src/AudioClient.cpp).
+    // IMPORTANT: Avoid changes to the implementation that touch shared data unless you can
+    // maintain this behavior.
+
     /// Read up to maxSamples into destination (will only read up to samplesAvailable())
     /// Returns number of read samples
     int readSamples(Sample* destination, int maxSamples);