From 025e13d7932e4bcff92344d235ae71f814df2e72 Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Wed, 18 Aug 2021 15:26:14 +0100 Subject: [PATCH 1/6] Iterate through item references - not copies (WIP) ...Otherwise you enact nothing. BREAKS UI! This reveals an underlying UI bug in that it is now impossible to check the checkboxes in the Add Item dialog. --- ear-production-suite-plugins/lib/src/scene_backend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ear-production-suite-plugins/lib/src/scene_backend.cpp b/ear-production-suite-plugins/lib/src/scene_backend.cpp index f961f2f35..c9c46716f 100644 --- a/ear-production-suite-plugins/lib/src/scene_backend.cpp +++ b/ear-production-suite-plugins/lib/src/scene_backend.cpp @@ -50,7 +50,7 @@ void SceneBackend::triggerMetadataSend() { } }); std::lock_guard lock(storeMutex_); - for (auto item : itemStore_) { + for (auto& item : itemStore_) { item.second.set_changed(false); } } From 7fa732bc0567412ce310dbfc3e3cfe9471f29126 Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Wed, 18 Aug 2021 15:26:42 +0100 Subject: [PATCH 2/6] Do not reset the checked state in Scene add item window on updates Checked state should remain as the user left it. --- .../plugins/scene/src/item_view.hpp | 5 +++++ .../scene/src/scene_frontend_connector.cpp | 16 ++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/ear-production-suite-plugins/plugins/scene/src/item_view.hpp b/ear-production-suite-plugins/plugins/scene/src/item_view.hpp index ae655fde5..17ed3a274 100644 --- a/ear-production-suite-plugins/plugins/scene/src/item_view.hpp +++ b/ear-production-suite-plugins/plugins/scene/src/item_view.hpp @@ -60,6 +60,11 @@ class ItemView : public Component { repaint(); } + void setMetadata(ear::plugin::proto::InputItemMetadata metadata) { + data_.metadata = metadata; + repaint(); + } + void setSelected(bool selected) { data_.selected = selected; repaint(); diff --git a/ear-production-suite-plugins/plugins/scene/src/scene_frontend_connector.cpp b/ear-production-suite-plugins/plugins/scene/src/scene_frontend_connector.cpp index 64c91d256..c5e81a85a 100644 --- a/ear-production-suite-plugins/plugins/scene/src/scene_frontend_connector.cpp +++ b/ear-production-suite-plugins/plugins/scene/src/scene_frontend_connector.cpp @@ -177,11 +177,11 @@ void JuceSceneFrontendConnector::reloadItemListCache() { }); if (it == container->directSpeakersItems.end()) { auto view = std::make_shared(); - view->setData({item, false}); + view->setMetadata(item); container->directSpeakersItems.push_back(view); container->directSpeakersList->addItem(view.get()); } else { - (*it)->setData({item, false}); + (*it)->setMetadata(item); } } else if (item.has_obj_metadata()) { auto it = std::find_if( @@ -192,11 +192,11 @@ void JuceSceneFrontendConnector::reloadItemListCache() { }); if (it == container->objectsItems.end()) { auto view = std::make_shared(); - view->setData({item, false}); + view->setMetadata(item); container->objectsItems.push_back(view); container->objectsList->addItem(view.get()); } else { - (*it)->setData({item, false}); + (*it)->setMetadata(item); } } } @@ -301,10 +301,10 @@ void JuceSceneFrontendConnector::updateItemView(communication::ConnectionId id, container->directSpeakersItems.end(), [id](auto entry) { return id == entry->getId(); }); if (it != container->directSpeakersItems.end()) { - (*it)->setData({item, false}); + (*it)->setMetadata(item); } else { auto view = std::make_shared(); - view->setData({item, false}); + view->setMetadata(item); container->directSpeakersItems.push_back(view); container->directSpeakersList->addItem(view.get()); } @@ -313,10 +313,10 @@ void JuceSceneFrontendConnector::updateItemView(communication::ConnectionId id, container->objectsItems.end(), [id](auto entry) { return id == entry->getId(); }); if (it != container->objectsItems.end()) { - (*it)->setData({item, false}); + (*it)->setMetadata(item); } else { auto view = std::make_shared(); - view->setData({item, false}); + view->setMetadata(item); container->objectsItems.push_back(view); container->objectsList->addItem(view.get()); } From 12aabef485e33b0bff93e80d67b38a41bd1b85b8 Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Wed, 18 Aug 2021 15:27:04 +0100 Subject: [PATCH 3/6] Input plugins set metadata changed state to false after send --- .../lib/src/communication/direct_speakers_metadata_sender.cpp | 1 + .../lib/src/communication/object_metadata_sender.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/ear-production-suite-plugins/lib/src/communication/direct_speakers_metadata_sender.cpp b/ear-production-suite-plugins/lib/src/communication/direct_speakers_metadata_sender.cpp index 65d712af0..8dfaf4042 100644 --- a/ear-production-suite-plugins/lib/src/communication/direct_speakers_metadata_sender.cpp +++ b/ear-production-suite-plugins/lib/src/communication/direct_speakers_metadata_sender.cpp @@ -73,6 +73,7 @@ void DirectSpeakersMetadataSender::sendMetadata() { if (!ec) { std::lock_guard lock(timeoutMutex_); lastSendTimestamp_ = std::chrono::system_clock::now(); + data_.set_changed(false); } else { EAR_LOGGER_WARN(logger_, "Metadata sending failed: {}", ec.message()); } diff --git a/ear-production-suite-plugins/lib/src/communication/object_metadata_sender.cpp b/ear-production-suite-plugins/lib/src/communication/object_metadata_sender.cpp index 58783420b..eeed35a15 100644 --- a/ear-production-suite-plugins/lib/src/communication/object_metadata_sender.cpp +++ b/ear-production-suite-plugins/lib/src/communication/object_metadata_sender.cpp @@ -71,6 +71,7 @@ void ObjectMetadataSender::sendMetadata() { if (!ec) { std::lock_guard lock(timeoutMutex_); lastSendTimestamp_ = std::chrono::system_clock::now(); + data_.set_changed(false); } else { EAR_LOGGER_WARN(logger_, "Metadata sending failed: {}", ec.message()); } From 17e7c23d2d15665c35136d772b3cef4779c79b2b Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Thu, 19 Aug 2021 12:17:06 +0100 Subject: [PATCH 4/6] sceneStroe must be rebuilt if any itemStore changed flag changes Otherwise the old flag state still exists in the sceneStore --- ear-production-suite-plugins/lib/src/scene_backend.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ear-production-suite-plugins/lib/src/scene_backend.cpp b/ear-production-suite-plugins/lib/src/scene_backend.cpp index c9c46716f..10311de45 100644 --- a/ear-production-suite-plugins/lib/src/scene_backend.cpp +++ b/ear-production-suite-plugins/lib/src/scene_backend.cpp @@ -51,7 +51,10 @@ void SceneBackend::triggerMetadataSend() { }); std::lock_guard lock(storeMutex_); for (auto& item : itemStore_) { - item.second.set_changed(false); + if(item.second.changed()) { + item.second.set_changed(false); + rebuildSceneStore_ = true; + } } } From c40cb1efbc4057269ffb48238296c50615e55da2 Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Fri, 20 Aug 2021 16:17:14 +0100 Subject: [PATCH 5/6] Avoid race - Don't set changed false after async send This creates a race condition where we might set false AFTER new data has come in (which in turn wouldn't get processed). Instead set false immediately after generating the message, then set back true if send fails. If data has changed in the meantime, then that's also fine - it should be true for that case too anyway. --- .../lib/src/communication/direct_speakers_metadata_sender.cpp | 3 ++- .../lib/src/communication/object_metadata_sender.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ear-production-suite-plugins/lib/src/communication/direct_speakers_metadata_sender.cpp b/ear-production-suite-plugins/lib/src/communication/direct_speakers_metadata_sender.cpp index 8dfaf4042..b3ce86852 100644 --- a/ear-production-suite-plugins/lib/src/communication/direct_speakers_metadata_sender.cpp +++ b/ear-production-suite-plugins/lib/src/communication/direct_speakers_metadata_sender.cpp @@ -68,13 +68,14 @@ void DirectSpeakersMetadataSender::sendMetadata() { return; } auto msg = getMessage(); + data_.set_changed(false); socket_.asyncSend( msg, [this](std::error_code ec, const nng::Message& ignored) { if (!ec) { std::lock_guard lock(timeoutMutex_); lastSendTimestamp_ = std::chrono::system_clock::now(); - data_.set_changed(false); } else { + data_.set_changed(true); EAR_LOGGER_WARN(logger_, "Metadata sending failed: {}", ec.message()); } }); diff --git a/ear-production-suite-plugins/lib/src/communication/object_metadata_sender.cpp b/ear-production-suite-plugins/lib/src/communication/object_metadata_sender.cpp index eeed35a15..5d695eff6 100644 --- a/ear-production-suite-plugins/lib/src/communication/object_metadata_sender.cpp +++ b/ear-production-suite-plugins/lib/src/communication/object_metadata_sender.cpp @@ -66,13 +66,14 @@ void ObjectMetadataSender::sendMetadata() { return; } auto msg = getMessage(); + data_.set_changed(false); socket_.asyncSend( msg, [this](std::error_code ec, const nng::Message& ignored) { if (!ec) { std::lock_guard lock(timeoutMutex_); lastSendTimestamp_ = std::chrono::system_clock::now(); - data_.set_changed(false); } else { + data_.set_changed(true); EAR_LOGGER_WARN(logger_, "Metadata sending failed: {}", ec.message()); } }); From 130d001551967306fa0093b90f567acb6fe0b33d Mon Sep 17 00:00:00 2001 From: Matt Firth Date: Fri, 20 Aug 2021 16:22:37 +0100 Subject: [PATCH 6/6] Use metadata reference for ItemView::setMetadata method .. since it will be copied anyway --- ear-production-suite-plugins/plugins/scene/src/item_view.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ear-production-suite-plugins/plugins/scene/src/item_view.hpp b/ear-production-suite-plugins/plugins/scene/src/item_view.hpp index 17ed3a274..6b4854391 100644 --- a/ear-production-suite-plugins/plugins/scene/src/item_view.hpp +++ b/ear-production-suite-plugins/plugins/scene/src/item_view.hpp @@ -60,7 +60,7 @@ class ItemView : public Component { repaint(); } - void setMetadata(ear::plugin::proto::InputItemMetadata metadata) { + void setMetadata(ear::plugin::proto::InputItemMetadata const& metadata) { data_.metadata = metadata; repaint(); }