From 569ae113a03242ec31266ed87f79f87529b4467a Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 2 Feb 2017 16:19:21 +0000 Subject: [PATCH 01/15] memoize audio ignore box in AudioMixerClientData --- .../src/audio/AudioMixerClientData.cpp | 34 +++++++++++++++++++ .../src/audio/AudioMixerClientData.h | 15 +++++++- .../src/audio/AudioMixerSlave.cpp | 27 +++------------ 3 files changed, 52 insertions(+), 24 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index 70d6a67b5b..216958a00d 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -59,6 +59,40 @@ AvatarAudioStream* AudioMixerClientData::getAvatarAudioStream() { return NULL; } +AABox& AudioMixerClientData::getIgnoreBox(unsigned int frame) { + // check for a memoized box + if (frame != _ignoreBoxMemo.frame.load(std::memory_order_acquire) { + // create the box + AABox box(getAvatarBoundingBoxCorner(), getAvatarBoundingBoxScale()); + + // enforce a minimum scale + static const glm::vec3 MIN_IGNORE_BOX_SCALE = glm::vec3(0.3f, 1.3f, 0.3f); + if (glm::any(glm::lessThan(getAvatarBoundingBoxScale(), MIN_IGNORE_BOX_SCALE))) { + box.setScaleStayCentered(MIN_IGNORE_BOX_SCALE); + } + + // quadruple the scale + const float IGNORE_BOX_SCALE_FACTOR = 4.0f; + box.embiggen(IGNORE_BOX_SCALE_FACTOR); + + // update the memoized box + // this may be called by multiple threads concurrently, + // so take a lock and only update the memo if this call is first. + // this prevents concurrent updates from invalidating the returned reference + // (contingent on the preconditions listed in the header). + std::lock_guard lock(_ignoreBoxMemo.mutex); + if (frame != _ignoreBoxMemo.frame.load(std::memory_order_acquire)) { + _ignoreBoxMemo.box = box; + unsigned int oldFrame = _ignoreBoxMemo.frame.exchange(frame, std::memory_order_release); + + // check the precondition + assert(frame == (oldFrame + 1)); + } + } + + return _ignoreBoxMemo.box; +} + void AudioMixerClientData::removeHRTFForStream(const QUuid& nodeID, const QUuid& streamID) { auto it = _nodeSourcesHRTFMap.find(nodeID); if (it != _nodeSourcesHRTFMap.end()) { diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index e637fd0409..1a4124ebff 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -38,6 +38,12 @@ public: AudioStreamMap getAudioStreams() { QReadLocker readLock { &_streamsLock }; return _audioStreams; } AvatarAudioStream* getAvatarAudioStream(); + // returns an ignore box, memoized by frame (lockless if the box is already memoized) + // preconditions: + // - frame is monotonically increasing + // - calls are only made to getIgnoreBox(frame + 1) when there are no references left from calls to getIgnoreBox(frame) + AABox& AudioMixerClientData::getIgnoreBox(unsigned int frame); + // the following methods should be called from the AudioMixer assignment thread ONLY // they are not thread-safe @@ -86,7 +92,7 @@ public: bool shouldFlushEncoder() { return _shouldFlushEncoder; } QString getCodecName() { return _selectedCodecName; } - + bool shouldMuteClient() { return _shouldMuteClient; } void setShouldMuteClient(bool shouldMuteClient) { _shouldMuteClient = shouldMuteClient; } glm::vec3 getPosition() { return getAvatarAudioStream() ? getAvatarAudioStream()->getPosition() : glm::vec3(0); } @@ -106,6 +112,13 @@ private: QReadWriteLock _streamsLock; AudioStreamMap _audioStreams; // microphone stream from avatar is stored under key of null UUID + struct IgnoreBoxMemo { + AABox box; + std::atomic frame { 0 }; + std::mutex mutex; + }; + IgnoreBoxMemo _ignoreBoxMemo; + using HRTFMap = std::unordered_map; using NodeSourcesHRTFMap = std::unordered_map; NodeSourcesHRTFMap _nodeSourcesHRTFMap; diff --git a/assignment-client/src/audio/AudioMixerSlave.cpp b/assignment-client/src/audio/AudioMixerSlave.cpp index adc6413316..fff8ef77bb 100644 --- a/assignment-client/src/audio/AudioMixerSlave.cpp +++ b/assignment-client/src/audio/AudioMixerSlave.cpp @@ -146,7 +146,7 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { mixStream(*listenerData, node->getUUID(), *listenerAudioStream, *nodeStream); } } - } else if (!shouldIgnoreNode(listener, node)) { + } else if (!shouldIgnoreNode(listener, node, _frame)) { if (!isThrottling) { allStreams(node, &AudioMixerSlave::mixStream); } else { @@ -452,7 +452,7 @@ void sendEnvironmentPacket(const SharedNodePointer& node, AudioMixerClientData& } } -bool shouldIgnoreNode(const SharedNodePointer& listener, const SharedNodePointer& node) { +bool shouldIgnoreNode(const SharedNodePointer& listener, const SharedNodePointer& node, unsigned int frame) { AudioMixerClientData* listenerData = static_cast(listener->getLinkedData()); AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); @@ -469,27 +469,8 @@ bool shouldIgnoreNode(const SharedNodePointer& listener, const SharedNodePointer // is either node enabling the space bubble / ignore radius? if ((listener->isIgnoreRadiusEnabled() || node->isIgnoreRadiusEnabled())) { - // define the minimum bubble size - static const glm::vec3 minBubbleSize = glm::vec3(0.3f, 1.3f, 0.3f); - - // set up the bounding box for the listener - AABox listenerBox(listenerData->getAvatarBoundingBoxCorner(), listenerData->getAvatarBoundingBoxScale()); - if (glm::any(glm::lessThan(listenerData->getAvatarBoundingBoxScale(), minBubbleSize))) { - listenerBox.setScaleStayCentered(minBubbleSize); - } - - // set up the bounding box for the node - AABox nodeBox(nodeData->getAvatarBoundingBoxCorner(), nodeData->getAvatarBoundingBoxScale()); - // Clamp the size of the bounding box to a minimum scale - if (glm::any(glm::lessThan(nodeData->getAvatarBoundingBoxScale(), minBubbleSize))) { - nodeBox.setScaleStayCentered(minBubbleSize); - } - - // quadruple the scale of both bounding boxes - listenerBox.embiggen(4.0f); - nodeBox.embiggen(4.0f); - - // perform the collision check between the two bounding boxes + AABox& listenerBox = listenerData->getIgnoreBox(frame); + AABox& nodeBox = nodeData->getIgnoreBox(frame); ignore = listenerBox.touches(nodeBox); } else { ignore = false; From 9bcc5c95b47871d014dcf6e594f040609730427a Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Thu, 2 Feb 2017 16:24:12 +0000 Subject: [PATCH 02/15] reduce stream lock usage in AudioMixerClientData::getIgnoreBox --- .../src/audio/AudioMixerClientData.cpp | 15 +++++++++------ .../src/audio/AudioMixerClientData.h | 2 -- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index 216958a00d..0811021cae 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -62,18 +62,21 @@ AvatarAudioStream* AudioMixerClientData::getAvatarAudioStream() { AABox& AudioMixerClientData::getIgnoreBox(unsigned int frame) { // check for a memoized box if (frame != _ignoreBoxMemo.frame.load(std::memory_order_acquire) { - // create the box - AABox box(getAvatarBoundingBoxCorner(), getAvatarBoundingBoxScale()); + stream = getAvatarAudioStream(); + glm::vec3 corner = stream ? stream->getAvatarBoundingBoxCorner() : glm::vec3(0); + glm::vec3 scale = stream ? stream->getAvatarBoundingBoxScale() : glm::vec3(0); // enforce a minimum scale static const glm::vec3 MIN_IGNORE_BOX_SCALE = glm::vec3(0.3f, 1.3f, 0.3f); - if (glm::any(glm::lessThan(getAvatarBoundingBoxScale(), MIN_IGNORE_BOX_SCALE))) { - box.setScaleStayCentered(MIN_IGNORE_BOX_SCALE); + if (glm::any(glm::lessThan(scale, MIN_IGNORE_BOX_SCALE))) { + scale = MIN_IGNORE_BOX_SCALE; } - // quadruple the scale const float IGNORE_BOX_SCALE_FACTOR = 4.0f; - box.embiggen(IGNORE_BOX_SCALE_FACTOR); + scale *= IGNORE_BOX_SCALE_FACTOR; + + // create the box + AABox box(corner, scale); // update the memoized box // this may be called by multiple threads concurrently, diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 1a4124ebff..8bc69897bf 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -96,8 +96,6 @@ public: bool shouldMuteClient() { return _shouldMuteClient; } void setShouldMuteClient(bool shouldMuteClient) { _shouldMuteClient = shouldMuteClient; } glm::vec3 getPosition() { return getAvatarAudioStream() ? getAvatarAudioStream()->getPosition() : glm::vec3(0); } - glm::vec3 getAvatarBoundingBoxCorner() { return getAvatarAudioStream() ? getAvatarAudioStream()->getAvatarBoundingBoxCorner() : glm::vec3(0); } - glm::vec3 getAvatarBoundingBoxScale() { return getAvatarAudioStream() ? getAvatarAudioStream()->getAvatarBoundingBoxScale() : glm::vec3(0); } bool getRequestsDomainListData() { return _requestsDomainListData; } void setRequestsDomainListData(bool requesting) { _requestsDomainListData = requesting; } From 3c1cf504d099aeabdf6b681a291af37bbc314034 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 3 Feb 2017 16:41:55 +0000 Subject: [PATCH 03/15] rename getIgnoreBox to getIgnoreZone to prevent confusion --- .../src/audio/AudioMixerClientData.cpp | 25 +++++++++++-------- .../src/audio/AudioMixerClientData.h | 13 +++++----- .../src/audio/AudioMixerSlave.cpp | 6 ++--- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index 0811021cae..9a168e1c51 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -59,10 +59,12 @@ AvatarAudioStream* AudioMixerClientData::getAvatarAudioStream() { return NULL; } -AABox& AudioMixerClientData::getIgnoreBox(unsigned int frame) { - // check for a memoized box - if (frame != _ignoreBoxMemo.frame.load(std::memory_order_acquire) { +IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame) { + // check for a memoized zone + if (frame != _ignoreZoneMemo.frame.load(std::memory_order_acquire) { stream = getAvatarAudioStream(); + + // get the initial dimensions from the stream glm::vec3 corner = stream ? stream->getAvatarBoundingBoxCorner() : glm::vec3(0); glm::vec3 scale = stream ? stream->getAvatarBoundingBoxScale() : glm::vec3(0); @@ -71,29 +73,30 @@ AABox& AudioMixerClientData::getIgnoreBox(unsigned int frame) { if (glm::any(glm::lessThan(scale, MIN_IGNORE_BOX_SCALE))) { scale = MIN_IGNORE_BOX_SCALE; } - // quadruple the scale + + // quadruple the scale (this is arbitrary number chosen for comfort) const float IGNORE_BOX_SCALE_FACTOR = 4.0f; scale *= IGNORE_BOX_SCALE_FACTOR; - // create the box + // create the box (we use a box for the zone for convenience) AABox box(corner, scale); - // update the memoized box + // update the memoized zone // this may be called by multiple threads concurrently, // so take a lock and only update the memo if this call is first. // this prevents concurrent updates from invalidating the returned reference // (contingent on the preconditions listed in the header). - std::lock_guard lock(_ignoreBoxMemo.mutex); - if (frame != _ignoreBoxMemo.frame.load(std::memory_order_acquire)) { - _ignoreBoxMemo.box = box; - unsigned int oldFrame = _ignoreBoxMemo.frame.exchange(frame, std::memory_order_release); + std::lock_guard lock(_ignoreZoneMemo.mutex); + if (frame != _ignoreZoneMemo.frame.load(std::memory_order_acquire)) { + _ignoreZoneMemo.zone = box; + unsigned int oldFrame = _ignoreZoneMemo.frame.exchange(frame, std::memory_order_release); // check the precondition assert(frame == (oldFrame + 1)); } } - return _ignoreBoxMemo.box; + return _ignoreZoneMemo.zone; } void AudioMixerClientData::removeHRTFForStream(const QUuid& nodeID, const QUuid& streamID) { diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 8bc69897bf..0215ca8996 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -33,16 +33,17 @@ public: using SharedStreamPointer = std::shared_ptr; using AudioStreamMap = std::unordered_map; + using IgnoreZone = AABox; // locks the mutex to make a copy AudioStreamMap getAudioStreams() { QReadLocker readLock { &_streamsLock }; return _audioStreams; } AvatarAudioStream* getAvatarAudioStream(); - // returns an ignore box, memoized by frame (lockless if the box is already memoized) + // returns an ignore zone, memoized by frame (lockless if the zone is already memoized) // preconditions: // - frame is monotonically increasing - // - calls are only made to getIgnoreBox(frame + 1) when there are no references left from calls to getIgnoreBox(frame) - AABox& AudioMixerClientData::getIgnoreBox(unsigned int frame); + // - calls are only made to getIgnoreZone(frame + 1) when there are no references left from calls to getIgnoreZone(frame) + IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame); // the following methods should be called from the AudioMixer assignment thread ONLY // they are not thread-safe @@ -110,12 +111,12 @@ private: QReadWriteLock _streamsLock; AudioStreamMap _audioStreams; // microphone stream from avatar is stored under key of null UUID - struct IgnoreBoxMemo { - AABox box; + struct IgnoreZoneMemo { + IgnoreZone zone; std::atomic frame { 0 }; std::mutex mutex; }; - IgnoreBoxMemo _ignoreBoxMemo; + IgnoreZoneMemo _ignoreZoneMemo; using HRTFMap = std::unordered_map; using NodeSourcesHRTFMap = std::unordered_map; diff --git a/assignment-client/src/audio/AudioMixerSlave.cpp b/assignment-client/src/audio/AudioMixerSlave.cpp index fff8ef77bb..797194b7df 100644 --- a/assignment-client/src/audio/AudioMixerSlave.cpp +++ b/assignment-client/src/audio/AudioMixerSlave.cpp @@ -469,9 +469,9 @@ bool shouldIgnoreNode(const SharedNodePointer& listener, const SharedNodePointer // is either node enabling the space bubble / ignore radius? if ((listener->isIgnoreRadiusEnabled() || node->isIgnoreRadiusEnabled())) { - AABox& listenerBox = listenerData->getIgnoreBox(frame); - AABox& nodeBox = nodeData->getIgnoreBox(frame); - ignore = listenerBox.touches(nodeBox); + auto& listenerZone = listenerData->getIgnoreZone(frame); + auto& nodeZone = nodeData->getIgnoreZone(frame); + ignore = listenerBox.touches(nodeZone); } else { ignore = false; } From 8a42755e8f5630d30ec6a873d699d82686bd31cb Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 3 Feb 2017 17:52:02 +0000 Subject: [PATCH 04/15] mv shouldIgnore from AudioMixerSlave to ClientData --- .../src/audio/AudioMixerClientData.cpp | 26 ++++++++++++++ .../src/audio/AudioMixerClientData.h | 17 ++++++---- .../src/audio/AudioMixerSlave.cpp | 34 ++----------------- 3 files changed, 39 insertions(+), 38 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index 9a168e1c51..3128e27e17 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -59,6 +59,32 @@ AvatarAudioStream* AudioMixerClientData::getAvatarAudioStream() { return NULL; } +bool shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, unsigned int frame) { + AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); + if (!nodeData) { + return false; + } + + bool ignore = true; + if ( // the nodes are not ignoring each other explicitly (or are but get data regardless) + (!self->isIgnoringNodeWithID(node->getUUID()) || + (nodeData->getRequestsDomainListData() && node->getCanKick())) && + (!node->isIgnoringNodeWithID(self->getUUID()) || + (getsRequestsDomainListData() && self->getCanKick()))) { + + // if either node is enabling an ignore radius, check their proximity + if ((listener->isIgnoreRadiusEnabled() || node->isIgnoreRadiusEnabled())) { + auto& listenerZone = listenerData->getIgnoreZone(frame); + auto& nodeZone = nodeData->getIgnoreZone(frame); + ignore = listenerBox.touches(nodeZone); + } else { + ignore = false; + } + } + + return ignore; +} + IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame) { // check for a memoized zone if (frame != _ignoreZoneMemo.frame.load(std::memory_order_acquire) { diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 0215ca8996..72991a9d3c 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -27,23 +27,22 @@ class AudioMixerClientData : public NodeData { Q_OBJECT + using IgnoreZone = AABox; + public: AudioMixerClientData(const QUuid& nodeID); ~AudioMixerClientData(); using SharedStreamPointer = std::shared_ptr; using AudioStreamMap = std::unordered_map; - using IgnoreZone = AABox; // locks the mutex to make a copy AudioStreamMap getAudioStreams() { QReadLocker readLock { &_streamsLock }; return _audioStreams; } AvatarAudioStream* getAvatarAudioStream(); - // returns an ignore zone, memoized by frame (lockless if the zone is already memoized) - // preconditions: - // - frame is monotonically increasing - // - calls are only made to getIgnoreZone(frame + 1) when there are no references left from calls to getIgnoreZone(frame) - IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame); + // returns whether self (this data's node) should ignore node, memoized by frame + // preconditions: frame is monotonically increasing + bool shouldIgnore(SharedNodePointer self, SharedNodePointer node, unsigned int frame); // the following methods should be called from the AudioMixer assignment thread ONLY // they are not thread-safe @@ -108,6 +107,12 @@ public slots: void sendSelectAudioFormat(SharedNodePointer node, const QString& selectedCodecName); private: + // returns an ignore zone, memoized by frame (lockless if the zone is already memoized) + // preconditions: + // - frame is monotonically increasing + // - calls are only made to getIgnoreZone(frame + 1) when there are no references left from calls to getIgnoreZone(frame) + IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame); + QReadWriteLock _streamsLock; AudioStreamMap _audioStreams; // microphone stream from avatar is stored under key of null UUID diff --git a/assignment-client/src/audio/AudioMixerSlave.cpp b/assignment-client/src/audio/AudioMixerSlave.cpp index 797194b7df..cd039b3722 100644 --- a/assignment-client/src/audio/AudioMixerSlave.cpp +++ b/assignment-client/src/audio/AudioMixerSlave.cpp @@ -46,7 +46,6 @@ void sendMutePacket(const SharedNodePointer& node, AudioMixerClientData&); void sendEnvironmentPacket(const SharedNodePointer& node, AudioMixerClientData& data); // mix helpers -inline bool shouldIgnoreNode(const SharedNodePointer& listener, const SharedNodePointer& node); inline float approximateGain(const AvatarAudioStream& listeningNodeStream, const PositionalAudioStream& streamToAdd, const glm::vec3& relativePosition); inline float computeGain(const AvatarAudioStream& listeningNodeStream, const PositionalAudioStream& streamToAdd, @@ -146,7 +145,7 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { mixStream(*listenerData, node->getUUID(), *listenerAudioStream, *nodeStream); } } - } else if (!shouldIgnoreNode(listener, node, _frame)) { + } else if (!listenerData->shouldIgnoreNode(listener, node, _frame)) { if (!isThrottling) { allStreams(node, &AudioMixerSlave::mixStream); } else { @@ -452,36 +451,6 @@ void sendEnvironmentPacket(const SharedNodePointer& node, AudioMixerClientData& } } -bool shouldIgnoreNode(const SharedNodePointer& listener, const SharedNodePointer& node, unsigned int frame) { - AudioMixerClientData* listenerData = static_cast(listener->getLinkedData()); - AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); - - // when this is true, the AudioMixer will send Audio data to a client about avatars that have ignored them - bool getsAnyIgnored = listenerData->getRequestsDomainListData() && listener->getCanKick(); - - bool ignore = true; - - if (nodeData && - // make sure that it isn't being ignored by our listening node - (!listener->isIgnoringNodeWithID(node->getUUID()) || (nodeData->getRequestsDomainListData() && node->getCanKick())) && - // and that it isn't ignoring our listening node - (!node->isIgnoringNodeWithID(listener->getUUID()) || getsAnyIgnored)) { - - // is either node enabling the space bubble / ignore radius? - if ((listener->isIgnoreRadiusEnabled() || node->isIgnoreRadiusEnabled())) { - auto& listenerZone = listenerData->getIgnoreZone(frame); - auto& nodeZone = nodeData->getIgnoreZone(frame); - ignore = listenerBox.touches(nodeZone); - } else { - ignore = false; - } - } - - return ignore; -} - -static const float ATTENUATION_START_DISTANCE = 1.0f; - float approximateGain(const AvatarAudioStream& listeningNodeStream, const PositionalAudioStream& streamToAdd, const glm::vec3& relativePosition) { float gain = 1.0f; @@ -537,6 +506,7 @@ float computeGain(const AvatarAudioStream& listeningNodeStream, const Positional } // distance attenuation + const float ATTENUATION_START_DISTANCE = 1.0f; float distance = glm::length(relativePosition); assert(ATTENUATION_START_DISTANCE > EPSILON); if (distance >= ATTENUATION_START_DISTANCE) { From 310c8b18eee7fd8a729953579e08da84a1f93019 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Fri, 3 Feb 2017 18:25:06 +0000 Subject: [PATCH 05/15] add caching over symmetric nodes for audio shouldIgnore --- assignment-client/src/audio/AudioMixer.cpp | 3 +-- .../src/audio/AudioMixerClientData.cpp | 26 ++++++++++++++++--- .../src/audio/AudioMixerClientData.h | 13 +++++++--- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 4f123a6a8f..42c3e9812b 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -184,8 +184,7 @@ void AudioMixer::handleNodeKilled(SharedNodePointer killedNode) { nodeList->eachNode([&killedNode](const SharedNodePointer& node) { auto clientData = dynamic_cast(node->getLinkedData()); if (clientData) { - QUuid killedUUID = killedNode->getUUID(); - clientData->removeHRTFsForNode(killedUUID); + clientData->removeNode(killedNode->getUUID()); } }); } diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index 3128e27e17..b55d28123f 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -60,12 +60,23 @@ AvatarAudioStream* AudioMixerClientData::getAvatarAudioStream() { } bool shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, unsigned int frame) { + // this is symmetric over self / node; they cache their computations to reduce work by 2x + + auto& localCache = _nodeSourcesIgnoreMap[node->getUUID()]; + if (localCache.isCached) { + assert(localCache.isCached.is_lock_free()); + bool shouldIgnore = localCache.shouldIgnore; + localCache.isCached = false; + return shouldIgnore; + } + AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); if (!nodeData) { return false; } - bool ignore = true; + // compute shouldIgnore + bool shouldIgnore = true; if ( // the nodes are not ignoring each other explicitly (or are but get data regardless) (!self->isIgnoringNodeWithID(node->getUUID()) || (nodeData->getRequestsDomainListData() && node->getCanKick())) && @@ -76,13 +87,20 @@ bool shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, un if ((listener->isIgnoreRadiusEnabled() || node->isIgnoreRadiusEnabled())) { auto& listenerZone = listenerData->getIgnoreZone(frame); auto& nodeZone = nodeData->getIgnoreZone(frame); - ignore = listenerBox.touches(nodeZone); + shouldIgnore = listenerBox.touches(nodeZone); } else { - ignore = false; + shouldIgnore = false; } } - return ignore; + remoteCache = nodeData._nodeSourcesIgnoreMap[self->getUUID()]; + // do not reset the cache until it has been used to avoid a data race + if (!remoteCache.isCached) { + cache.shouldIgnore = shouldIgnore; + remoteCache.isCached = true; + } + + return shouldIgnore; } IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame) { diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 72991a9d3c..a23bc32498 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -50,12 +50,12 @@ public: // 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) { _nodeSourcesHRTFMap.erase(nodeID); } - // removes an AudioHRTF object for a given stream void removeHRTFForStream(const QUuid& nodeID, const QUuid& streamID = QUuid()); + // remove all sources and data from this node + void removeNode(const QUuid& nodeID) { _nodeSourcesIgnoreMap.erase(nodeID); _nodeSourcesHRTFMap.erase(nodeID); } + void removeAgentAvatarAudioStream(); int parseData(ReceivedMessage& message) override; @@ -123,6 +123,13 @@ private: }; IgnoreZoneMemo _ignoreZoneMemo; + struct IgnoreNodeData { + std::atomic flag { false }; + bool ignore { false }; + }; + using NodeSourcesIgnoreMap = std::unordered_map; + NodeSourcesIgnoreMap _nodeSourcesIgnoreMap; + using HRTFMap = std::unordered_map; using NodeSourcesHRTFMap = std::unordered_map; NodeSourcesHRTFMap _nodeSourcesHRTFMap; From 207d2e78f05f45eb85f88a139bd342bb71e5bf95 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 6 Feb 2017 21:24:36 +0000 Subject: [PATCH 06/15] fix should ignore opts --- .../src/audio/AudioMixerClientData.cpp | 23 ++++++++++--------- .../src/audio/AudioMixerClientData.h | 9 ++++---- .../src/audio/AudioMixerSlave.cpp | 2 +- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index b55d28123f..c4199b5d37 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -59,7 +59,7 @@ AvatarAudioStream* AudioMixerClientData::getAvatarAudioStream() { return NULL; } -bool shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, unsigned int frame) { +bool AudioMixerClientData::shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, unsigned int frame) { // this is symmetric over self / node; they cache their computations to reduce work by 2x auto& localCache = _nodeSourcesIgnoreMap[node->getUUID()]; @@ -81,32 +81,32 @@ bool shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, un (!self->isIgnoringNodeWithID(node->getUUID()) || (nodeData->getRequestsDomainListData() && node->getCanKick())) && (!node->isIgnoringNodeWithID(self->getUUID()) || - (getsRequestsDomainListData() && self->getCanKick()))) { + (getRequestsDomainListData() && self->getCanKick()))) { // if either node is enabling an ignore radius, check their proximity - if ((listener->isIgnoreRadiusEnabled() || node->isIgnoreRadiusEnabled())) { - auto& listenerZone = listenerData->getIgnoreZone(frame); + if ((self->isIgnoreRadiusEnabled() || node->isIgnoreRadiusEnabled())) { + auto& zone = getIgnoreZone(frame); auto& nodeZone = nodeData->getIgnoreZone(frame); - shouldIgnore = listenerBox.touches(nodeZone); + shouldIgnore = zone.touches(nodeZone); } else { shouldIgnore = false; } } - remoteCache = nodeData._nodeSourcesIgnoreMap[self->getUUID()]; + auto& remoteCache = nodeData->_nodeSourcesIgnoreMap[self->getUUID()]; // do not reset the cache until it has been used to avoid a data race if (!remoteCache.isCached) { - cache.shouldIgnore = shouldIgnore; + remoteCache.shouldIgnore = shouldIgnore; remoteCache.isCached = true; } return shouldIgnore; } -IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame) { +AudioMixerClientData::IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame) { // check for a memoized zone - if (frame != _ignoreZoneMemo.frame.load(std::memory_order_acquire) { - stream = getAvatarAudioStream(); + if (frame != _ignoreZoneMemo.frame.load(std::memory_order_acquire)) { + auto stream = getAvatarAudioStream(); // get the initial dimensions from the stream glm::vec3 corner = stream ? stream->getAvatarBoundingBoxCorner() : glm::vec3(0); @@ -130,10 +130,11 @@ IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame) { // so take a lock and only update the memo if this call is first. // this prevents concurrent updates from invalidating the returned reference // (contingent on the preconditions listed in the header). - std::lock_guard lock(_ignoreZoneMemo.mutex); + std::lock_guard lock(_ignoreZoneMemo.mutex); if (frame != _ignoreZoneMemo.frame.load(std::memory_order_acquire)) { _ignoreZoneMemo.zone = box; unsigned int oldFrame = _ignoreZoneMemo.frame.exchange(frame, std::memory_order_release); + Q_UNUSED(oldFrame); // check the precondition assert(frame == (oldFrame + 1)); diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index a23bc32498..75cfa7ff18 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -27,7 +27,6 @@ class AudioMixerClientData : public NodeData { Q_OBJECT - using IgnoreZone = AABox; public: AudioMixerClientData(const QUuid& nodeID); @@ -107,11 +106,13 @@ public slots: void sendSelectAudioFormat(SharedNodePointer node, const QString& selectedCodecName); private: + using IgnoreZone = AABox; + // returns an ignore zone, memoized by frame (lockless if the zone is already memoized) // preconditions: // - frame is monotonically increasing // - calls are only made to getIgnoreZone(frame + 1) when there are no references left from calls to getIgnoreZone(frame) - IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame); + IgnoreZone& getIgnoreZone(unsigned int frame); QReadWriteLock _streamsLock; AudioStreamMap _audioStreams; // microphone stream from avatar is stored under key of null UUID @@ -124,8 +125,8 @@ private: IgnoreZoneMemo _ignoreZoneMemo; struct IgnoreNodeData { - std::atomic flag { false }; - bool ignore { false }; + std::atomic isCached { false }; + bool shouldIgnore { false }; }; using NodeSourcesIgnoreMap = std::unordered_map; NodeSourcesIgnoreMap _nodeSourcesIgnoreMap; diff --git a/assignment-client/src/audio/AudioMixerSlave.cpp b/assignment-client/src/audio/AudioMixerSlave.cpp index cd039b3722..341f605b2d 100644 --- a/assignment-client/src/audio/AudioMixerSlave.cpp +++ b/assignment-client/src/audio/AudioMixerSlave.cpp @@ -145,7 +145,7 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { mixStream(*listenerData, node->getUUID(), *listenerAudioStream, *nodeStream); } } - } else if (!listenerData->shouldIgnoreNode(listener, node, _frame)) { + } else if (!listenerData->shouldIgnore(listener, node, _frame)) { if (!isThrottling) { allStreams(node, &AudioMixerSlave::mixStream); } else { From e7cf84324bcb8b08131e517f7b979d3ded99b98a Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 6 Feb 2017 22:38:34 +0000 Subject: [PATCH 07/15] abstract audio ignore caching --- .../src/audio/AudioMixerClientData.cpp | 42 ++++++++++++------- .../src/audio/AudioMixerClientData.h | 11 +++-- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index c4199b5d37..a5df130c04 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -59,15 +59,33 @@ AvatarAudioStream* AudioMixerClientData::getAvatarAudioStream() { return NULL; } -bool AudioMixerClientData::shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, unsigned int frame) { - // this is symmetric over self / node; they cache their computations to reduce work by 2x +void AudioMixerClientData::IgnoreNodeData::cache(bool shouldIgnore) { + // do not reset the cache until it has been used, to avoid a data race + if (!_flag) { + _ignore = shouldIgnore; + _flag = true; + } +} - auto& localCache = _nodeSourcesIgnoreMap[node->getUUID()]; - if (localCache.isCached) { - assert(localCache.isCached.is_lock_free()); - bool shouldIgnore = localCache.shouldIgnore; - localCache.isCached = false; - return shouldIgnore; +bool AudioMixerClientData::IgnoreNodeData::isCached() { + assert(_flag.is_lock_free()); + return _flag; +} + +bool AudioMixerClientData::IgnoreNodeData::shouldIgnore() { + // do not reset the cache until it has been used, to avoid a data race + bool ignore = _ignore; + _flag = false; + return ignore; +} + +bool AudioMixerClientData::shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, unsigned int frame) { + // this is symmetric over self / node; if computed, it is cached in the other + + // check the cache to avoid computation + auto& cache = _nodeSourcesIgnoreMap[node->getUUID()]; + if (cache.isCached()) { + return cache.shouldIgnore(); } AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); @@ -93,12 +111,8 @@ bool AudioMixerClientData::shouldIgnore(const SharedNodePointer self, const Shar } } - auto& remoteCache = nodeData->_nodeSourcesIgnoreMap[self->getUUID()]; - // do not reset the cache until it has been used to avoid a data race - if (!remoteCache.isCached) { - remoteCache.shouldIgnore = shouldIgnore; - remoteCache.isCached = true; - } + // cache in node + nodeData->_nodeSourcesIgnoreMap[self->getUUID()].cache(shouldIgnore); return shouldIgnore; } diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 75cfa7ff18..1cfc476b43 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -124,9 +124,14 @@ private: }; IgnoreZoneMemo _ignoreZoneMemo; - struct IgnoreNodeData { - std::atomic isCached { false }; - bool shouldIgnore { false }; + class IgnoreNodeData { + public: + void cache(bool shouldIgnore); + bool isCached(); + bool shouldIgnore(); + private: + std::atomic _flag { false }; + bool _ignore { false }; }; using NodeSourcesIgnoreMap = std::unordered_map; NodeSourcesIgnoreMap _nodeSourcesIgnoreMap; From 093f748d7d4146517d43eb33731833744d1c60d2 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 6 Feb 2017 23:12:55 +0000 Subject: [PATCH 08/15] make auto stream explicit --- assignment-client/src/audio/AudioMixerClientData.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index a5df130c04..2b4043dee4 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -120,7 +120,7 @@ bool AudioMixerClientData::shouldIgnore(const SharedNodePointer self, const Shar AudioMixerClientData::IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame) { // check for a memoized zone if (frame != _ignoreZoneMemo.frame.load(std::memory_order_acquire)) { - auto stream = getAvatarAudioStream(); + AvatarAudioStream* stream = getAvatarAudioStream(); // get the initial dimensions from the stream glm::vec3 corner = stream ? stream->getAvatarBoundingBoxCorner() : glm::vec3(0); From 4bcc9d307237c4e32375df0617c81a8b1bc7615d Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 6 Feb 2017 23:14:14 +0000 Subject: [PATCH 09/15] bail audio mix if node is not initialized --- .../src/audio/AudioMixerSlave.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerSlave.cpp b/assignment-client/src/audio/AudioMixerSlave.cpp index 341f605b2d..d41fe598cc 100644 --- a/assignment-client/src/audio/AudioMixerSlave.cpp +++ b/assignment-client/src/audio/AudioMixerSlave.cpp @@ -125,8 +125,7 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { typedef void (AudioMixerSlave::*MixFunctor)( AudioMixerClientData&, const QUuid&, const AvatarAudioStream&, const PositionalAudioStream&); - auto allStreams = [&](const SharedNodePointer& node, MixFunctor mixFunctor) { - AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); + auto forAllStreams = [&](const SharedNodePointer& node, AudioMixerClientData* nodeData, MixFunctor mixFunctor) { auto nodeID = node->getUUID(); for (auto& streamPair : nodeData->getAudioStreams()) { auto nodeStream = streamPair.second; @@ -135,9 +134,12 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { }; std::for_each(_begin, _end, [&](const SharedNodePointer& node) { - if (*node == *listener) { - AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); + AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); + if (!nodeData) { + return; + } + if (*node == *listener) { // only mix the echo, if requested for (auto& streamPair : nodeData->getAudioStreams()) { auto nodeStream = streamPair.second; @@ -147,13 +149,12 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { } } else if (!listenerData->shouldIgnore(listener, node, _frame)) { if (!isThrottling) { - allStreams(node, &AudioMixerSlave::mixStream); + forAllStreams(node, nodeData, &AudioMixerSlave::mixStream); } else { #ifdef HIFI_AUDIO_THROTTLE_DEBUG auto throttleStart = p_high_resolution_clock::now(); #endif - AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); auto nodeID = node->getUUID(); // compute the node's max relative volume @@ -200,7 +201,8 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { std::pop_heap(throttledNodes.begin(), throttledNodes.end()); auto& node = throttledNodes.back().second; - allStreams(node, &AudioMixerSlave::mixStream); + AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); + forAllStreams(node, nodeData, &AudioMixerSlave::mixStream); throttledNodes.pop_back(); } @@ -208,7 +210,8 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { // throttle the remaining nodes' streams for (const std::pair& nodePair : throttledNodes) { auto& node = nodePair.second; - allStreams(node, &AudioMixerSlave::throttleStream); + AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); + forAllStreams(node, nodeData, &AudioMixerSlave::throttleStream); } } From d42b6a64c447ed580794f18fe167dc0d7b344a09 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 6 Feb 2017 23:43:15 +0000 Subject: [PATCH 10/15] use threadsafe map for audio node ignore cache --- assignment-client/src/audio/AudioMixerClientData.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 1cfc476b43..8882acf2f0 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -53,7 +53,7 @@ public: void removeHRTFForStream(const QUuid& nodeID, const QUuid& streamID = QUuid()); // remove all sources and data from this node - void removeNode(const QUuid& nodeID) { _nodeSourcesIgnoreMap.erase(nodeID); _nodeSourcesHRTFMap.erase(nodeID); } + void removeNode(const QUuid& nodeID) { _nodeSourcesIgnoreMap.unsafe_erase(nodeID); _nodeSourcesHRTFMap.erase(nodeID); } void removeAgentAvatarAudioStream(); @@ -126,6 +126,10 @@ private: class IgnoreNodeData { public: + // always begin unset + IgnoreNodeData() {} + IgnoreNodeData(const IgnoreNodeData& other) {} + void cache(bool shouldIgnore); bool isCached(); bool shouldIgnore(); @@ -133,7 +137,9 @@ private: std::atomic _flag { false }; bool _ignore { false }; }; - using NodeSourcesIgnoreMap = std::unordered_map; + struct IgnoreNodeDataHasher { std::size_t operator()(const QUuid& key) const { return qHash(key); } }; + + using NodeSourcesIgnoreMap = tbb::concurrent_unordered_map; NodeSourcesIgnoreMap _nodeSourcesIgnoreMap; using HRTFMap = std::unordered_map; From 308e3cab71350590a33c0dba810eeb670ae8957c Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 7 Feb 2017 17:23:11 +0000 Subject: [PATCH 11/15] fix memoized ignoreZone assertion for initialization --- assignment-client/src/audio/AudioMixerClientData.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index 2b4043dee4..e0a49b2b1b 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -151,7 +151,7 @@ AudioMixerClientData::IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned i Q_UNUSED(oldFrame); // check the precondition - assert(frame == (oldFrame + 1)); + assert(oldFrame == 0 || frame == (oldFrame + 1)); } } From ad7c01e86eaedfe57e5c811e105882b5508fb428 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 7 Feb 2017 18:03:17 +0000 Subject: [PATCH 12/15] modularize audio ignore zone computations --- .../src/audio/AudioMixerClientData.cpp | 199 +++++++++--------- .../src/audio/AudioMixerClientData.h | 55 ++--- 2 files changed, 130 insertions(+), 124 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index e0a49b2b1b..ac5316db37 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -26,6 +26,7 @@ AudioMixerClientData::AudioMixerClientData(const QUuid& nodeID) : NodeData(nodeID), audioLimiter(AudioConstants::SAMPLE_RATE, AudioConstants::STEREO), + _ignoreZone(*this), _outgoingMixedAudioSequenceNumber(0), _downstreamAudioStreamStats() { @@ -59,105 +60,6 @@ AvatarAudioStream* AudioMixerClientData::getAvatarAudioStream() { return NULL; } -void AudioMixerClientData::IgnoreNodeData::cache(bool shouldIgnore) { - // do not reset the cache until it has been used, to avoid a data race - if (!_flag) { - _ignore = shouldIgnore; - _flag = true; - } -} - -bool AudioMixerClientData::IgnoreNodeData::isCached() { - assert(_flag.is_lock_free()); - return _flag; -} - -bool AudioMixerClientData::IgnoreNodeData::shouldIgnore() { - // do not reset the cache until it has been used, to avoid a data race - bool ignore = _ignore; - _flag = false; - return ignore; -} - -bool AudioMixerClientData::shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, unsigned int frame) { - // this is symmetric over self / node; if computed, it is cached in the other - - // check the cache to avoid computation - auto& cache = _nodeSourcesIgnoreMap[node->getUUID()]; - if (cache.isCached()) { - return cache.shouldIgnore(); - } - - AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); - if (!nodeData) { - return false; - } - - // compute shouldIgnore - bool shouldIgnore = true; - if ( // the nodes are not ignoring each other explicitly (or are but get data regardless) - (!self->isIgnoringNodeWithID(node->getUUID()) || - (nodeData->getRequestsDomainListData() && node->getCanKick())) && - (!node->isIgnoringNodeWithID(self->getUUID()) || - (getRequestsDomainListData() && self->getCanKick()))) { - - // if either node is enabling an ignore radius, check their proximity - if ((self->isIgnoreRadiusEnabled() || node->isIgnoreRadiusEnabled())) { - auto& zone = getIgnoreZone(frame); - auto& nodeZone = nodeData->getIgnoreZone(frame); - shouldIgnore = zone.touches(nodeZone); - } else { - shouldIgnore = false; - } - } - - // cache in node - nodeData->_nodeSourcesIgnoreMap[self->getUUID()].cache(shouldIgnore); - - return shouldIgnore; -} - -AudioMixerClientData::IgnoreZone& AudioMixerClientData::getIgnoreZone(unsigned int frame) { - // check for a memoized zone - if (frame != _ignoreZoneMemo.frame.load(std::memory_order_acquire)) { - AvatarAudioStream* stream = getAvatarAudioStream(); - - // get the initial dimensions from the stream - glm::vec3 corner = stream ? stream->getAvatarBoundingBoxCorner() : glm::vec3(0); - glm::vec3 scale = stream ? stream->getAvatarBoundingBoxScale() : glm::vec3(0); - - // enforce a minimum scale - static const glm::vec3 MIN_IGNORE_BOX_SCALE = glm::vec3(0.3f, 1.3f, 0.3f); - if (glm::any(glm::lessThan(scale, MIN_IGNORE_BOX_SCALE))) { - scale = MIN_IGNORE_BOX_SCALE; - } - - // quadruple the scale (this is arbitrary number chosen for comfort) - const float IGNORE_BOX_SCALE_FACTOR = 4.0f; - scale *= IGNORE_BOX_SCALE_FACTOR; - - // create the box (we use a box for the zone for convenience) - AABox box(corner, scale); - - // update the memoized zone - // this may be called by multiple threads concurrently, - // so take a lock and only update the memo if this call is first. - // this prevents concurrent updates from invalidating the returned reference - // (contingent on the preconditions listed in the header). - std::lock_guard lock(_ignoreZoneMemo.mutex); - if (frame != _ignoreZoneMemo.frame.load(std::memory_order_acquire)) { - _ignoreZoneMemo.zone = box; - unsigned int oldFrame = _ignoreZoneMemo.frame.exchange(frame, std::memory_order_release); - Q_UNUSED(oldFrame); - - // check the precondition - assert(oldFrame == 0 || frame == (oldFrame + 1)); - } - } - - return _ignoreZoneMemo.zone; -} - void AudioMixerClientData::removeHRTFForStream(const QUuid& nodeID, const QUuid& streamID) { auto it = _nodeSourcesHRTFMap.find(nodeID); if (it != _nodeSourcesHRTFMap.end()) { @@ -526,3 +428,102 @@ void AudioMixerClientData::cleanupCodec() { } } } + +AudioMixerClientData::IgnoreZone& AudioMixerClientData::IgnoreZoneMemo::get(unsigned int frame) { + assert(_frame.is_lock_free()); + + // check for a memoized zone + if (frame != _frame.load(std::memory_order_acquire)) { + AvatarAudioStream* stream = _data.getAvatarAudioStream(); + + // get the initial dimensions from the stream + glm::vec3 corner = stream ? stream->getAvatarBoundingBoxCorner() : glm::vec3(0); + glm::vec3 scale = stream ? stream->getAvatarBoundingBoxScale() : glm::vec3(0); + + // enforce a minimum scale + static const glm::vec3 MIN_IGNORE_BOX_SCALE = glm::vec3(0.3f, 1.3f, 0.3f); + if (glm::any(glm::lessThan(scale, MIN_IGNORE_BOX_SCALE))) { + scale = MIN_IGNORE_BOX_SCALE; + } + + // quadruple the scale (this is arbitrary number chosen for comfort) + const float IGNORE_BOX_SCALE_FACTOR = 4.0f; + scale *= IGNORE_BOX_SCALE_FACTOR; + + // create the box (we use a box for the zone for convenience) + AABox box(corner, scale); + + // update the memoized zone + // This may be called by multiple threads concurrently, + // so take a lock and only update the memo if this call is first. + // This prevents concurrent updates from invalidating the returned reference + // (contingent on the preconditions listed in the header). + std::lock_guard lock(_mutex); + if (frame != _frame.load(std::memory_order_acquire)) { + _zone = box; + unsigned int oldFrame = _frame.exchange(frame, std::memory_order_release); + Q_UNUSED(oldFrame); + + // check the precondition + assert(oldFrame == 0 || frame == (oldFrame + 1)); + } + } + + return _zone; +} + +void AudioMixerClientData::IgnoreNodeCache::cache(bool shouldIgnore) { + if (!_isCached) { + _shouldIgnore = shouldIgnore; + _isCached = true; + } +} + +bool AudioMixerClientData::IgnoreNodeCache::isCached() { + assert(_isCached.is_lock_free()); + return _isCached; +} + +bool AudioMixerClientData::IgnoreNodeCache::shouldIgnore() { + bool ignore = _shouldIgnore; + _isCached = false; + return ignore; +} + +bool AudioMixerClientData::shouldIgnore(const SharedNodePointer self, const SharedNodePointer node, unsigned int frame) { + // this is symmetric over self / node; if computed, it is cached in the other + + // check the cache to avoid computation + auto& cache = _nodeSourcesIgnoreMap[node->getUUID()]; + if (cache.isCached()) { + return cache.shouldIgnore(); + } + + AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); + if (!nodeData) { + return false; + } + + // compute shouldIgnore + bool shouldIgnore = true; + if ( // the nodes are not ignoring each other explicitly (or are but get data regardless) + (!self->isIgnoringNodeWithID(node->getUUID()) || + (nodeData->getRequestsDomainListData() && node->getCanKick())) && + (!node->isIgnoringNodeWithID(self->getUUID()) || + (getRequestsDomainListData() && self->getCanKick()))) { + + // if either node is enabling an ignore radius, check their proximity + if ((self->isIgnoreRadiusEnabled() || node->isIgnoreRadiusEnabled())) { + auto& zone = _ignoreZone.get(frame); + auto& nodeZone = nodeData->_ignoreZone.get(frame); + shouldIgnore = zone.touches(nodeZone); + } else { + shouldIgnore = false; + } + } + + // cache in node + nodeData->_nodeSourcesIgnoreMap[self->getUUID()].cache(shouldIgnore); + + return shouldIgnore; +} diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 8882acf2f0..c30923f411 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -27,7 +27,6 @@ class AudioMixerClientData : public NodeData { Q_OBJECT - public: AudioMixerClientData(const QUuid& nodeID); ~AudioMixerClientData(); @@ -40,7 +39,7 @@ public: AvatarAudioStream* getAvatarAudioStream(); // returns whether self (this data's node) should ignore node, memoized by frame - // preconditions: frame is monotonically increasing + // precondition: frame is monotonically increasing after first call bool shouldIgnore(SharedNodePointer self, SharedNodePointer node, unsigned int frame); // the following methods should be called from the AudioMixer assignment thread ONLY @@ -108,38 +107,44 @@ public slots: private: using IgnoreZone = AABox; - // returns an ignore zone, memoized by frame (lockless if the zone is already memoized) - // preconditions: - // - frame is monotonically increasing - // - calls are only made to getIgnoreZone(frame + 1) when there are no references left from calls to getIgnoreZone(frame) - IgnoreZone& getIgnoreZone(unsigned int frame); - QReadWriteLock _streamsLock; AudioStreamMap _audioStreams; // microphone stream from avatar is stored under key of null UUID - struct IgnoreZoneMemo { - IgnoreZone zone; - std::atomic frame { 0 }; - std::mutex mutex; - }; - IgnoreZoneMemo _ignoreZoneMemo; - - class IgnoreNodeData { + class IgnoreZoneMemo { public: - // always begin unset - IgnoreNodeData() {} - IgnoreNodeData(const IgnoreNodeData& other) {} + IgnoreZoneMemo(AudioMixerClientData& data) : _data(data) {} + + // returns an ignore zone, memoized by frame (lockless if the zone is already memoized) + // preconditions: + // - frame is monotonically increasing after first call + // - there are no references left from calls to getIgnoreZone(frame - 1) + IgnoreZone& get(unsigned int frame); + + private: + AudioMixerClientData& _data; + IgnoreZone _zone; + std::atomic _frame { 0 }; + std::mutex _mutex; + }; + IgnoreZoneMemo _ignoreZone; + + class IgnoreNodeCache { + public: + // std::atomic is not copyable - always initialize uncached + IgnoreNodeCache() {} + IgnoreNodeCache(const IgnoreNodeCache& other) {} void cache(bool shouldIgnore); bool isCached(); bool shouldIgnore(); - private: - std::atomic _flag { false }; - bool _ignore { false }; - }; - struct IgnoreNodeDataHasher { std::size_t operator()(const QUuid& key) const { return qHash(key); } }; - using NodeSourcesIgnoreMap = tbb::concurrent_unordered_map; + private: + std::atomic _isCached { false }; + bool _shouldIgnore { false }; + }; + struct IgnoreNodeCacheHasher { std::size_t operator()(const QUuid& key) const { return qHash(key); } }; + + using NodeSourcesIgnoreMap = tbb::concurrent_unordered_map; NodeSourcesIgnoreMap _nodeSourcesIgnoreMap; using HRTFMap = std::unordered_map; From 5e9fb1794954a9853b8d9e933d522273aceed15a Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 7 Feb 2017 18:50:23 +0000 Subject: [PATCH 13/15] use tbb::atomic for cp --- assignment-client/src/audio/AudioMixerClientData.cpp | 3 --- assignment-client/src/audio/AudioMixerClientData.h | 7 ++----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index ac5316db37..791ccb8b03 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -430,8 +430,6 @@ void AudioMixerClientData::cleanupCodec() { } AudioMixerClientData::IgnoreZone& AudioMixerClientData::IgnoreZoneMemo::get(unsigned int frame) { - assert(_frame.is_lock_free()); - // check for a memoized zone if (frame != _frame.load(std::memory_order_acquire)) { AvatarAudioStream* stream = _data.getAvatarAudioStream(); @@ -480,7 +478,6 @@ void AudioMixerClientData::IgnoreNodeCache::cache(bool shouldIgnore) { } bool AudioMixerClientData::IgnoreNodeCache::isCached() { - assert(_isCached.is_lock_free()); return _isCached; } diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index c30923f411..1314023d36 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -130,16 +130,13 @@ private: class IgnoreNodeCache { public: - // std::atomic is not copyable - always initialize uncached - IgnoreNodeCache() {} - IgnoreNodeCache(const IgnoreNodeCache& other) {} - void cache(bool shouldIgnore); bool isCached(); bool shouldIgnore(); private: - std::atomic _isCached { false }; + // tbb::atomic supports copy-ctor + tbb::atomic _isCached { false }; bool _shouldIgnore { false }; }; struct IgnoreNodeCacheHasher { std::size_t operator()(const QUuid& key) const { return qHash(key); } }; From ee699d3fa60a51d88f1e2908eda51578cbf2e87c Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 7 Feb 2017 20:40:06 +0000 Subject: [PATCH 14/15] Revert 'use tbb::atomic for cp' --- assignment-client/src/audio/AudioMixerClientData.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 1314023d36..c30923f411 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -130,13 +130,16 @@ private: class IgnoreNodeCache { public: + // std::atomic is not copyable - always initialize uncached + IgnoreNodeCache() {} + IgnoreNodeCache(const IgnoreNodeCache& other) {} + void cache(bool shouldIgnore); bool isCached(); bool shouldIgnore(); private: - // tbb::atomic supports copy-ctor - tbb::atomic _isCached { false }; + std::atomic _isCached { false }; bool _shouldIgnore { false }; }; struct IgnoreNodeCacheHasher { std::size_t operator()(const QUuid& key) const { return qHash(key); } }; From 553fffd8aee5332b778793920071c1f4714c7373 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 7 Feb 2017 21:49:23 +0000 Subject: [PATCH 15/15] time mix instead of throttle with HIFI_AUDIO_MIXER_DEBUG --- assignment-client/src/audio/AudioMixer.cpp | 4 ++-- .../src/audio/AudioMixerSlave.cpp | 21 +++++++++---------- .../src/audio/AudioMixerStats.cpp | 8 +++---- assignment-client/src/audio/AudioMixerStats.h | 6 +++--- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 42c3e9812b..4885720ce4 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -315,8 +315,8 @@ void AudioMixer::sendStatsPacket() { addTiming(_mixTiming, "mix"); addTiming(_eventsTiming, "events"); -#ifdef HIFI_AUDIO_THROTTLE_DEBUG - timingStats["ns_per_throttle"] = (_stats.totalMixes > 0) ? (float)(_stats.throttleTime / _stats.totalMixes) : 0; +#ifdef HIFI_AUDIO_MIXER_DEBUG + timingStats["ns_per_mix"] = (_stats.totalMixes > 0) ? (float)(_stats.mixTime / _stats.totalMixes) : 0; #endif // call it "avg_..." to keep it higher in the display, sorted alphabetically diff --git a/assignment-client/src/audio/AudioMixerSlave.cpp b/assignment-client/src/audio/AudioMixerSlave.cpp index d41fe598cc..370df60ec5 100644 --- a/assignment-client/src/audio/AudioMixerSlave.cpp +++ b/assignment-client/src/audio/AudioMixerSlave.cpp @@ -133,6 +133,10 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { } }; +#ifdef HIFI_AUDIO_MIXER_DEBUG + auto mixStart = p_high_resolution_clock::now(); +#endif + std::for_each(_begin, _end, [&](const SharedNodePointer& node) { AudioMixerClientData* nodeData = static_cast(node->getLinkedData()); if (!nodeData) { @@ -151,10 +155,6 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { if (!isThrottling) { forAllStreams(node, nodeData, &AudioMixerSlave::mixStream); } else { -#ifdef HIFI_AUDIO_THROTTLE_DEBUG - auto throttleStart = p_high_resolution_clock::now(); -#endif - auto nodeID = node->getUUID(); // compute the node's max relative volume @@ -179,13 +179,6 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { if (!throttledNodes.empty()) { std::push_heap(throttledNodes.begin(), throttledNodes.end()); } - -#ifdef HIFI_AUDIO_THROTTLE_DEBUG - auto throttleEnd = p_high_resolution_clock::now(); - uint64_t throttleTime = - std::chrono::duration_cast(throttleEnd - throttleStart).count(); - stats.throttleTime += throttleTime; -#endif } } }); @@ -215,6 +208,12 @@ bool AudioMixerSlave::prepareMix(const SharedNodePointer& listener) { } } +#ifdef HIFI_AUDIO_MIXER_DEBUG + auto mixEnd = p_high_resolution_clock::now(); + auto mixTime = std::chrono::duration_cast(mixEnd - mixStart); + stats.mixTime += mixTime.count(); +#endif + // use the per listener AudioLimiter to render the mixed data... listenerData->audioLimiter.render(_mixSamples, _bufferSamples, AudioConstants::NETWORK_FRAME_SAMPLES_PER_CHANNEL); diff --git a/assignment-client/src/audio/AudioMixerStats.cpp b/assignment-client/src/audio/AudioMixerStats.cpp index a3a3a215bc..a831210871 100644 --- a/assignment-client/src/audio/AudioMixerStats.cpp +++ b/assignment-client/src/audio/AudioMixerStats.cpp @@ -20,8 +20,8 @@ void AudioMixerStats::reset() { hrtfThrottleRenders = 0; manualStereoMixes = 0; manualEchoMixes = 0; -#ifdef HIFI_AUDIO_THROTTLE_DEBUG - throttleTime = 0; +#ifdef HIFI_AUDIO_MIXER_DEBUG + mixTime = 0; #endif } @@ -34,7 +34,7 @@ void AudioMixerStats::accumulate(const AudioMixerStats& otherStats) { hrtfThrottleRenders += otherStats.hrtfThrottleRenders; manualStereoMixes += otherStats.manualStereoMixes; manualEchoMixes += otherStats.manualEchoMixes; -#ifdef HIFI_AUDIO_THROTTLE_DEBUG - throttleTime += otherStats.throttleTime; +#ifdef HIFI_AUDIO_MIXER_DEBUG + mixTime += otherStats.mixTime; #endif } diff --git a/assignment-client/src/audio/AudioMixerStats.h b/assignment-client/src/audio/AudioMixerStats.h index f7e3ed1525..77ac8b985d 100644 --- a/assignment-client/src/audio/AudioMixerStats.h +++ b/assignment-client/src/audio/AudioMixerStats.h @@ -12,7 +12,7 @@ #ifndef hifi_AudioMixerStats_h #define hifi_AudioMixerStats_h -#ifdef HIFI_AUDIO_THROTTLE_DEBUG +#ifdef HIFI_AUDIO_MIXER_DEBUG #include #endif @@ -29,8 +29,8 @@ struct AudioMixerStats { int manualStereoMixes { 0 }; int manualEchoMixes { 0 }; -#ifdef HIFI_AUDIO_THROTTLE_DEBUG - uint64_t throttleTime { 0 }; +#ifdef HIFI_AUDIO_MIXER_DEBUG + uint64_t mixTime { 0 }; #endif void reset();