From e175410f549044f34b81cd88c8161ee3823be78b Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Tue, 16 Jul 2024 11:12:41 -0600 Subject: [PATCH] midi surfaces fixes (partially for PR #898) 1. do more to ensure that we do not call MidiSurface::begin_using_device() multiple times without ::stop_using_device() in between. This reduces the risk of duplicate signal handler connections being made (it might even eliminate it). 2. Notify all control surfaces when MIDI connectivity is established AND disestablished. This gives them a chance to update their notion of their current connection state. This can be important with JACK across zombification, but also likely across backend stop&start. These changes currntly only impact classes derived from MidiSurface but something equivalent is required for all control surfaces --- libs/ardour/ardour/control_protocol_manager.h | 2 +- libs/ardour/control_protocol_manager.cc | 4 +- libs/ardour/session_state.cc | 2 +- libs/ardour/session_transport.cc | 3 ++ .../control_protocol/control_protocol.h | 2 +- .../midi_surface/midi_surface.cc | 46 ++++++++++++++----- .../midi_surface/midi_surface/midi_surface.h | 1 + 7 files changed, 43 insertions(+), 17 deletions(-) diff --git a/libs/ardour/ardour/control_protocol_manager.h b/libs/ardour/ardour/control_protocol_manager.h index 4b0a688653..01576bf5a2 100644 --- a/libs/ardour/ardour/control_protocol_manager.h +++ b/libs/ardour/ardour/control_protocol_manager.h @@ -72,7 +72,7 @@ class LIBARDOUR_API ControlProtocolManager : public PBD::Stateful, public ARDOUR void set_session (Session*); void discover_control_protocols (); void foreach_known_protocol (boost::function); - void midi_connectivity_established (); + void midi_connectivity_established (bool); void drop_protocols (); void probe_midi_control_protocols (); void probe_usb_control_protocols (bool, uint16_t, uint16_t); diff --git a/libs/ardour/control_protocol_manager.cc b/libs/ardour/control_protocol_manager.cc index b316abc0f6..fe570c8dff 100644 --- a/libs/ardour/control_protocol_manager.cc +++ b/libs/ardour/control_protocol_manager.cc @@ -619,12 +619,12 @@ ControlProtocolManager::instance () } void -ControlProtocolManager::midi_connectivity_established () +ControlProtocolManager::midi_connectivity_established (bool yn) { Glib::Threads::RWLock::ReaderLock lm (protocols_lock); for (list::iterator p = control_protocols.begin(); p != control_protocols.end(); ++p) { - (*p)->midi_connectivity_established (); + (*p)->midi_connectivity_established (yn); } } diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc index 61f4f3c1c2..08c01cb9df 100644 --- a/libs/ardour/session_state.cc +++ b/libs/ardour/session_state.cc @@ -355,7 +355,7 @@ Session::post_engine_init () * could start talking to surfaces if they want to. */ - ControlProtocolManager::instance().midi_connectivity_established (); + ControlProtocolManager::instance().midi_connectivity_established (true); if (_is_new && !no_auto_connect()) { Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock()); diff --git a/libs/ardour/session_transport.cc b/libs/ardour/session_transport.cc index 941da0cea9..6d17ecd532 100644 --- a/libs/ardour/session_transport.cc +++ b/libs/ardour/session_transport.cc @@ -52,6 +52,7 @@ #include "ardour/automation_watch.h" #include "ardour/butler.h" #include "ardour/click.h" +#include "ardour/control_protocol_manager.h" #include "ardour/debug.h" #include "ardour/disk_reader.h" #include "ardour/io_tasklist.h" @@ -1868,6 +1869,8 @@ Session::engine_halted () */ realtime_stop (false, true); + + ControlProtocolManager::instance().midi_connectivity_established (false); } void diff --git a/libs/ctrl-interface/control_protocol/control_protocol/control_protocol.h b/libs/ctrl-interface/control_protocol/control_protocol/control_protocol.h index 32804fa5ca..6c1db91cc9 100644 --- a/libs/ctrl-interface/control_protocol/control_protocol/control_protocol.h +++ b/libs/ctrl-interface/control_protocol/control_protocol/control_protocol.h @@ -57,7 +57,7 @@ public: virtual int set_feedback (bool /*yn*/) { return 0; } virtual bool get_feedback () const { return false; } - virtual void midi_connectivity_established () {} + virtual void midi_connectivity_established (bool) {} virtual void stripable_selection_changed () = 0; diff --git a/libs/ctrl-interface/midi_surface/midi_surface.cc b/libs/ctrl-interface/midi_surface/midi_surface.cc index 12e6391e4a..5081697de2 100644 --- a/libs/ctrl-interface/midi_surface/midi_surface.cc +++ b/libs/ctrl-interface/midi_surface/midi_surface.cc @@ -256,6 +256,8 @@ MIDISurface::connection_handler (std::weak_ptr, std::string name1, std::string ni = ARDOUR::AudioEngine::instance()->make_port_name_non_relative (std::shared_ptr(_async_in)->name()); std::string no = ARDOUR::AudioEngine::instance()->make_port_name_non_relative (std::shared_ptr(_async_out)->name()); + int old_connection_state = _connection_state; + if (ni == name1 || ni == name2) { if (yn) { _connection_state |= InputConnected; @@ -277,23 +279,34 @@ MIDISurface::connection_handler (std::weak_ptr, std::string name1, DEBUG_TRACE (DEBUG::MIDISurface, string_compose ("our ports changed connection state: %1 -> %2 connected ? %3, connection state now %4\n", name1, name2, yn, _connection_state)); - if ((_connection_state & (InputConnected|OutputConnected)) == (InputConnected|OutputConnected)) { + /* it ought o be impossible for the connection state of our ports to + * change without a corresponding change in _connection_state. But + * since the consequences of calling device_acquire() and + * begin_using_device() are substantial, include it as a test to catch + * any weird corner cases. + */ - /* XXX this is a horrible hack. Without a short sleep here, - something prevents the device wakeup messages from being - sent and/or the responses from being received. - */ + if ((_connection_state != old_connection_state) && (_connection_state & (InputConnected|OutputConnected)) == (InputConnected|OutputConnected)) { DEBUG_TRACE (DEBUG::MIDISurface, "device now connected for both input and output\n"); - g_usleep (100000); - /* may not have the device open if it was just plugged - in. Really need USB device detection rather than MIDI port - detection for this to work well. - */ + if (!_in_use) { - device_acquire (); - begin_using_device (); + /* XXX this is a horrible hack. Without a short sleep here, + something prevents the device wakeup messages from being + sent and/or the responses from being received. + */ + + g_usleep (100000); + + /* may not have the device open if it was just plugged + in. Really need USB device detection rather than MIDI port + detection for this to work well. + */ + + device_acquire (); + begin_using_device (); + } } else { DEBUG_TRACE (DEBUG::MIDISurface, "Device disconnected (input or output or both) or not yet fully connected\n"); @@ -332,6 +345,15 @@ MIDISurface::write (MIDI::byte const * data, size_t size) _output_port->write (data, size, 0); } +void +MIDISurface::midi_connectivity_established (bool yn) +{ + if (!yn) { + _connection_state = ConnectionState (0); + stop_using_device (); + } +} + bool MIDISurface::midi_input_handler (IOCondition ioc, MIDI::Port* port) { diff --git a/libs/ctrl-interface/midi_surface/midi_surface/midi_surface.h b/libs/ctrl-interface/midi_surface/midi_surface/midi_surface.h index 301fc29670..9248c39343 100644 --- a/libs/ctrl-interface/midi_surface/midi_surface/midi_surface.h +++ b/libs/ctrl-interface/midi_surface/midi_surface/midi_surface.h @@ -85,6 +85,7 @@ class MIDISurface : public ARDOUR::ControlProtocol CONTROL_PROTOCOL_THREADS_NEED_TEMPO_MAP_DECL(); virtual bool midi_input_handler (Glib::IOCondition ioc, MIDI::Port* port); + void midi_connectivity_established (bool); protected: bool with_pad_filter;