modularize audio ignore zone computations

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<AudioMixerClientData*>(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<std::mutex> 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<std::mutex> 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<AudioMixerClientData*>(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;
+}

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 {
+    class IgnoreZoneMemo {
-        IgnoreZone zone;
-        std::atomic<unsigned int> frame { 0 };
-        std::mutex mutex;
-    };
-    IgnoreZoneMemo _ignoreZoneMemo;
-
-    class IgnoreNodeData {
     public:
-        // always begin unset
+        IgnoreZoneMemo(AudioMixerClientData& data) : _data(data) {}
-        IgnoreNodeData() {}
+
-        IgnoreNodeData(const IgnoreNodeData& other) {}
+        // 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<unsigned int> _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<bool> _flag { false };
-        bool _ignore { false };
-    };
-    struct IgnoreNodeDataHasher { std::size_t operator()(const QUuid& key) const { return qHash(key); } };
 
-    using NodeSourcesIgnoreMap = tbb::concurrent_unordered_map<QUuid, IgnoreNodeData, IgnoreNodeDataHasher>;
+    private:
+        std::atomic<bool> _isCached { false };
+        bool _shouldIgnore { false };
+    };
+    struct IgnoreNodeCacheHasher { std::size_t operator()(const QUuid& key) const { return qHash(key); } };
+
+    using NodeSourcesIgnoreMap = tbb::concurrent_unordered_map<QUuid, IgnoreNodeCache, IgnoreNodeCacheHasher>;
     NodeSourcesIgnoreMap _nodeSourcesIgnoreMap;
 
     using HRTFMap = std::unordered_map<QUuid, AudioHRTF>;