cleanup management of HRTF objects across connected Nodes

assignment-client/src/audio/AudioMixer.cpp

@@ -175,18 +175,11 @@ float AudioMixer::azimuthForSource(const PositionalAudioStream& streamToAdd, con
     return glm::orientedAngle(glm::vec3(0.0f, 0.0f, -1.0f), glm::normalize(rotatedSourcePosition), glm::vec3(0.0f, 1.0f, 0.0f));
 }
 
-void AudioMixer::addStreamToMixForListeningNodeWithStream(ListenerSourceIDPair idPair,
+void AudioMixer::addStreamToMixForListeningNodeWithStream(AudioMixerClientData& listenerNodeData,
-                                                          const AudioMixerClientData& listenerNodeData,
                                                           const PositionalAudioStream& streamToAdd,
+                                                          const QUuid& sourceNodeID,
                                                           const AvatarAudioStream& listeningNodeStream) {
 
-    // get the existing listener-source HRTF object, or create a new one
-
-    if (_hrtfMap.find(idPair) == _hrtfMap.end()) {
-        qDebug() << "Setting up a new HRTF for" << idPair.first << idPair.second;
-    }
-
-    auto& hrtf = _hrtfMap[idPair];
 
     // to reduce artifacts we calculate the gain and azimuth for every source for this listener
     // even if we are not going to end up mixing in this source
@@ -237,7 +230,10 @@ void AudioMixer::addStreamToMixForListeningNodeWithStream(ListenerSourceIDPair i
             // this ensures the correct tail from the previously mixed block and the correct spatialization of first block
             // of any upcoming audio
 
-            if (!streamToAdd.isStereo()) {
+            if (!streamToAdd.isStereo() && !isEcho) {
+                // get the existing listener-source HRTF object, or create a new one
+                auto& hrtf = listenerNodeData.hrtfForStream(sourceNodeID, streamToAdd.getStreamIdentifier());
+
                 // this is not done for stereo streams since they do not go through the HRTF
                 static int16_t silentMonoBlock[AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL] = {};
                 hrtf.renderSilent(silentMonoBlock, _mixedSamples, HRTF_DATASET_INDEX, azimuth, gain,
@@ -251,13 +247,10 @@ void AudioMixer::addStreamToMixForListeningNodeWithStream(ListenerSourceIDPair i
     // grab the stream from the ring buffer
     AudioRingBuffer::ConstIterator streamPopOutput = streamToAdd.getLastPopOutput();
 
-    static int16_t streamBlock[AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL];
-
     if (streamToAdd.isStereo() || isEcho) {
         // this is a stereo source or server echo so we do not pass it through the HRTF
         // simply apply our calculated gain to each sample
         if (streamToAdd.isStereo()) {
-
             for (int i = 0; i < AudioConstants::NETWORK_FRAME_SAMPLES_STEREO; ++i) {
                 _mixedSamples[i] += float(streamPopOutput[i] * gain / AudioConstants::MAX_SAMPLE_VALUE);
             }
@@ -272,6 +265,11 @@ void AudioMixer::addStreamToMixForListeningNodeWithStream(ListenerSourceIDPair i
         return;
     }
 
+    // get the existing listener-source HRTF object, or create a new one
+    auto& hrtf = listenerNodeData.hrtfForStream(sourceNodeID, streamToAdd.getStreamIdentifier());
+
+    static int16_t streamBlock[AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL];
+
     streamPopOutput.readSamples(streamBlock, AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL);
 
     // if the frame we're about to mix is silent, simply call render silent and move on
@@ -323,15 +321,10 @@ bool AudioMixer::prepareMixForListeningNode(Node* node) {
             for (auto& streamPair : streamsCopy) {
 
                 auto otherNodeStream = streamPair.second;
-                auto streamUUID = streamPair.first;
-
-                if (otherNodeStream->getType() == PositionalAudioStream::Microphone) {
-                    streamUUID = otherNode->getUUID();
-                }
 
                 if (*otherNode != *node || otherNodeStream->shouldLoopbackForNode()) {
-                    addStreamToMixForListeningNodeWithStream({ node->getUUID(), streamUUID },
+                    addStreamToMixForListeningNodeWithStream(*listenerNodeData, *otherNodeStream, otherNode->getUUID(),
-                                                             *listenerNodeData, *otherNodeStream, *nodeAudioStream);
+                                                             *nodeAudioStream);
                 }
             }
         }
@@ -456,28 +449,28 @@ void AudioMixer::handleMuteEnvironmentPacket(QSharedPointer<ReceivedMessage> mes
 }
 
 void AudioMixer::handleNodeKilled(SharedNodePointer killedNode) {
-    // call our helper to clear HRTF data that had this node as a source or listener
+    // enumerate the connected listeners to remove HRTF objects for the disconnected node
-    clearHRTFsMatchingStreamID(killedNode->getUUID());
+    auto nodeList = DependencyManager::get<NodeList>();
+
+    nodeList->eachNode([](const SharedNodePointer& node) {
+        auto clientData = dynamic_cast<AudioMixerClientData*>(node->getLinkedData());
+        if (clientData) {
+            clientData->removeHRTFsForNode(node->getUUID());
+        }
+    });
 }
 
-void AudioMixer::clearHRTFsMatchingStreamID(const QUuid& streamID) {
+void AudioMixer::removeHRTFsForFinishedInjector(const QUuid& streamID) {
-    qDebug() << "Removing HRTF objects for" << streamID;
+    auto injectorClientData = qobject_cast<AudioMixerClientData*>(sender());
+    if (injectorClientData) {
+        // enumerate the connected listeners to remove HRTF objects for the disconnected injector
+        auto nodeList = DependencyManager::get<NodeList>();
 
-    // enumerate the HRTF map to remove any HRTFs that included this stream as a source or listener
+        nodeList->eachNode([injectorClientData, &streamID](const SharedNodePointer& node){
-    auto it = _hrtfMap.begin();
+            auto listenerClientData = dynamic_cast<AudioMixerClientData*>(node->getLinkedData());
-    while (it != _hrtfMap.end()) {
+            listenerClientData->removeHRTFForStream(injectorClientData->getNodeID(), streamID);
-        auto& idPair = it->first;
+        });
-
-        if (idPair.first == streamID || idPair.second == streamID) {
-            // matched the stream ID to source or listener, remove that HRTF from the map
-            it = _hrtfMap.erase(it);
-        } else {
-            // did not match, push the iterator
-            ++it;
-        }
     }
-
-    qDebug() << "HRTF now has" << _hrtfMap.size();
 }
 
 void AudioMixer::sendStatsPacket() {
@@ -579,10 +572,10 @@ void AudioMixer::domainSettingsRequestComplete() {
     nodeList->addNodeTypeToInterestSet(NodeType::Agent);
 
     nodeList->linkedDataCreateCallback = [&](Node* node) {
-        node->setLinkedData(std::unique_ptr<NodeData> { new AudioMixerClientData });
+        node->setLinkedData(std::unique_ptr<NodeData> { new AudioMixerClientData(node->getUUID()) });
         auto clientData = dynamic_cast<AudioMixerClientData*>(node->getLinkedData());
 
-        connect(clientData, &AudioMixerClientData::injectorStreamFinished, this, &AudioMixer::clearHRTFsMatchingStreamID);
+        connect(clientData, &AudioMixerClientData::injectorStreamFinished, this, &AudioMixer::removeHRTFsForFinishedInjector);
     };
 
     DomainHandler& domainHandler = nodeList->getDomainHandler();

assignment-client/src/audio/AudioMixer.h

@@ -15,7 +15,6 @@
 #include <AABox.h>
 #include <AudioHRTF.h>
 #include <AudioRingBuffer.h>
-#include <PairHash.h>
 #include <ThreadedAssignment.h>
 #include <UUIDHasher.h>
 
@@ -48,17 +47,15 @@ private slots:
     void handleMuteEnvironmentPacket(QSharedPointer<ReceivedMessage> packet, SharedNodePointer sendingNode);
     void handleNodeKilled(SharedNodePointer killedNode);
 
-    void clearHRTFsMatchingStreamID(const QUuid& streamID);
+    void removeHRTFsForFinishedInjector(const QUuid& streamID);
 
 private:
     void domainSettingsRequestComplete();
     
-    using ListenerSourceIDPair = std::pair<QUuid, QUuid>;
-    
     /// adds one stream to the mix for a listening node
-    void addStreamToMixForListeningNodeWithStream(ListenerSourceIDPair idPair,
+    void addStreamToMixForListeningNodeWithStream(AudioMixerClientData& listenerNodeData,
-                                                  const AudioMixerClientData& listenerNodeData,
                                                   const PositionalAudioStream& streamToAdd,
+                                                  const QUuid& sourceNodeID,
                                                   const AvatarAudioStream& listeningNodeStream);
 
     float gainForSource(const PositionalAudioStream& streamToAdd, const AvatarAudioStream& listeningNodeStream,
@@ -91,9 +88,6 @@ private:
     float _mixedSamples[AudioConstants::NETWORK_FRAME_SAMPLES_STEREO];
     int16_t _clampedSamples[AudioConstants::NETWORK_FRAME_SAMPLES_STEREO];
 
-    using HRTFMap = std::unordered_map<ListenerSourceIDPair, AudioHRTF, pair_hash>;
-    HRTFMap _hrtfMap;
-
     QHash<QString, AABox> _audioZones;
     struct ZonesSettings {
         QString source;

assignment-client/src/audio/AudioMixerClientData.cpp

@@ -21,7 +21,8 @@
 #include "AudioMixerClientData.h"
 
 
-AudioMixerClientData::AudioMixerClientData() :
+AudioMixerClientData::AudioMixerClientData(const QUuid& nodeID) :
+    NodeData(nodeID),
     _outgoingMixedAudioSequenceNumber(0),
     _downstreamAudioStreamStats()
 {
@@ -40,6 +41,22 @@ AvatarAudioStream* AudioMixerClientData::getAvatarAudioStream() {
     return NULL;
 }
 
+void AudioMixerClientData::removeHRTFForStream(const QUuid& nodeID, const QUuid& streamID) {
+    auto it = _nodeSourcesHRTFMap.find(nodeID);
+    if (it != _nodeSourcesHRTFMap.end()) {
+        qDebug() << "Erasing stream" << streamID << "from node" << nodeID << "for listener" << getNodeID();
+        // erase the stream with the given ID from the given node
+        it->second.erase(streamID);
+
+        // is the map for this node now empty?
+        // if so we can remove it
+        if (it->second.size() == 0) {
+            qDebug() << "Last injector was erased, erasing map for" << nodeID << "for listener" << getNodeID();
+            _nodeSourcesHRTFMap.erase(it);
+        }
+    }
+}
+
 int AudioMixerClientData::parseData(ReceivedMessage& message) {
     PacketType packetType = message.getType();
     
@@ -110,6 +127,8 @@ int AudioMixerClientData::parseData(ReceivedMessage& message) {
                     std::unique_ptr<InjectedAudioStream> { new InjectedAudioStream(streamIdentifier, isStereo, AudioMixer::getStreamSettings()) }
                 );
 
+                qDebug() << "Added an injector at" << streamIdentifier;
+
                 streamIt = emplaced.first;
             }
 
@@ -160,6 +179,8 @@ void AudioMixerClientData::removeDeadInjectedStreams() {
 
     QWriteLocker writeLocker { &_streamsLock };
 
+    qDebug() << _audioStreams.size();
+
     auto it = _audioStreams.begin();
     while (it != _audioStreams.end()) {
         PositionalAudioStream* audioStream = it->second.get();
@@ -183,9 +204,6 @@ void AudioMixerClientData::removeDeadInjectedStreams() {
 
 void AudioMixerClientData::sendAudioStreamStatsPackets(const SharedNodePointer& destinationNode) {
 
-    // since audio stream stats packets are sent periodically, this is a good place to remove our dead injected streams.
-    removeDeadInjectedStreams();
-
     auto nodeList = DependencyManager::get<NodeList>();
 
     // The append flag is a boolean value that will be packed right after the header.  The first packet sent

assignment-client/src/audio/AudioMixerClientData.h

@@ -15,6 +15,7 @@
 #include <QtCore/QJsonObject>
 
 #include <AABox.h>
+#include <AudioHRTF.h>
 #include <UUIDHasher.h>
 
 #include "PositionalAudioStream.h"
@@ -23,7 +24,7 @@
 class AudioMixerClientData : public NodeData {
     Q_OBJECT
 public:
-    AudioMixerClientData();
+    AudioMixerClientData(const QUuid& nodeID);
 
     using SharedStreamPointer = std::shared_ptr<PositionalAudioStream>;
     using AudioStreamMap = std::unordered_map<QUuid, SharedStreamPointer>;
@@ -31,8 +32,20 @@ public:
     // locks the mutex to make a copy
     AudioStreamMap getAudioStreams() { QReadLocker readLock { &_streamsLock }; return _audioStreams; }
     AvatarAudioStream* getAvatarAudioStream();
+
+    // the following methods should be called from the AudioMixer assignment thread ONLY
+    // they are not thread-safe
+
+    // returns a new or existing HRTF object for the given stream from the given node
+    AudioHRTF& hrtfForStream(const QUuid& nodeID, const QUuid& streamID = QUuid()) { return _nodeSourcesHRTFMap[nodeID][streamID]; }
+
+    // remove HRTFs for all sources from this node
+    void removeHRTFsForNode(const QUuid& nodeID) { qDebug() << "Removing all HRTF for listener" << getNodeID() << "and source" << nodeID;_nodeSourcesHRTFMap.erase(nodeID); }
+
+    // removes an AudioHRTF object for a given stream
+    void removeHRTFForStream(const QUuid& nodeID, const QUuid& streamID = QUuid());
     
-    int parseData(ReceivedMessage& message);
+    int parseData(ReceivedMessage& me ssage);
 
     void checkBuffersBeforeFrameSend();
 
@@ -57,6 +70,10 @@ private:
     QReadWriteLock _streamsLock;
     AudioStreamMap _audioStreams; // microphone stream from avatar is stored under key of null UUID
 
+    using HRTFMap = std::unordered_map<QUuid, AudioHRTF>;
+    using NodeSourcesHRTFMap = std::unordered_map<QUuid, HRTFMap>;
+    NodeSourcesHRTFMap _nodeSourcesHRTFMap;
+
     quint16 _outgoingMixedAudioSequenceNumber;
 
     AudioStreamStats _downstreamAudioStreamStats;

libraries/audio/src/InjectedAudioStream.h

@@ -23,15 +23,15 @@ public:
     float getRadius() const { return _radius; }
     float getAttenuationRatio() const { return _attenuationRatio; }
 
-    QUuid getStreamIdentifier() const { return _streamIdentifier; }
+    virtual const QUuid& getStreamIdentifier() const override { return _streamIdentifier; }
 
 private:
     // disallow copying of InjectedAudioStream objects
     InjectedAudioStream(const InjectedAudioStream&);
     InjectedAudioStream& operator= (const InjectedAudioStream&);
 
-    AudioStreamStats getAudioStreamStats() const;
+    AudioStreamStats getAudioStreamStats() const override;
-    int parseStreamProperties(PacketType type, const QByteArray& packetAfterSeqNum, int& numAudioSamples);
+    int parseStreamProperties(PacketType type, const QByteArray& packetAfterSeqNum, int& numAudioSamples) override;
 
     const QUuid _streamIdentifier;
     float _radius;

libraries/audio/src/PositionalAudioStream.h

@@ -28,7 +28,10 @@ public:
     };
 
     PositionalAudioStream(PositionalAudioStream::Type type, bool isStereo, const InboundAudioStream::Settings& settings);
-    
+
+    const QUuid DEFAULT_STREAM_IDENTIFIER = QUuid();
+    virtual const QUuid& getStreamIdentifier() const { return DEFAULT_STREAM_IDENTIFIER; }
+
     virtual void resetStats();
 
     virtual AudioStreamStats getAudioStreamStats() const;

libraries/networking/src/NodeData.cpp

@@ -11,8 +11,9 @@
 
 #include "NodeData.h"
 
-NodeData::NodeData() :
+NodeData::NodeData(const QUuid& nodeID) :
-    _mutex()
+    _mutex(),
+    _nodeID(nodeID)
 {
     
 }

libraries/networking/src/NodeData.h

@@ -24,14 +24,17 @@ class Node;
 class NodeData : public QObject {
     Q_OBJECT
 public:
-    NodeData();
+    NodeData(const QUuid& nodeID = QUuid());
     virtual ~NodeData() = 0;
     virtual int parseData(ReceivedMessage& message) { return 0; }
+
+    const QUuid& getNodeID() const { return _nodeID; }
     
     QMutex& getMutex() { return _mutex; }
 
 private:
     QMutex _mutex;
+    QUuid _nodeID;
 };
 
 #endif // hifi_NodeData_h

libraries/shared/src/PairHash.h

@@ -1,39 +0,0 @@
-//
-//  PairHash.h
-//  libraries/shared/src
-//
-//  Created by Stephen Birarda on 2016-02-08.
-//  Copyright 2016 High Fidelity, Inc.
-//
-//  Distributed under the Apache License, Version 2.0.
-//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
-//
-
-#pragma once
-
-#ifndef hifi_PairHash_h
-#define hifi_PairHash_h
-
-// this header adds struct pair_hash in order to handle the use of an std::pair as the key of an unordered_map
-
-template <class T>
-inline void hash_combine(std::size_t& seed, const T& v) {
-    std::hash<T> hasher;
-    seed ^= hasher(v) + 0x9e3779b9 + (seed<<6) + (seed>>2);
-}
-
-// Only for pairs of std::hash-able types for simplicity.
-// You can of course template this struct to allow other hash functions
-struct pair_hash {
-    template <class T1, class T2>
-    std::size_t operator () (const std::pair<T1,T2> &p) const {
-        std::size_t seed = 0;
-
-        hash_combine(seed, p.first);
-        hash_combine(seed, p.second);
-
-        return seed;
-    }
-};
-
-#endif // hifi_PairHash_h