Merge pull request #8253 from davidkelly/dk/localNoEcho

Injectors now always play locally, and send to server without echo

libraries/audio-client/src/AudioClient.cpp

@@ -862,6 +862,9 @@ void AudioClient::mixLocalAudioInjectors(int16_t* inputBuffer) {
     static const float INT16_TO_FLOAT_SCALE_FACTOR = 1/32768.0f;
 
     bool injectorsHaveData = false;
+    
+    // lock the injector vector
+    Lock lock(_injectorsMutex);
 
     for (AudioInjector* injector : getActiveLocalAudioInjectors()) {
         if (injector->getLocalBuffer()) {
@@ -871,6 +874,7 @@ void AudioClient::mixLocalAudioInjectors(int16_t* inputBuffer) {
                 AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL;
 
             // get one frame from the injector (mono or stereo)
+            memset(_scratchBuffer, 0, sizeof(_scratchBuffer));
             if (0 < injector->getLocalBuffer()->readData((char*)_scratchBuffer, samplesToRead)) {
                 
                 injectorsHaveData = true;
@@ -894,14 +898,14 @@ void AudioClient::mixLocalAudioInjectors(int16_t* inputBuffer) {
             } else {
                 
                 qDebug() << "injector has no more data, marking finished for removal";
-                injector->finish();
+                injector->finishLocalInjection();
                 injectorsToRemove.append(injector);
             }
 
         } else {
             
             qDebug() << "injector has no local buffer, marking as finished for removal";
-            injector->finish();
+            injector->finishLocalInjection();
             injectorsToRemove.append(injector);
         }
     }
@@ -1003,6 +1007,7 @@ void AudioClient::setIsStereoInput(bool isStereoInput) {
 
 
 bool AudioClient::outputLocalInjector(bool isStereo, AudioInjector* injector) {
+    Lock lock(_injectorsMutex);
     if (injector->getLocalBuffer() && _audioInput ) {
         // just add it to the vector of active local injectors, if 
         // not already there.

libraries/audio-client/src/AudioClient.h

@@ -15,6 +15,7 @@
 #include <fstream>
 #include <memory>
 #include <vector>
+#include <mutex>
 
 #include <QtCore/qsystemdetection.h>
 #include <QtCore/QByteArray>
@@ -83,6 +84,9 @@ public:
     using AudioPositionGetter = std::function<glm::vec3()>;
     using AudioOrientationGetter = std::function<glm::quat()>;
 
+    using Mutex = std::mutex;
+    using Lock = std::unique_lock<Mutex>;
+    
     class AudioOutputIODevice : public QIODevice {
     public:
         AudioOutputIODevice(MixedProcessedAudioStream& receivedAudioStream, AudioClient* audio) :
@@ -219,6 +223,7 @@ private:
     float azimuthForSource(const glm::vec3& relativePosition);
     float gainForSource(float distance, float volume);
 
+    Mutex _injectorsMutex;
     QByteArray firstInputFrame;
     QAudioInput* _audioInput;
     QAudioFormat _desiredInputFormat;

libraries/audio/src/AudioInjector.cpp

@@ -28,6 +28,15 @@
 
 int audioInjectorPtrMetaTypeId = qRegisterMetaType<AudioInjector*>();
 
+AudioInjectorState operator& (AudioInjectorState lhs, AudioInjectorState rhs) {
+    return static_cast<AudioInjectorState>(static_cast<uint8_t>(lhs) & static_cast<uint8_t>(rhs));
+};
+
+AudioInjectorState& operator|= (AudioInjectorState& lhs, AudioInjectorState rhs) {
+    lhs = static_cast<AudioInjectorState>(static_cast<uint8_t>(lhs) | static_cast<uint8_t>(rhs));
+    return lhs;
+};
+
 AudioInjector::AudioInjector(QObject* parent) :
     QObject(parent)
 {
@@ -48,6 +57,10 @@ AudioInjector::AudioInjector(const QByteArray& audioData, const AudioInjectorOpt
 
 }
 
+bool AudioInjector::stateHas(AudioInjectorState state) const {
+    return (_state & state) == state;
+}
+
 void AudioInjector::setOptions(const AudioInjectorOptions& options) {
     // since options.stereo is computed from the audio stream, 
     // we need to copy it from existing options just in case.
@@ -56,10 +69,25 @@ void AudioInjector::setOptions(const AudioInjectorOptions& options) {
     _options.stereo = currentlyStereo;
 }
 
+void AudioInjector::finishNetworkInjection() {
+    _state |= AudioInjectorState::NetworkInjectionFinished;
+    
+    // if we are already finished with local
+    // injection, then we are finished
+    if(stateHas(AudioInjectorState::LocalInjectionFinished)) {
+        finish();
+    }
+}
+
+void AudioInjector::finishLocalInjection() {
+    _state |= AudioInjectorState::LocalInjectionFinished;
+    if(_options.localOnly || stateHas(AudioInjectorState::NetworkInjectionFinished)) {
+        finish();
+    }
+}
 
 void AudioInjector::finish() {
-    bool shouldDelete = (_state == State::NotFinishedWithPendingDelete);
+    _state |= AudioInjectorState::Finished;
-    _state = State::Finished;
 
     emit finished();
 
@@ -69,7 +97,7 @@ void AudioInjector::finish() {
         _localBuffer = NULL;
     }
 
-    if (shouldDelete) {
+    if (stateHas(AudioInjectorState::PendingDelete)) {
         // we've been asked to delete after finishing, trigger a deleteLater here
         deleteLater();
     }
@@ -121,23 +149,31 @@ void AudioInjector::restart() {
     _hasSentFirstFrame = false;
 
     // check our state to decide if we need extra handling for the restart request
-    if (_state == State::Finished) {
+    if (stateHas(AudioInjectorState::Finished)) {
         // we finished playing, need to reset state so we can get going again
         _hasSetup = false;
         _shouldStop = false;
-        _state = State::NotFinished;
+        _state = AudioInjectorState::NotFinished;
         
         // call inject audio to start injection over again
         setupInjection();
 
-        // if we're a local injector, just inject again 
+        // inject locally
-        if (_options.localOnly) {
+        if(injectLocally()) {
-            injectLocally();
+        
-        } else {
+            // if not localOnly, wake the AudioInjectorManager back up if it is stuck waiting
-            // wake the AudioInjectorManager back up if it's stuck waiting
+            if (!_options.localOnly) {
-            if (!injectorManager->restartFinishedInjector(this)) {
+
-                _state = State::Finished; // we're not playing, so reset the state used by isPlaying.
+                if (!injectorManager->restartFinishedInjector(this)) {
+                    // TODO: this logic seems to remove the pending delete,
+                    // which makes me wonder about the deleteLater calls
+                    _state = AudioInjectorState::Finished; // we're not playing, so reset the state used by isPlaying.
+                }
             }
+        } else {
+            // TODO: this logic seems to remove the pending delete,
+            // which makes me wonder about the deleteLater calls
+            _state = AudioInjectorState::Finished; // we failed to play, so we are finished again
         }
     }
 }
@@ -183,7 +219,7 @@ static const int64_t NEXT_FRAME_DELTA_ERROR_OR_FINISHED = -1;
 static const int64_t NEXT_FRAME_DELTA_IMMEDIATELY = 0;
 
 int64_t AudioInjector::injectNextFrame() {
-    if (_state == AudioInjector::State::Finished) {
+    if (stateHas(AudioInjectorState::NetworkInjectionFinished)) {
         qDebug() << "AudioInjector::injectNextFrame called but AudioInjector has finished and was not restarted. Returning.";
         return NEXT_FRAME_DELTA_ERROR_OR_FINISHED;
     }
@@ -234,8 +270,10 @@ int64_t AudioInjector::injectNextFrame() {
             // pack the stereo/mono type of the stream
             audioPacketStream << _options.stereo;
 
-            // pack the flag for loopback
+            // pack the flag for loopback.  Now, we don't loopback
-            uchar loopbackFlag = (uchar)true;
+            // and _always_ play locally, so loopbackFlag should be 
+            // false always.
+            uchar loopbackFlag = (uchar)false;
             audioPacketStream << loopbackFlag;
 
             // pack the position for injected audio
@@ -333,7 +371,7 @@ int64_t AudioInjector::injectNextFrame() {
     }
 
     if (_currentSendOffset >= _audioData.size() && !_options.loop) {
-        finish();
+        finishNetworkInjection();
         return NEXT_FRAME_DELTA_ERROR_OR_FINISHED;
     }
 
@@ -372,10 +410,10 @@ void AudioInjector::triggerDeleteAfterFinish() {
         return;
     }
 
-    if (_state == State::Finished) {
+    if (_state == AudioInjectorState::Finished) {
         stopAndDeleteLater();
     } else {
-        _state = State::NotFinishedWithPendingDelete;
+        _state |= AudioInjectorState::PendingDelete;
     }
 }
 
@@ -421,7 +459,7 @@ AudioInjector* AudioInjector::playSoundAndDelete(const QByteArray& buffer, const
     AudioInjector* sound = playSound(buffer, options, localInterface);
 
     if (sound) {
-        sound->_state = AudioInjector::State::NotFinishedWithPendingDelete;
+        sound->_state |= AudioInjectorState::PendingDelete;
     }
 
     return sound;
@@ -438,21 +476,23 @@ AudioInjector* AudioInjector::playSound(const QByteArray& buffer, const AudioInj
     // setup parameters required for injection
     injector->setupInjection();
 
-    if (options.localOnly) {
+    // we always inject locally
-        if (injector->injectLocally()) {
+    //
-            // local injection succeeded, return the pointer to injector
+    if (!injector->injectLocally()) {
-            return injector;
+        // failed, so don't bother sending to server
-        } else {
+        qDebug() << "AudioInjector::playSound failed to inject locally";
-            // unable to inject locally, return a nullptr
+        return nullptr;
-            return nullptr;
-        }
-    } else {
-        // attempt to thread the new injector
-        if (injectorManager->threadInjector(injector)) {
-            return injector;
-        } else {
-            // we failed to thread the new injector (we are at the max number of injector threads)
-            return nullptr;
-        }
     }
+    // if localOnly, we are done, just return injector.
+    if (options.localOnly) {
+        return injector;
+    }
+
+    // send off to server for everyone else
+    if (!injectorManager->threadInjector(injector)) {
+        // we failed to thread the new injector (we are at the max number of injector threads)
+        qDebug() << "AudioInjector::playSound failed to thread injector";
+    }
+    return injector;
+
 }

libraries/audio/src/AudioInjector.h

@@ -32,24 +32,35 @@
 class AbstractAudioInterface;
 class AudioInjectorManager;
 
+
+enum class AudioInjectorState : uint8_t {
+    NotFinished = 1,
+    Finished = 2,
+    PendingDelete = 4,
+    LocalInjectionFinished = 8,
+    NetworkInjectionFinished = 16
+};
+
+AudioInjectorState operator& (AudioInjectorState lhs, AudioInjectorState rhs);
+AudioInjectorState& operator|= (AudioInjectorState& lhs, AudioInjectorState rhs);
+
 // In order to make scripting cleaner for the AudioInjector, the script now holds on to the AudioInjector object
 // until it dies. 
-
 class AudioInjector : public QObject {
     Q_OBJECT
     
 public:
-    enum class State : uint8_t {
+    static const uint8_t NotFinished = 1;
-        NotFinished,
+    static const uint8_t Finished = 2;
-        NotFinishedWithPendingDelete,
+    static const uint8_t PendingDelete = 4;
-        Finished
+    static const uint8_t LocalInjectionFinished = 8;
-    };
+    static const uint8_t NetworkInjectionFinished = 16;
-    
+
     AudioInjector(QObject* parent);
     AudioInjector(const Sound& sound, const AudioInjectorOptions& injectorOptions);
     AudioInjector(const QByteArray& audioData, const AudioInjectorOptions& injectorOptions);
     
-    bool isFinished() const { return _state == State::Finished; }
+    bool isFinished() const { return (stateHas(AudioInjectorState::Finished)); }
     
     int getCurrentSendOffset() const { return _currentSendOffset; }
     void setCurrentSendOffset(int currentSendOffset) { _currentSendOffset = currentSendOffset; }
@@ -63,6 +74,7 @@ public:
     bool isStereo() const { return _options.stereo; }
     void setLocalAudioInterface(AbstractAudioInterface* localAudioInterface) { _localAudioInterface = localAudioInterface; }
 
+    bool stateHas(AudioInjectorState state) const ;
     static AudioInjector* playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options, AbstractAudioInterface* localInterface);
     static AudioInjector* playSound(const QByteArray& buffer, const AudioInjectorOptions options, AbstractAudioInterface* localInterface);
     static AudioInjector* playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position);
@@ -78,8 +90,10 @@ public slots:
     void setOptions(const AudioInjectorOptions& options);
     
     float getLoudness() const { return _loudness; }
-    bool isPlaying() const { return _state == State::NotFinished || _state == State::NotFinishedWithPendingDelete; }
+    bool isPlaying() const { return stateHas(AudioInjectorState::NotFinished); }
     void finish();
+    void finishLocalInjection();
+    void finishNetworkInjection();
     
 signals:
     void finished();
@@ -92,7 +106,7 @@ private:
     
     QByteArray _audioData;
     AudioInjectorOptions _options;
-    State _state { State::NotFinished };
+    AudioInjectorState _state { AudioInjectorState::NotFinished };
     bool _hasSentFirstFrame { false };
     bool _hasSetup { false };
     bool _shouldStop { false };
@@ -111,4 +125,5 @@ private:
     friend class AudioInjectorManager;
 };
 
+    
 #endif // hifi_AudioInjector_h