From 942ba7b7ae1e751887fe7cb34b3aa88277086f3e Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Mon, 13 May 2024 12:41:29 +0100 Subject: [PATCH 1/4] Launch message handler thread further up stack This ensures message handlers for both Bin mon and LS mons are launched in a new thread to overcome NNG stack limits --- .../monitoring_metadata_receiver.cpp | 8 ++- .../lib/src/scene_gains_calculator.cpp | 54 ++++++++----------- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp b/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp index 34d17afa6..fd6a96d21 100644 --- a/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp +++ b/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp @@ -1,6 +1,7 @@ #include "communication/monitoring_metadata_receiver.hpp" #include "scene_store.pb.h" #include +#include namespace ear { namespace plugin { @@ -62,7 +63,12 @@ void MonitoringMetadataReceiver::handleReceive(std::error_code ec, if (!sceneStore.ParseFromArray(message.data(), message.size())) { throw std::runtime_error("Failed to parse Scene Object"); } - handler_(std::move(sceneStore)); + // Called by NNG callback on thread with small stack. + // Launch task in another thread to overcome stack limitation. + auto future = std::async(std::launch::async, [this, sceneStore]() { + handler_(sceneStore); + }); + future.get(); } catch (const std::runtime_error& e) { EAR_LOGGER_ERROR( logger_, "Failed to parse and dispatch scene metadata: {}", e.what()); diff --git a/ear-production-suite-plugins/lib/src/scene_gains_calculator.cpp b/ear-production-suite-plugins/lib/src/scene_gains_calculator.cpp index 958910ad2..3f2b08733 100644 --- a/ear-production-suite-plugins/lib/src/scene_gains_calculator.cpp +++ b/ear-production-suite-plugins/lib/src/scene_gains_calculator.cpp @@ -2,10 +2,8 @@ #include "ear/metadata.hpp" #include "helper/eps_to_ear_metadata_converter.hpp" #include "helper/container_helpers.hpp" -#include #include - namespace { int inputCount(ear::plugin::ItemGains const& itemGains) { @@ -54,40 +52,32 @@ SceneGainsCalculator::SceneGainsCalculator(ear::Layout outputLayout, totalInputChannels{inputChannelCount} {} bool SceneGainsCalculator::update(proto::SceneStore store) { - // Called by NNG callback on thread with small stack. - // Launch task in another thread to overcome stack limitation. - auto future = std::async(std::launch::async, [this, store]() { - - // First figure out what we need to process updates for - std::vector cachedIdsChecklist; - cachedIdsChecklist.reserve(routingCache_.size()); - for(auto const&[key, val] : routingCache_) { - cachedIdsChecklist.push_back(key); - } - /// Check-off found items, and also delete changed items from routing cache to be re-evaluated - for(const auto& item : store.monitoring_items()) { - auto itemId = communication::ConnectionId{ item.connection_id() }; - cachedIdsChecklist.erase(std::remove(cachedIdsChecklist.begin(), cachedIdsChecklist.end(), itemId), cachedIdsChecklist.end()); - if(item.changed()) { - removeItem(itemId); - } - } - /// Delete removed items from routing cache (i.e, those that weren't checked-off and therefore remain in cachedIdsChecklist) - for(const auto& itemId : cachedIdsChecklist) { + // First figure out what we need to process updates for + std::vector cachedIdsChecklist; + cachedIdsChecklist.reserve(routingCache_.size()); + for(auto const&[key, val] : routingCache_) { + cachedIdsChecklist.push_back(key); + } + /// Check-off found items, and also delete changed items from routing cache to be re-evaluated + for(const auto& item : store.monitoring_items()) { + auto itemId = communication::ConnectionId{ item.connection_id() }; + cachedIdsChecklist.erase(std::remove(cachedIdsChecklist.begin(), cachedIdsChecklist.end(), itemId), cachedIdsChecklist.end()); + if(item.changed()) { removeItem(itemId); } + } + /// Delete removed items from routing cache (i.e, those that weren't checked-off and therefore remain in cachedIdsChecklist) + for(const auto& itemId : cachedIdsChecklist) { + removeItem(itemId); + } - // Now get the gain updates we need - for(const auto& item : store.monitoring_items()) { - /// If it's not in routingCache_, it's new or changed, so needs re-evaluating - if(!mapHasKey(routingCache_, communication::ConnectionId{ item.connection_id() })) { - addOrUpdateItem(item); - } + // Now get the gain updates we need + for(const auto& item : store.monitoring_items()) { + /// If it's not in routingCache_, it's new or changed, so needs re-evaluating + if(!mapHasKey(routingCache_, communication::ConnectionId{ item.connection_id() })) { + addOrUpdateItem(item); } - - }); - - future.get(); + } return true; } From d2160659b0e8d87542d04b1625e06f0417f5043e Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Mon, 13 May 2024 16:32:34 +0100 Subject: [PATCH 2/4] Avoid unnecessary copy in async lambda --- .../lib/src/communication/monitoring_metadata_receiver.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp b/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp index fd6a96d21..906533cc0 100644 --- a/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp +++ b/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp @@ -65,10 +65,10 @@ void MonitoringMetadataReceiver::handleReceive(std::error_code ec, } // Called by NNG callback on thread with small stack. // Launch task in another thread to overcome stack limitation. - auto future = std::async(std::launch::async, [this, sceneStore]() { + auto future = std::async(std::launch::async, [this, sceneStore = std::move(sceneStore)]() { handler_(sceneStore); }); - future.get(); + future.get(); //blocking } catch (const std::runtime_error& e) { EAR_LOGGER_ERROR( logger_, "Failed to parse and dispatch scene metadata: {}", e.what()); From ab7adb945015cb50ad2e241ac4274c2fff7f3558 Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Tue, 14 May 2024 16:40:35 +0100 Subject: [PATCH 3/4] Pass SceneStore as const ref through handlers --- .../lib/include/binaural_monitoring_backend.hpp | 2 +- .../communication/monitoring_metadata_receiver.hpp | 2 +- .../lib/include/monitoring_backend.hpp | 4 ++-- .../lib/include/scene_gains_calculator.hpp | 2 +- .../lib/src/binaural_monitoring_backend.cpp | 3 ++- .../src/communication/monitoring_metadata_receiver.cpp | 2 +- .../lib/src/monitoring_backend.cpp | 8 ++++---- .../lib/src/scene_gains_calculator.cpp | 2 +- 8 files changed, 13 insertions(+), 12 deletions(-) diff --git a/ear-production-suite-plugins/lib/include/binaural_monitoring_backend.hpp b/ear-production-suite-plugins/lib/include/binaural_monitoring_backend.hpp index 6a6295ee8..888c8baa2 100644 --- a/ear-production-suite-plugins/lib/include/binaural_monitoring_backend.hpp +++ b/ear-production-suite-plugins/lib/include/binaural_monitoring_backend.hpp @@ -68,7 +68,7 @@ class BinauralMonitoringBackend { bool isExporting() { return isExporting_; } private: - void onSceneReceived(proto::SceneStore store); + void onSceneReceived(const proto::SceneStore& store); void onConnection(communication::ConnectionId connectionId, const std::string& streamEndpoint); void onConnectionLost(); diff --git a/ear-production-suite-plugins/lib/include/communication/monitoring_metadata_receiver.hpp b/ear-production-suite-plugins/lib/include/communication/monitoring_metadata_receiver.hpp index 1f48b7759..9da1d3aa9 100644 --- a/ear-production-suite-plugins/lib/include/communication/monitoring_metadata_receiver.hpp +++ b/ear-production-suite-plugins/lib/include/communication/monitoring_metadata_receiver.hpp @@ -14,7 +14,7 @@ class SceneStore; namespace communication { class MonitoringMetadataReceiver { public: - using RequestHandler = std::function; + using RequestHandler = std::function; MonitoringMetadataReceiver(std::shared_ptr logger = nullptr); ~MonitoringMetadataReceiver(); MonitoringMetadataReceiver(const MonitoringMetadataReceiver&) = delete; diff --git a/ear-production-suite-plugins/lib/include/monitoring_backend.hpp b/ear-production-suite-plugins/lib/include/monitoring_backend.hpp index e4caf7239..5f9634792 100644 --- a/ear-production-suite-plugins/lib/include/monitoring_backend.hpp +++ b/ear-production-suite-plugins/lib/include/monitoring_backend.hpp @@ -35,11 +35,11 @@ class MonitoringBackend { bool isExporting() { return isExporting_; } private: - void onSceneReceived(proto::SceneStore store); + void onSceneReceived(const proto::SceneStore& store); void onConnection(communication::ConnectionId connectionId, const std::string& streamEndpoint); void onConnectionLost(); - void updateActiveGains(proto::SceneStore store); + void updateActiveGains(const proto::SceneStore& store); std::shared_ptr logger_; std::mutex gainsMutex_; diff --git a/ear-production-suite-plugins/lib/include/scene_gains_calculator.hpp b/ear-production-suite-plugins/lib/include/scene_gains_calculator.hpp index 0a61a2610..cc1e18d9a 100644 --- a/ear-production-suite-plugins/lib/include/scene_gains_calculator.hpp +++ b/ear-production-suite-plugins/lib/include/scene_gains_calculator.hpp @@ -27,7 +27,7 @@ struct ItemGains { class SceneGainsCalculator { public: SceneGainsCalculator(Layout outputLayout, int inputChannelCount); - bool update(proto::SceneStore store); + bool update(const proto::SceneStore &store); Eigen::MatrixXf directGains(); Eigen::MatrixXf diffuseGains(); diff --git a/ear-production-suite-plugins/lib/src/binaural_monitoring_backend.cpp b/ear-production-suite-plugins/lib/src/binaural_monitoring_backend.cpp index 2defd6d82..80447eb21 100644 --- a/ear-production-suite-plugins/lib/src/binaural_monitoring_backend.cpp +++ b/ear-production-suite-plugins/lib/src/binaural_monitoring_backend.cpp @@ -200,7 +200,8 @@ BinauralMonitoringBackend::getLatestObjectsTypeMetadata(ConnId id) { return std::optional(); } -void BinauralMonitoringBackend::onSceneReceived(proto::SceneStore store) { +void BinauralMonitoringBackend::onSceneReceived( + const proto::SceneStore& store) { isExporting_ = store.has_is_exporting() && store.is_exporting(); size_t totalDsChannels = 0; diff --git a/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp b/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp index 906533cc0..bc3ef4b03 100644 --- a/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp +++ b/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp @@ -65,7 +65,7 @@ void MonitoringMetadataReceiver::handleReceive(std::error_code ec, } // Called by NNG callback on thread with small stack. // Launch task in another thread to overcome stack limitation. - auto future = std::async(std::launch::async, [this, sceneStore = std::move(sceneStore)]() { + auto future = std::async(std::launch::async, [this, sceneStore]() { handler_(sceneStore); }); future.get(); //blocking diff --git a/ear-production-suite-plugins/lib/src/monitoring_backend.cpp b/ear-production-suite-plugins/lib/src/monitoring_backend.cpp index fdd241654..f311ad334 100644 --- a/ear-production-suite-plugins/lib/src/monitoring_backend.cpp +++ b/ear-production-suite-plugins/lib/src/monitoring_backend.cpp @@ -47,9 +47,9 @@ MonitoringBackend::~MonitoringBackend() { controlConnection_.onConnectionEstablished(nullptr); } -void MonitoringBackend::onSceneReceived(proto::SceneStore store) { +void MonitoringBackend::onSceneReceived(const proto::SceneStore& store) { isExporting_ = store.has_is_exporting() && store.is_exporting(); - updateActiveGains(std::move(store)); + updateActiveGains(store); } GainHolder MonitoringBackend::currentGains() { @@ -57,10 +57,10 @@ GainHolder MonitoringBackend::currentGains() { return gains_; } -void MonitoringBackend::updateActiveGains(proto::SceneStore store) { +void MonitoringBackend::updateActiveGains(const proto::SceneStore& store) { { std::lock_guard lock(gainsCalculatorMutex_); - gainsCalculator_.update(std::move(store)); + gainsCalculator_.update(store); } { std::lock_guard lock(gainsMutex_); diff --git a/ear-production-suite-plugins/lib/src/scene_gains_calculator.cpp b/ear-production-suite-plugins/lib/src/scene_gains_calculator.cpp index 3f2b08733..9d094edda 100644 --- a/ear-production-suite-plugins/lib/src/scene_gains_calculator.cpp +++ b/ear-production-suite-plugins/lib/src/scene_gains_calculator.cpp @@ -51,7 +51,7 @@ SceneGainsCalculator::SceneGainsCalculator(ear::Layout outputLayout, totalOutputChannels{static_cast(outputLayout.channels().size())}, totalInputChannels{inputChannelCount} {} -bool SceneGainsCalculator::update(proto::SceneStore store) { +bool SceneGainsCalculator::update(const proto::SceneStore& store) { // First figure out what we need to process updates for std::vector cachedIdsChecklist; cachedIdsChecklist.reserve(routingCache_.size()); From c4be5124c50e3686720366ded50814a1cb236cdd Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Wed, 15 May 2024 11:44:33 +0100 Subject: [PATCH 4/4] Capture ref to sceneStore --- .../lib/src/communication/monitoring_metadata_receiver.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp b/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp index bc3ef4b03..a776848ff 100644 --- a/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp +++ b/ear-production-suite-plugins/lib/src/communication/monitoring_metadata_receiver.cpp @@ -65,7 +65,7 @@ void MonitoringMetadataReceiver::handleReceive(std::error_code ec, } // Called by NNG callback on thread with small stack. // Launch task in another thread to overcome stack limitation. - auto future = std::async(std::launch::async, [this, sceneStore]() { + auto future = std::async(std::launch::async, [this, &sceneStore]() { handler_(sceneStore); }); future.get(); //blocking