From cb73eb350d2aefbcddd1821a7b2c951414109010 Mon Sep 17 00:00:00 2001 From: Luciano Iam Date: Sun, 13 Jun 2021 23:49:13 +0200 Subject: [PATCH] WebSockets: less invasive version of 5407232 There is no need to connect signals twice, can connect them directly to the helper UI loop and skip the surface loop. Then let the server decide if it is necessary to call lws_cancel_service() or not. Also rename WebsocketsServer::should_request_write() to read_blocks_event_loop() it makes more sense for the caller now on-demand write logic is completely implemented by the server class. --- libs/surfaces/websockets/component.h | 2 +- libs/surfaces/websockets/feedback.cc | 43 +++++++++------------------- libs/surfaces/websockets/feedback.h | 6 ++-- libs/surfaces/websockets/server.cc | 24 +++++++++------- libs/surfaces/websockets/server.h | 5 ++-- 5 files changed, 36 insertions(+), 44 deletions(-) diff --git a/libs/surfaces/websockets/component.h b/libs/surfaces/websockets/component.h index e9e80ec447..7b2443bd1c 100644 --- a/libs/surfaces/websockets/component.h +++ b/libs/surfaces/websockets/component.h @@ -53,7 +53,7 @@ public: } BasicUI& basic_ui () const; - PBD::EventLoop* event_loop () const; + virtual PBD::EventLoop* event_loop () const; Glib::RefPtr main_loop () const; ARDOUR::Session& session () const; ArdourMixer& mixer () const; diff --git a/libs/surfaces/websockets/feedback.cc b/libs/surfaces/websockets/feedback.cc index 9542c1beff..4e34f08e11 100644 --- a/libs/surfaces/websockets/feedback.cc +++ b/libs/surfaces/websockets/feedback.cc @@ -30,20 +30,9 @@ // TO DO: make this configurable #define POLL_INTERVAL_MS 100 -#define OPTIONAL_CONNECT_HELPER(s,c) if (server ().should_request_write ()) \ - s.connect (c, MISSING_INVALIDATOR, boost::bind \ - (ServerWriteObserver (), &server ()), &_helper); - using namespace ARDOUR; using namespace ArdourSurface; -struct ServerWriteObserver { - void operator() (WebsocketsServer *server) - { - server->request_write (); - } -}; - struct TransportObserver { void operator() (ArdourFeedback* p) { @@ -141,11 +130,11 @@ ArdourFeedback::start () // server must be started before feedback otherwise // should_request_write() will always return false - if (!server ().should_request_write ()) { - periodic_timeout->attach (main_loop ()->get_context ()); - } else { + if (server ().read_blocks_event_loop ()) { _helper.run(); periodic_timeout->attach (_helper.main_loop()->get_context ()); + } else { + periodic_timeout->attach (main_loop ()->get_context ()); } return 0; @@ -154,7 +143,7 @@ ArdourFeedback::start () int ArdourFeedback::stop () { - if (server ().should_request_write ()) { + if (server ().read_blocks_event_loop ()) { _helper.quit(); } @@ -207,6 +196,16 @@ ArdourFeedback::update_all (std::string node, uint32_t strip_id, uint32_t plugin server ().update_all_clients (NodeState (node, addr, val), false); } +PBD::EventLoop* +ArdourFeedback::event_loop () const +{ + if (server ().read_blocks_event_loop ()) { + return static_cast (&_helper); + } else { + return SurfaceComponent::event_loop (); + } +} + bool ArdourFeedback::poll () const { @@ -219,10 +218,6 @@ ArdourFeedback::poll () const update_all (Node::strip_meter, it->first, db); } - if (server ().should_request_write ()) { - server ().request_write (); - } - return true; } @@ -232,15 +227,10 @@ ArdourFeedback::observe_transport () ARDOUR::Session& sess = session (); sess.TransportStateChange.connect (_transport_connections, MISSING_INVALIDATOR, boost::bind (TransportObserver (), this), event_loop ()); - OPTIONAL_CONNECT_HELPER(sess.TransportStateChange, _transport_connections); - sess.RecordStateChanged.connect (_transport_connections, MISSING_INVALIDATOR, boost::bind (RecordStateObserver (), this), event_loop ()); - OPTIONAL_CONNECT_HELPER(sess.RecordStateChanged, _transport_connections); - sess.tempo_map ().PropertyChanged.connect (_transport_connections, MISSING_INVALIDATOR, boost::bind (TempoObserver (), this), event_loop ()); - OPTIONAL_CONNECT_HELPER(sess.tempo_map ().PropertyChanged, _transport_connections); } void @@ -254,17 +244,14 @@ ArdourFeedback::observe_mixer () stripable->gain_control ()->Changed.connect (*it->second, MISSING_INVALIDATOR, boost::bind (StripGainObserver (), this, strip_id), event_loop ()); - OPTIONAL_CONNECT_HELPER(stripable->gain_control ()->Changed, *it->second); if (stripable->pan_azimuth_control ()) { stripable->pan_azimuth_control ()->Changed.connect (*it->second, MISSING_INVALIDATOR, boost::bind (StripPanObserver (), this, strip_id), event_loop ()); - OPTIONAL_CONNECT_HELPER(stripable->pan_azimuth_control ()->Changed, *it->second); } stripable->mute_control ()->Changed.connect (*it->second, MISSING_INVALIDATOR, boost::bind (StripMuteObserver (), this, strip_id), event_loop ()); - OPTIONAL_CONNECT_HELPER(stripable->mute_control ()->Changed, *it->second); observe_strip_plugins (strip_id, strip->plugins ()); } @@ -284,7 +271,6 @@ ArdourFeedback::observe_strip_plugins (uint32_t strip_id, ArdourMixerStrip::Plug if (control) { control->Changed.connect (*plugin, MISSING_INVALIDATOR, boost::bind (PluginBypassObserver (), this, strip_id, plugin_id), event_loop ()); - OPTIONAL_CONNECT_HELPER(control->Changed, *plugin); } for (uint32_t param_id = 0; param_id < plugin->param_count (); ++param_id) { @@ -295,7 +281,6 @@ ArdourFeedback::observe_strip_plugins (uint32_t strip_id, ArdourMixerStrip::Plug boost::bind (PluginParamValueObserver (), this, strip_id, plugin_id, param_id, boost::weak_ptr(control)), event_loop ()); - OPTIONAL_CONNECT_HELPER(control->Changed, *plugin); } catch (ArdourMixerNotFoundException& e) { /* ignore */ } diff --git a/libs/surfaces/websockets/feedback.h b/libs/surfaces/websockets/feedback.h index fdf600d3ee..f236be8a72 100644 --- a/libs/surfaces/websockets/feedback.h +++ b/libs/surfaces/websockets/feedback.h @@ -62,8 +62,10 @@ private: PBD::ScopedConnectionList _transport_connections; sigc::connection _periodic_connection; - // Only needed for server event loop integration method 3 - FeedbackHelperUI _helper; + // Only needed for server event loop integration method #3 + mutable FeedbackHelperUI _helper; + + PBD::EventLoop* event_loop () const override; bool poll () const; diff --git a/libs/surfaces/websockets/server.cc b/libs/surfaces/websockets/server.cc index 7dc6497a06..c6291e47fa 100644 --- a/libs/surfaces/websockets/server.cc +++ b/libs/surfaces/websockets/server.cc @@ -218,7 +218,7 @@ WebsocketsServer::update_client (Client wsi, const NodeState& state, bool force) /* write to client only if state was updated */ it->second.update_state (state); it->second.output_buf ().push_back (NodeStateMessage (state)); - lws_callback_on_writable (wsi); + request_write (wsi); } } @@ -308,7 +308,7 @@ WebsocketsServer::write_client (Client wsi) } if (!pending.empty ()) { - lws_callback_on_writable (wsi); + request_write (wsi); } return 0; @@ -360,7 +360,7 @@ WebsocketsServer::send_availsurf_hdr (Client wsi) } #endif - lws_callback_on_writable (wsi); + request_write (wsi); return 0; } @@ -393,6 +393,17 @@ WebsocketsServer::send_availsurf_body (Client wsi) return -1; // end connection } +void +WebsocketsServer::request_write (Client wsi) +{ + lws_callback_on_writable (wsi); + + if (read_blocks_event_loop ()) { + // cancel lws_service() in the idle callback to write pending data asap + lws_cancel_service (_lws_context); + } +} + int WebsocketsServer::lws_callback (struct lws* wsi, enum lws_callback_reasons reason, void* user, void* in, size_t len) @@ -631,13 +642,6 @@ WebsocketsServer::ioc_to_events (IOCondition ioc) return events; } -void -WebsocketsServer::request_write () -{ - // cancel lws_service() in the idle callback to write pending data asap - lws_cancel_service (_lws_context); -} - gboolean WebsocketsServer::glib_idle_callback (void *data) { diff --git a/libs/surfaces/websockets/server.h b/libs/surfaces/websockets/server.h index 2986914b5f..da81ccbffb 100644 --- a/libs/surfaces/websockets/server.h +++ b/libs/surfaces/websockets/server.h @@ -75,6 +75,8 @@ private: int send_availsurf_hdr (Client); int send_availsurf_body (Client); + void request_write (Client); + static int lws_callback (struct lws*, enum lws_callback_reasons, void*, void*, size_t); /* Glib event loop integration that requires LWS_WITH_EXTERNAL_POLL */ @@ -115,8 +117,7 @@ private: static gboolean glib_idle_callback (void *); public: - bool should_request_write () { return _g_source != 0; } - void request_write (); + bool read_blocks_event_loop () { return _g_source != 0; } };