From 9a1c22d7e15b428868024df6883bcaf38a16b874 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 01:28:23 +0100 Subject: [PATCH 01/16] Remove unusued API Create/Delete Binding --- libs/pbd/controllable.cc | 2 - libs/pbd/pbd/controllable.h | 2 - .../generic_midi_control_protocol.cc | 61 ------------------- .../generic_midi_control_protocol.h | 3 - 4 files changed, 68 deletions(-) diff --git a/libs/pbd/controllable.cc b/libs/pbd/controllable.cc index c289ee0287..fa25826294 100644 --- a/libs/pbd/controllable.cc +++ b/libs/pbd/controllable.cc @@ -32,8 +32,6 @@ using namespace std; PBD::Signal1 Controllable::Destroyed; PBD::Signal1 Controllable::StartLearning; PBD::Signal1 Controllable::StopLearning; -PBD::Signal3 Controllable::CreateBinding; -PBD::Signal1 Controllable::DeleteBinding; PBD::Signal1 > Controllable::GUIFocusChanged; Glib::Threads::RWLock Controllable::registry_lock; diff --git a/libs/pbd/pbd/controllable.h b/libs/pbd/pbd/controllable.h index abaa933348..d40530c7e2 100644 --- a/libs/pbd/pbd/controllable.h +++ b/libs/pbd/pbd/controllable.h @@ -122,8 +122,6 @@ public: virtual std::string get_user_string() const { return std::string(); } PBD::Signal0 LearningFinished; - static PBD::Signal3 CreateBinding; - static PBD::Signal1 DeleteBinding; static PBD::Signal1 StartLearning; static PBD::Signal1 StopLearning; diff --git a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc index 214c9fede7..931ef5aff2 100644 --- a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc +++ b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc @@ -108,8 +108,6 @@ GenericMidiControlProtocol::GenericMidiControlProtocol (Session& s) Controllable::StartLearning.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::start_learning, this, _1)); Controllable::StopLearning.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::stop_learning, this, _1)); - Controllable::CreateBinding.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::create_binding, this, _1, _2, _3)); - Controllable::DeleteBinding.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::delete_binding, this, _1)); /* this signal is emitted by the process() callback, and if * send_feedback() is going to do anything, it should do it in the @@ -468,64 +466,6 @@ GenericMidiControlProtocol::stop_learning (Controllable* c) delete dptr; } -void -GenericMidiControlProtocol::delete_binding (PBD::Controllable* control) -{ - if (control != 0) { - Glib::Threads::Mutex::Lock lm2 (controllables_lock); - - for (MIDIControllables::iterator iter = controllables.begin(); iter != controllables.end();) { - MIDIControllable* existingBinding = (*iter); - - if (control == (existingBinding->get_controllable())) { - delete existingBinding; - iter = controllables.erase (iter); - } else { - ++iter; - } - - } - } -} - -// This next function seems unused -void -GenericMidiControlProtocol::create_binding (PBD::Controllable* control, int pos, int control_number) -{ - if (control != NULL) { - Glib::Threads::Mutex::Lock lm2 (controllables_lock); - - MIDI::channel_t channel = (pos & 0xf); - MIDI::byte value = control_number; - - // Create a MIDIControllable - MIDIControllable* mc = new MIDIControllable (this, *_input_port->parser(), *control, false); - - // Remove any old binding for this midi channel/type/value pair - // Note: can't use delete_binding() here because we don't know the specific controllable we want to remove, only the midi information - for (MIDIControllables::iterator iter = controllables.begin(); iter != controllables.end();) { - MIDIControllable* existingBinding = (*iter); - - if ((existingBinding->get_control_channel() & 0xf ) == channel && - existingBinding->get_control_additional() == value && - (existingBinding->get_control_type() & 0xf0 ) == MIDI::controller) { - - delete existingBinding; - iter = controllables.erase (iter); - } else { - ++iter; - } - - } - - // Update the MIDI Controllable based on the the pos param - // Here is where a table lookup for user mappings could go; for now we'll just wing it... - mc->bind_midi(channel, MIDI::controller, value); - DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Create binding: Channel: %1 Controller: %2 Value: %3 \n", channel, MIDI::controller, value)); - controllables.push_back (mc); - } -} - void GenericMidiControlProtocol::check_used_event (int pos, int control_number) { @@ -537,7 +477,6 @@ GenericMidiControlProtocol::check_used_event (int pos, int control_number) DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("checking for used event: Channel: %1 Controller: %2 value: %3\n", (int) channel, (pos & 0xf0), (int) value)); // Remove any old binding for this midi channel/type/value pair - // Note: can't use delete_binding() here because we don't know the specific controllable we want to remove, only the midi information for (MIDIControllables::iterator iter = controllables.begin(); iter != controllables.end();) { MIDIControllable* existingBinding = (*iter); if ( (existingBinding->get_control_type() & 0xf0 ) == (pos & 0xf0) && (existingBinding->get_control_channel() & 0xf ) == channel ) { diff --git a/libs/surfaces/generic_midi/generic_midi_control_protocol.h b/libs/surfaces/generic_midi/generic_midi_control_protocol.h index da4bb619a3..48e7f12961 100644 --- a/libs/surfaces/generic_midi/generic_midi_control_protocol.h +++ b/libs/surfaces/generic_midi/generic_midi_control_protocol.h @@ -152,9 +152,6 @@ class GenericMidiControlProtocol : public ARDOUR::ControlProtocol { void learning_stopped (MIDIControllable*); - void create_binding (PBD::Controllable*, int, int); - void delete_binding (PBD::Controllable*); - MIDIControllable* create_binding (const XMLNode&); MIDIFunction* create_function (const XMLNode&); MIDIAction* create_action (const XMLNode&); From baed14c17e1ff27887afdb4e661bc877081d1e16 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 01:32:31 +0100 Subject: [PATCH 02/16] Prepare PBD::Controllable API cleanup (remove only registry user) --- libs/surfaces/generic_midi/generic_midi_control_protocol.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc index 931ef5aff2..6bc2fc813a 100644 --- a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc +++ b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc @@ -106,8 +106,10 @@ GenericMidiControlProtocol::GenericMidiControlProtocol (Session& s) * thread */ - Controllable::StartLearning.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::start_learning, this, _1)); +#if 0 // XXX temp. disabled for API change +Controllab:le::StartLearning.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::start_learning, this, _1)); Controllable::StopLearning.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::stop_learning, this, _1)); +#endif /* this signal is emitted by the process() callback, and if * send_feedback() is going to do anything, it should do it in the @@ -625,7 +627,7 @@ GenericMidiControlProtocol::set_state (const XMLNode& node, int version) if ((*niter)->get_property ("id", id)) { DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Relearned binding for session: Control ID: %1\n", id.to_s())); - Controllable* c = Controllable::by_id (id); + Controllable* c = 0; // Controllable::by_id (id); // XXX temp. disabled for API change if (c) { MIDIControllable* mc = new MIDIControllable (this, *_input_port->parser(), *c, false); From 1dedadd03f83663ecda2ec0a49c16dbec8d6bd18 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 01:37:28 +0100 Subject: [PATCH 03/16] Remove c-pointer Controllable* registry --- libs/pbd/controllable.cc | 63 +------------------------------------ libs/pbd/pbd/controllable.h | 9 ------ 2 files changed, 1 insertion(+), 71 deletions(-) diff --git a/libs/pbd/controllable.cc b/libs/pbd/controllable.cc index fa25826294..69a58b4d0e 100644 --- a/libs/pbd/controllable.cc +++ b/libs/pbd/controllable.cc @@ -34,74 +34,13 @@ PBD::Signal1 Controllable::StartLearning; PBD::Signal1 Controllable::StopLearning; PBD::Signal1 > Controllable::GUIFocusChanged; -Glib::Threads::RWLock Controllable::registry_lock; -Controllable::Controllables Controllable::registry; -PBD::ScopedConnectionList* registry_connections = 0; const std::string Controllable::xml_node_name = X_("Controllable"); Controllable::Controllable (const string& name, Flag f) : _name (name) , _flags (f) , _touching (false) -{ - add (*this); -} - -void -Controllable::add (Controllable& ctl) -{ - using namespace boost; - - Glib::Threads::RWLock::WriterLock lm (registry_lock); - registry.insert (&ctl); - - if (!registry_connections) { - registry_connections = new ScopedConnectionList; - } - - /* Controllable::remove() is static - no need to manage this connection */ - - ctl.DropReferences.connect_same_thread (*registry_connections, boost::bind (&Controllable::remove, &ctl)); -} - -void -Controllable::remove (Controllable* ctl) -{ - Glib::Threads::RWLock::WriterLock lm (registry_lock); - - for (Controllables::iterator i = registry.begin(); i != registry.end(); ++i) { - if ((*i) == ctl) { - registry.erase (i); - break; - } - } -} - -Controllable* -Controllable::by_id (const ID& id) -{ - Glib::Threads::RWLock::ReaderLock lm (registry_lock); - - for (Controllables::iterator i = registry.begin(); i != registry.end(); ++i) { - if ((*i)->id() == id) { - return (*i); - } - } - return 0; -} - -Controllable* -Controllable::by_name (const string& str) -{ - Glib::Threads::RWLock::ReaderLock lm (registry_lock); - - for (Controllables::iterator i = registry.begin(); i != registry.end(); ++i) { - if ((*i)->_name == str) { - return (*i); - } - } - return 0; -} +{} XMLNode& Controllable::get_state () diff --git a/libs/pbd/pbd/controllable.h b/libs/pbd/pbd/controllable.h index d40530c7e2..dbbea5dc5f 100644 --- a/libs/pbd/pbd/controllable.h +++ b/libs/pbd/pbd/controllable.h @@ -150,8 +150,6 @@ public: Flag flags() const { return _flags; } void set_flags (Flag f); - static Controllable* by_id (const PBD::ID&); - static Controllable* by_name (const std::string&); static const std::string xml_node_name; protected: @@ -167,13 +165,6 @@ private: std::string _units; Flag _flags; bool _touching; - - static void add (Controllable&); - static void remove (Controllable*); - - typedef std::set Controllables; - static Glib::Threads::RWLock registry_lock; - static Controllables registry; }; /* a utility class for the occasions when you need but do not have From 16fe286ed97e89a6768e0eb1e983ab55cc396eaf Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 01:45:29 +0100 Subject: [PATCH 04/16] Use weak-pointer for Controllable learning --- libs/pbd/controllable.cc | 4 ++-- libs/pbd/pbd/controllable.h | 4 ++-- libs/widgets/binding_proxy.cc | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libs/pbd/controllable.cc b/libs/pbd/controllable.cc index 69a58b4d0e..de1dab71a3 100644 --- a/libs/pbd/controllable.cc +++ b/libs/pbd/controllable.cc @@ -30,8 +30,8 @@ using namespace PBD; using namespace std; PBD::Signal1 Controllable::Destroyed; -PBD::Signal1 Controllable::StartLearning; -PBD::Signal1 Controllable::StopLearning; +PBD::Signal1 > Controllable::StartLearning; +PBD::Signal1 > Controllable::StopLearning; PBD::Signal1 > Controllable::GUIFocusChanged; const std::string Controllable::xml_node_name = X_("Controllable"); diff --git a/libs/pbd/pbd/controllable.h b/libs/pbd/pbd/controllable.h index dbbea5dc5f..1cc4cdf083 100644 --- a/libs/pbd/pbd/controllable.h +++ b/libs/pbd/pbd/controllable.h @@ -123,8 +123,8 @@ public: PBD::Signal0 LearningFinished; - static PBD::Signal1 StartLearning; - static PBD::Signal1 StopLearning; + static PBD::Signal1 > StartLearning; + static PBD::Signal1 > StopLearning; static PBD::Signal1 Destroyed; diff --git a/libs/widgets/binding_proxy.cc b/libs/widgets/binding_proxy.cc index 85119ab1c0..98f4443ad2 100644 --- a/libs/widgets/binding_proxy.cc +++ b/libs/widgets/binding_proxy.cc @@ -91,7 +91,7 @@ bool BindingProxy::button_press_handler (GdkEventButton *ev) { if ( controllable && is_bind_action(ev) ) { - if (Controllable::StartLearning (controllable.get())) { + if (Controllable::StartLearning (controllable)) { string prompt = _("operate controller now"); if (prompter == 0) { prompter = new PopUp (Gtk::WIN_POS_MOUSE, 30000, false); @@ -121,7 +121,7 @@ BindingProxy::prompter_hiding (GdkEventAny* /*ev*/) { learning_connection.disconnect (); if (controllable) { - Controllable::StopLearning (controllable.get()); + Controllable::StopLearning (controllable); } return false; } From e9b36f2beab7d7c22d321291e5cfe39f5b0a4349 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 02:09:39 +0100 Subject: [PATCH 05/16] Update GenericMidiControlProtocol to use shared/weak Controllable pointers This fixes a race-condition when a controllable is deleted while sending feedback to the device. Previously there was a race-condition MIDIControllable::write_feedback() triggered from rt-thread, processed in Surface-thread and deleting a route or processor. This is a first step, currently state-restore is not fully functional session->controllable_by_id() does not cover all Controllables. --- .../generic_midi_control_protocol.cc | 28 +++--- .../generic_midi_control_protocol.h | 10 +-- .../surfaces/generic_midi/midicontrollable.cc | 87 +++++++++++++------ libs/surfaces/generic_midi/midicontrollable.h | 19 ++-- 4 files changed, 92 insertions(+), 52 deletions(-) diff --git a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc index 6bc2fc813a..be835b7ffb 100644 --- a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc +++ b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc @@ -106,10 +106,8 @@ GenericMidiControlProtocol::GenericMidiControlProtocol (Session& s) * thread */ -#if 0 // XXX temp. disabled for API change -Controllab:le::StartLearning.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::start_learning, this, _1)); - Controllable::StopLearning.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::stop_learning, this, _1)); -#endif + Controllable::StartLearning.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::start_learning, this, _1)); + Controllable::StopLearning.connect_same_thread (*this, boost::bind (&GenericMidiControlProtocol::stop_learning, this, _1)); /* this signal is emitted by the process() callback, and if * send_feedback() is going to do anything, it should do it in the @@ -345,9 +343,10 @@ GenericMidiControlProtocol::_send_feedback () } bool -GenericMidiControlProtocol::start_learning (Controllable* c) +GenericMidiControlProtocol::start_learning (boost::weak_ptr wc) { - if (c == 0) { + boost::shared_ptr c = wc.lock (); + if (!c) { return false; } @@ -401,7 +400,7 @@ GenericMidiControlProtocol::start_learning (Controllable* c) } if (!mc) { - mc = new MIDIControllable (this, *_input_port->parser(), *c, false); + mc = new MIDIControllable (this, *_input_port->parser(), c, false); own_mc = true; } @@ -443,8 +442,13 @@ GenericMidiControlProtocol::learning_stopped (MIDIControllable* mc) } void -GenericMidiControlProtocol::stop_learning (Controllable* c) +GenericMidiControlProtocol::stop_learning (boost::weak_ptr wc) { + boost::shared_ptr c = wc.lock (); + if (!c) { + return; + } + Glib::Threads::Mutex::Lock lm (pending_lock); Glib::Threads::Mutex::Lock lm2 (controllables_lock); MIDIControllable* dptr = 0; @@ -627,10 +631,10 @@ GenericMidiControlProtocol::set_state (const XMLNode& node, int version) if ((*niter)->get_property ("id", id)) { DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Relearned binding for session: Control ID: %1\n", id.to_s())); - Controllable* c = 0; // Controllable::by_id (id); // XXX temp. disabled for API change + boost::shared_ptr c = session->controllable_by_id (id); // XXX are these all? if (c) { - MIDIControllable* mc = new MIDIControllable (this, *_input_port->parser(), *c, false); + MIDIControllable* mc = new MIDIControllable (this, *_input_port->parser(), c, false); if (mc->set_state (**niter, version) == 0) { controllables.push_back (mc); @@ -1503,9 +1507,9 @@ GenericMidiControlProtocol::input_port() const } void -GenericMidiControlProtocol::maybe_start_touch (Controllable* controllable) +GenericMidiControlProtocol::maybe_start_touch (boost::shared_ptr controllable) { - AutomationControl *actl = dynamic_cast (controllable); + boost::shared_ptr actl = boost::dynamic_pointer_cast (controllable); if (actl) { actl->start_touch (session->audible_sample ()); } diff --git a/libs/surfaces/generic_midi/generic_midi_control_protocol.h b/libs/surfaces/generic_midi/generic_midi_control_protocol.h index 48e7f12961..c198969ca0 100644 --- a/libs/surfaces/generic_midi/generic_midi_control_protocol.h +++ b/libs/surfaces/generic_midi/generic_midi_control_protocol.h @@ -47,7 +47,7 @@ class MIDIFunction; class MIDIAction; class GenericMidiControlProtocol : public ARDOUR::ControlProtocol { - public: +public: GenericMidiControlProtocol (ARDOUR::Session&); virtual ~GenericMidiControlProtocol(); @@ -68,7 +68,7 @@ class GenericMidiControlProtocol : public ARDOUR::ControlProtocol { boost::shared_ptr lookup_controllable (std::string const &) const; - void maybe_start_touch (PBD::Controllable*); + void maybe_start_touch (boost::shared_ptr); XMLNode& get_state (); int set_state (const XMLNode&, int version); @@ -110,7 +110,7 @@ class GenericMidiControlProtocol : public ARDOUR::ControlProtocol { PBD::Signal0 ConnectionChange; - private: +private: boost::shared_ptr _input_bundle; boost::shared_ptr _output_bundle; boost::shared_ptr _input_port; @@ -147,8 +147,8 @@ class GenericMidiControlProtocol : public ARDOUR::ControlProtocol { Glib::Threads::Mutex controllables_lock; Glib::Threads::Mutex pending_lock; - bool start_learning (PBD::Controllable*); - void stop_learning (PBD::Controllable*); + bool start_learning (boost::weak_ptr); + void stop_learning (boost::weak_ptr); void learning_stopped (MIDIControllable*); diff --git a/libs/surfaces/generic_midi/midicontrollable.cc b/libs/surfaces/generic_midi/midicontrollable.cc index cbf525bd1b..a33855a0f4 100644 --- a/libs/surfaces/generic_midi/midicontrollable.cc +++ b/libs/surfaces/generic_midi/midicontrollable.cc @@ -47,7 +47,6 @@ using namespace ARDOUR; MIDIControllable::MIDIControllable (GenericMidiControlProtocol* s, MIDI::Parser& p, bool m) : _surface (s) - , controllable (0) , _parser (p) , _momentary (m) { @@ -65,12 +64,12 @@ MIDIControllable::MIDIControllable (GenericMidiControlProtocol* s, MIDI::Parser& control_additional = (MIDI::byte) -1; } -MIDIControllable::MIDIControllable (GenericMidiControlProtocol* s, MIDI::Parser& p, Controllable& c, bool m) +MIDIControllable::MIDIControllable (GenericMidiControlProtocol* s, MIDI::Parser& p, boost::shared_ptr c, bool m) : _surface (s) , _parser (p) , _momentary (m) { - set_controllable (&c); + set_controllable (c); _learned = true; /* from controllable */ _ctltype = Ctl_Momentary; @@ -119,29 +118,35 @@ MIDIControllable::drop_external_control () control_additional = (MIDI::byte) -1; } -void -MIDIControllable::set_controllable (Controllable* c) +boost::shared_ptr +MIDIControllable::get_controllable () const { - if (c == controllable) { + return _controllable.lock (); +} + +void +MIDIControllable::set_controllable (boost::shared_ptr c) +{ + if (c == _controllable.lock()) { return; } controllable_death_connection.disconnect (); - controllable = c; - - if (controllable) { - last_controllable_value = controllable->get_value(); + if (c) { + _controllable = boost::weak_ptr (c); + last_controllable_value = c->get_value(); } else { + _controllable.reset(); last_controllable_value = 0.0f; // is there a better value? } last_incoming = 256; - if (controllable) { - controllable->Destroyed.connect (controllable_death_connection, MISSING_INVALIDATOR, - boost::bind (&MIDIControllable::drop_controllable, this, _1), - MidiControlUI::instance()); + if (c) { + printf ("MIDIControllable::set %s\n", c->name().c_str()); + c->Destroyed.connect_same_thread (controllable_death_connection, + boost::bind (&MIDIControllable::drop_controllable, this, _1)); } } @@ -171,6 +176,11 @@ MIDIControllable::stop_learning () int MIDIControllable::control_to_midi (float val) { + boost::shared_ptr controllable = _controllable.lock (); + if (!controllable) { + return 0; + } + if (controllable->is_gain_like()) { return controllable->internal_to_interface (val) * max_value_for_type (); } @@ -186,7 +196,7 @@ MIDIControllable::control_to_midi (float val) return 0; } } else { - AutomationControl *actl = dynamic_cast (controllable); + boost::shared_ptr actl = boost::dynamic_pointer_cast (controllable); if (actl) { control_min = actl->internal_to_interface(control_min); control_max = actl->internal_to_interface(control_max); @@ -202,6 +212,10 @@ MIDIControllable::control_to_midi (float val) float MIDIControllable::midi_to_control (int val) { + boost::shared_ptr controllable = _controllable.lock (); + if (!controllable) { + return 0; + } /* fiddle with MIDI value so that we get an odd number of integer steps and can thus represent "middle" precisely as 0.5. this maps to the range 0..+1.0 (0 to 126) @@ -219,7 +233,7 @@ MIDIControllable::midi_to_control (int val) float control_range = control_max - control_min; DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Min %1 Max %2 Range %3\n", control_min, control_max, control_range)); - AutomationControl *actl = dynamic_cast (controllable); + boost::shared_ptr actl = boost::dynamic_pointer_cast (controllable); if (actl) { if (fv == 0.f) return control_min; if (fv == 1.f) return control_max; @@ -256,7 +270,7 @@ MIDIControllable::lookup_controllable() return -1; } - set_controllable (c.get ()); + set_controllable (c); return 0; } @@ -264,20 +278,26 @@ MIDIControllable::lookup_controllable() void MIDIControllable::drop_controllable (Controllable* c) { - if (c == controllable) { - set_controllable (0); + printf ("MIDIControllable::drop_controllable ? %s\n", c->name().c_str()); + + boost::shared_ptr controllable = _controllable.lock (); + if (controllable && c == controllable.get()) { + set_controllable (boost::shared_ptr()); } } void MIDIControllable::midi_sense_note (Parser &, EventTwoBytes *msg, bool /*is_on*/) { - if (!controllable) { + if (_controllable.expired ()) { if (lookup_controllable()) { return; } } + boost::shared_ptr controllable = _controllable.lock (); + assert (controllable); + _surface->maybe_start_touch (controllable); if (!controllable->is_toggle()) { @@ -299,12 +319,13 @@ MIDIControllable::midi_sense_note (Parser &, EventTwoBytes *msg, bool /*is_on*/) void MIDIControllable::midi_sense_controller (Parser &, EventTwoBytes *msg) { - if (!controllable) { + if (_controllable.expired ()) { if (lookup_controllable ()) { return; } } + boost::shared_ptr controllable = _controllable.lock (); assert (controllable); _surface->maybe_start_touch (controllable); @@ -420,12 +441,15 @@ MIDIControllable::midi_sense_controller (Parser &, EventTwoBytes *msg) void MIDIControllable::midi_sense_program_change (Parser &, MIDI::byte msg) { - if (!controllable) { + if (_controllable.expired ()) { if (lookup_controllable ()) { return; } } + boost::shared_ptr controllable = _controllable.lock (); + assert (controllable); + _surface->maybe_start_touch (controllable); if (msg == control_additional) { @@ -446,12 +470,15 @@ MIDIControllable::midi_sense_program_change (Parser &, MIDI::byte msg) void MIDIControllable::midi_sense_pitchbend (Parser &, pitchbend_t pb) { - if (!controllable) { + if (_controllable.expired ()) { if (lookup_controllable ()) { return; } } + boost::shared_ptr controllable = _controllable.lock (); + assert (controllable); + _surface->maybe_start_touch (controllable); if (!controllable->is_toggle()) { @@ -482,6 +509,7 @@ MIDIControllable::midi_receiver (Parser &, MIDI::byte *msg, size_t /*len*/) _surface->check_used_event(msg[0], msg[1]); bind_midi ((channel_t) (msg[0] & 0xf), eventType (msg[0] & 0xF0), msg[1]); + boost::shared_ptr controllable = _controllable.lock (); if (controllable) { controllable->LearningFinished (); } @@ -491,6 +519,7 @@ void MIDIControllable::rpn_value_change (Parser&, uint16_t rpn, float val) { if (control_rpn == rpn) { + boost::shared_ptr controllable = _controllable.lock (); if (controllable) { controllable->set_value (val, Controllable::UseGroup); } @@ -501,6 +530,7 @@ void MIDIControllable::nrpn_value_change (Parser&, uint16_t nrpn, float val) { if (control_nrpn == nrpn) { + boost::shared_ptr controllable = _controllable.lock (); if (controllable) { controllable->set_value (val, Controllable::UseGroup); } @@ -511,6 +541,7 @@ void MIDIControllable::rpn_change (Parser&, uint16_t rpn, int dir) { if (control_rpn == rpn) { + boost::shared_ptr controllable = _controllable.lock (); if (controllable) { /* XXX how to increment/decrement ? */ // controllable->set_value (val); @@ -522,6 +553,7 @@ void MIDIControllable::nrpn_change (Parser&, uint16_t nrpn, int dir) { if (control_nrpn == nrpn) { + boost::shared_ptr controllable = _controllable.lock (); if (controllable) { /* XXX how to increment/decrement ? */ // controllable->set_value (val); @@ -629,10 +661,13 @@ MIDIControllable::bind_midi (channel_t chn, eventType ev, MIDI::byte additional) MIDI::byte* MIDIControllable::write_feedback (MIDI::byte* buf, int32_t& bufsize, bool /*force*/) { - if (!controllable || !_surface->get_feedback ()) { + if (_controllable.expired () || !_surface->get_feedback ()) { return buf; } + boost::shared_ptr controllable = _controllable.lock (); + assert (controllable); + float val = controllable->get_value (); /* Note that when sending RPN/NPRN we do two things: @@ -768,7 +803,9 @@ MIDIControllable::get_state () XMLNode* node = new XMLNode ("MIDIControllable"); - if (_current_uri.empty()) { + boost::shared_ptr controllable = _controllable.lock (); + + if (_current_uri.empty() && controllable) { node->set_property ("id", controllable->id ()); } else { node->set_property ("uri", _current_uri); diff --git a/libs/surfaces/generic_midi/midicontrollable.h b/libs/surfaces/generic_midi/midicontrollable.h index b795067a61..a3f4eaabc2 100644 --- a/libs/surfaces/generic_midi/midicontrollable.h +++ b/libs/surfaces/generic_midi/midicontrollable.h @@ -43,9 +43,9 @@ namespace ARDOUR { class MIDIControllable : public PBD::Stateful { - public: - MIDIControllable (GenericMidiControlProtocol *, MIDI::Parser&, PBD::Controllable&, bool momentary); - MIDIControllable (GenericMidiControlProtocol *, MIDI::Parser&, bool momentary = false); +public: + MIDIControllable (GenericMidiControlProtocol*, MIDI::Parser&, boost::shared_ptr, bool momentary); + MIDIControllable (GenericMidiControlProtocol*, MIDI::Parser&, bool momentary = false); virtual ~MIDIControllable (); int init (const std::string&); @@ -89,8 +89,8 @@ class MIDIControllable : public PBD::Stateful void set_encoder (Encoder val) { _encoder = val; } MIDI::Parser& get_parser() { return _parser; } - PBD::Controllable* get_controllable() const { return controllable; } - void set_controllable (PBD::Controllable*); + void set_controllable (boost::shared_ptr); + boost::shared_ptr get_controllable () const; const std::string& current_uri() const { return _current_uri; } std::string control_description() const { return _control_description; } @@ -108,16 +108,16 @@ class MIDIControllable : public PBD::Stateful MIDI::eventType get_control_type () { return control_type; } MIDI::byte get_control_additional () { return control_additional; } - int lookup_controllable(); + int lookup_controllable(); - private: +private: int max_value_for_type () const; GenericMidiControlProtocol* _surface; - PBD::Controllable* controllable; + boost::weak_ptr _controllable; std::string _current_uri; - MIDI::Parser& _parser; + MIDI::Parser& _parser; bool setting; int last_value; int last_incoming; @@ -159,4 +159,3 @@ class MIDIControllable : public PBD::Stateful }; #endif // __gm_midicontrollable_h__ - From 087fd57d37b7c07989b470019870fe8d72917bb3 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 03:10:49 +0100 Subject: [PATCH 06/16] Re-add Controllable registry To facilitate a central registry with weak/shared pointer lookup, enable_shared_from_this was migrated to enable_shared_from_this The main (and only) user is generic-midi surface's state interface :( --- libs/ardour/ardour/automation_control.h | 2 -- libs/ardour/automation_control.cc | 16 +++++----- libs/ardour/pan_controllable.cc | 2 +- libs/pbd/controllable.cc | 39 ++++++++++++++++++++++++- libs/pbd/pbd/controllable.h | 18 ++++++++++-- 5 files changed, 63 insertions(+), 14 deletions(-) diff --git a/libs/ardour/ardour/automation_control.h b/libs/ardour/ardour/automation_control.h index 8de0ec6ec0..80cbf1c2f5 100644 --- a/libs/ardour/ardour/automation_control.h +++ b/libs/ardour/ardour/automation_control.h @@ -26,7 +26,6 @@ #include #include -#include #include "pbd/controllable.h" @@ -51,7 +50,6 @@ class ControlGroup; class LIBARDOUR_API AutomationControl : public PBD::Controllable , public Evoral::Control - , public boost::enable_shared_from_this , public ControlGroupMember , public SessionHandleRef { diff --git a/libs/ardour/automation_control.cc b/libs/ardour/automation_control.cc index 5755e04aac..a194e2858a 100644 --- a/libs/ardour/automation_control.cc +++ b/libs/ardour/automation_control.cc @@ -141,7 +141,7 @@ AutomationControl::set_value (double val, PBD::Controllable::GroupControlDisposi } if (_group && _group->use_me (gcd)) { - _group->set_group_value (shared_from_this(), val); + _group->set_group_value (boost::dynamic_pointer_cast(shared_from_this()), val); } else { actually_set_value (val, gcd); } @@ -252,7 +252,7 @@ AutomationControl::set_automation_state (AutoState as) } if (as == Write) { - AutomationWatch::instance().add_automation_watch (shared_from_this()); + AutomationWatch::instance().add_automation_watch (boost::dynamic_pointer_cast(shared_from_this())); } else if (as & (Touch | Latch)) { if (alist()->empty()) { Control::set_double (val, _session.current_start_sample (), true); @@ -260,17 +260,17 @@ AutomationControl::set_automation_state (AutoState as) Changed (true, Controllable::NoGroup); } if (!touching()) { - AutomationWatch::instance().remove_automation_watch (shared_from_this()); + AutomationWatch::instance().remove_automation_watch (boost::dynamic_pointer_cast(shared_from_this())); } else { /* this seems unlikely, but the combination of * a control surface and the mouse could make * it possible to put the control into Touch * mode *while* touching it. */ - AutomationWatch::instance().add_automation_watch (shared_from_this()); + AutomationWatch::instance().add_automation_watch (boost::dynamic_pointer_cast(shared_from_this())); } } else { - AutomationWatch::instance().remove_automation_watch (shared_from_this()); + AutomationWatch::instance().remove_automation_watch (boost::dynamic_pointer_cast(shared_from_this())); Changed (false, Controllable::NoGroup); } } @@ -293,7 +293,7 @@ AutomationControl::start_touch (double when) AutomationControl::actually_set_value (get_value (), Controllable::NoGroup); alist()->start_touch (when); if (!_desc.toggled) { - AutomationWatch::instance().add_automation_watch (shared_from_this()); + AutomationWatch::instance().add_automation_watch (boost::dynamic_pointer_cast(shared_from_this())); } set_touching (true); } @@ -315,7 +315,7 @@ AutomationControl::stop_touch (double when) if (alist()->automation_state() & (Touch | Latch)) { alist()->stop_touch (when); if (!_desc.toggled) { - AutomationWatch::instance().remove_automation_watch (shared_from_this()); + AutomationWatch::instance().remove_automation_watch (boost::dynamic_pointer_cast(shared_from_this())); } } } @@ -378,7 +378,7 @@ AutomationControl::check_rt (double val, Controllable::GroupControlDisposition g { if (!_session.loading() && (flags() & Controllable::RealTime) && !AudioEngine::instance()->in_process_thread()) { /* queue change in RT context */ - _session.set_control (shared_from_this(), val, gcd); + _session.set_control (boost::dynamic_pointer_cast(shared_from_this()), val, gcd); return true; } diff --git a/libs/ardour/pan_controllable.cc b/libs/ardour/pan_controllable.cc index 35354102e9..9f558898b4 100644 --- a/libs/ardour/pan_controllable.cc +++ b/libs/ardour/pan_controllable.cc @@ -57,5 +57,5 @@ PanControllable::actually_set_value (double v, Controllable::GroupControlDisposi std::string PanControllable::get_user_string () const { - return owner->value_as_string (shared_from_this()); + return owner->value_as_string (boost::dynamic_pointer_cast(shared_from_this())); } diff --git a/libs/pbd/controllable.cc b/libs/pbd/controllable.cc index de1dab71a3..67268c1db2 100644 --- a/libs/pbd/controllable.cc +++ b/libs/pbd/controllable.cc @@ -34,13 +34,19 @@ PBD::Signal1 > Controllable::StartLearn PBD::Signal1 > Controllable::StopLearning; PBD::Signal1 > Controllable::GUIFocusChanged; +Glib::Threads::RWLock Controllable::registry_lock; +Controllable::Controllables Controllable::registry; +PBD::ScopedConnectionList Controllable::registry_connections; + const std::string Controllable::xml_node_name = X_("Controllable"); Controllable::Controllable (const string& name, Flag f) : _name (name) , _flags (f) , _touching (false) -{} +{ + add (*this); +} XMLNode& Controllable::get_state () @@ -91,3 +97,34 @@ Controllable::set_flags (Flag f) { _flags = f; } + +void +Controllable::add (Controllable& ctl) +{ + Glib::Threads::RWLock::WriterLock lm (registry_lock); + registry.insert (&ctl); + ctl.DropReferences.connect_same_thread (registry_connections, boost::bind (&Controllable::remove, &ctl)); +} + +void +Controllable::remove (Controllable* ctl) +{ + Glib::Threads::RWLock::WriterLock lm (registry_lock); + Controllables::iterator i = std::find (registry.begin(), registry.end(), ctl); + if (i != registry.end()) { + registry.erase (i); + } +} + +boost::shared_ptr +Controllable::by_id (const ID& id) +{ + Glib::Threads::RWLock::ReaderLock lm (registry_lock); + + for (Controllables::iterator i = registry.begin(); i != registry.end(); ++i) { + if ((*i)->id() == id) { + return (*i)->shared_from_this (); + } + } + return boost::shared_ptr(); +} diff --git a/libs/pbd/pbd/controllable.h b/libs/pbd/pbd/controllable.h index 1cc4cdf083..6eeb9956da 100644 --- a/libs/pbd/pbd/controllable.h +++ b/libs/pbd/pbd/controllable.h @@ -28,6 +28,8 @@ #include "pbd/signals.h" #include +#include + #include "pbd/statefuldestructible.h" using std::min; @@ -49,7 +51,8 @@ namespace PBD { * as a control whose value can range between 0 and 1.0. * */ -class LIBPBD_API Controllable : public PBD::StatefulDestructible { +class LIBPBD_API Controllable : public PBD::StatefulDestructible, public boost::enable_shared_from_this +{ public: enum Flag { Toggle = 0x1, @@ -150,6 +153,8 @@ public: Flag flags() const { return _flags; } void set_flags (Flag f); + static boost::shared_ptr by_id (const PBD::ID&); + static const std::string xml_node_name; protected: @@ -160,11 +165,20 @@ protected: } private: - std::string _name; std::string _units; Flag _flags; bool _touching; + + typedef std::set Controllables; + + static ScopedConnectionList registry_connections; + static Glib::Threads::RWLock registry_lock; + static Controllables registry; + + static void add (Controllable&); + static void remove (Controllable*); + }; /* a utility class for the occasions when you need but do not have From 73029d45baf97da6b0a2c8ec9688c33da69fff8d Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 03:11:54 +0100 Subject: [PATCH 07/16] Re-add global lookup for generic-midi ctrl state --- libs/surfaces/generic_midi/generic_midi_control_protocol.cc | 2 +- libs/surfaces/generic_midi/midicontrollable.cc | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc index be835b7ffb..44b9351554 100644 --- a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc +++ b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc @@ -631,7 +631,7 @@ GenericMidiControlProtocol::set_state (const XMLNode& node, int version) if ((*niter)->get_property ("id", id)) { DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Relearned binding for session: Control ID: %1\n", id.to_s())); - boost::shared_ptr c = session->controllable_by_id (id); // XXX are these all? + boost::shared_ptr c = Controllable::by_id (id); if (c) { MIDIControllable* mc = new MIDIControllable (this, *_input_port->parser(), c, false); diff --git a/libs/surfaces/generic_midi/midicontrollable.cc b/libs/surfaces/generic_midi/midicontrollable.cc index a33855a0f4..a110f32904 100644 --- a/libs/surfaces/generic_midi/midicontrollable.cc +++ b/libs/surfaces/generic_midi/midicontrollable.cc @@ -144,7 +144,6 @@ MIDIControllable::set_controllable (boost::shared_ptr c) last_incoming = 256; if (c) { - printf ("MIDIControllable::set %s\n", c->name().c_str()); c->Destroyed.connect_same_thread (controllable_death_connection, boost::bind (&MIDIControllable::drop_controllable, this, _1)); } @@ -278,8 +277,6 @@ MIDIControllable::lookup_controllable() void MIDIControllable::drop_controllable (Controllable* c) { - printf ("MIDIControllable::drop_controllable ? %s\n", c->name().c_str()); - boost::shared_ptr controllable = _controllable.lock (); if (controllable && c == controllable.get()) { set_controllable (boost::shared_ptr()); From ff8bd935cf27a833f5611d2123230893a9862c4f Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 14:28:12 +0100 Subject: [PATCH 08/16] Remove chicken/egg d'tor Session::Controllables is a shared_ptr<> list. As long as the session exists the Controllables will be around. Destroyed(*) can only be called after the session is destroyed and releases the shared_ptr<> NB. this code had a nice hack to construct a "shared_from_this" workaround. For future reference: struct null_deleter { void operator()(void const *) const {} }; boost::shared_ptr(c, null_deleter()) --- libs/ardour/ardour/session.h | 1 - libs/ardour/session_state.cc | 19 ------------------- 2 files changed, 20 deletions(-) diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h index ab7b59c5c6..265e197d00 100644 --- a/libs/ardour/ardour/session.h +++ b/libs/ardour/ardour/session.h @@ -1079,7 +1079,6 @@ public: boost::shared_ptr automation_control_by_id (const PBD::ID&); void add_controllable (boost::shared_ptr); - void remove_controllable (PBD::Controllable*); boost::shared_ptr solo_cut_control() const; diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc index 6e791b4d8a..9ed5a27269 100644 --- a/libs/ardour/session_state.cc +++ b/libs/ardour/session_state.cc @@ -208,7 +208,6 @@ Session::pre_engine_init (string fullpath) SourceFactory::SourceCreated.connect_same_thread (*this, boost::bind (&Session::add_source, this, _1)); PlaylistFactory::PlaylistCreated.connect_same_thread (*this, boost::bind (&Session::add_playlist, this, _1, _2)); AutomationList::AutomationListCreated.connect_same_thread (*this, boost::bind (&Session::add_automation_list, this, _1)); - Controllable::Destroyed.connect_same_thread (*this, boost::bind (&Session::remove_controllable, this, _1)); IO::PortCountChanged.connect_same_thread (*this, boost::bind (&Session::ensure_buffers, this, _1)); /* stop IO objects from doing stuff until we're ready for them */ @@ -3739,24 +3738,6 @@ Session::add_controllable (boost::shared_ptr c) controllables.insert (c); } -struct null_deleter { void operator()(void const *) const {} }; - -void -Session::remove_controllable (Controllable* c) -{ - if (deletion_in_progress()) { - return; - } - - Glib::Threads::Mutex::Lock lm (controllables_lock); - - Controllables::iterator x = controllables.find (boost::shared_ptr(c, null_deleter())); - - if (x != controllables.end()) { - controllables.erase (x); - } -} - boost::shared_ptr Session::controllable_by_id (const PBD::ID& id) { From da114c5a4db20cf91c8092284f290e8b90d078b0 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 14:32:00 +0100 Subject: [PATCH 09/16] Remove static Destroyed(*), prefer StatefulDestructable API This also add a debug-dump method to show remaining registered Controllables. --- libs/pbd/controllable.cc | 14 +++++++++++++- libs/pbd/pbd/controllable.h | 8 ++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/libs/pbd/controllable.cc b/libs/pbd/controllable.cc index 67268c1db2..9572bade6e 100644 --- a/libs/pbd/controllable.cc +++ b/libs/pbd/controllable.cc @@ -29,7 +29,6 @@ using namespace PBD; using namespace std; -PBD::Signal1 Controllable::Destroyed; PBD::Signal1 > Controllable::StartLearning; PBD::Signal1 > Controllable::StopLearning; PBD::Signal1 > Controllable::GUIFocusChanged; @@ -104,6 +103,7 @@ Controllable::add (Controllable& ctl) Glib::Threads::RWLock::WriterLock lm (registry_lock); registry.insert (&ctl); ctl.DropReferences.connect_same_thread (registry_connections, boost::bind (&Controllable::remove, &ctl)); + ctl.Destroyed.connect_same_thread (registry_connections, boost::bind (&Controllable::remove, &ctl)); } void @@ -128,3 +128,15 @@ Controllable::by_id (const ID& id) } return boost::shared_ptr(); } + +void +Controllable::dump_registry () +{ + Glib::Threads::RWLock::ReaderLock lm (registry_lock); + unsigned int cnt = 0; + cout << "-- List Of Registered Controllables\n"; + for (Controllables::iterator i = registry.begin(); i != registry.end(); ++i, ++cnt) { + cout << "CTRL: " << (*i)->name() << "\n"; + } + cout << "Total number of registered sontrollables: " << cnt << "\n"; +} diff --git a/libs/pbd/pbd/controllable.h b/libs/pbd/pbd/controllable.h index 6eeb9956da..92743c4f66 100644 --- a/libs/pbd/pbd/controllable.h +++ b/libs/pbd/pbd/controllable.h @@ -22,7 +22,6 @@ #include #include -#include #include "pbd/libpbd_visibility.h" #include "pbd/signals.h" @@ -62,7 +61,6 @@ public: }; Controllable (const std::string& name, Flag f = Flag (0)); - virtual ~Controllable() { Destroyed (this); } /* We express Controllable values in one of three ways: * 1. `user' --- as presented to the user (e.g. dB, Hz, etc.) @@ -129,8 +127,6 @@ public: static PBD::Signal1 > StartLearning; static PBD::Signal1 > StopLearning; - static PBD::Signal1 Destroyed; - static PBD::Signal1 > GUIFocusChanged; PBD::Signal2 Changed; @@ -138,7 +134,7 @@ public: int set_state (const XMLNode&, int version); virtual XMLNode& get_state (); - std::string name() const { return _name; } + std::string name() const { return _name; } bool touching () const { return _touching; } PBD::Signal0 TouchChanged; @@ -154,6 +150,7 @@ public: void set_flags (Flag f); static boost::shared_ptr by_id (const PBD::ID&); + static void dump_registry (); static const std::string xml_node_name; @@ -178,7 +175,6 @@ private: static void add (Controllable&); static void remove (Controllable*); - }; /* a utility class for the occasions when you need but do not have From 96e991d08f94a7da04a9bb6fe2eb55968013759d Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 15:52:36 +0100 Subject: [PATCH 10/16] Clean out session-global controllables This isn't strictly speaking needed, there are only a handful of users (most notably generic-midi ctrl surface, and Selection) --- libs/ardour/session.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc index 2c00fbbd80..d6753c50d9 100644 --- a/libs/ardour/session.cc +++ b/libs/ardour/session.cc @@ -680,6 +680,14 @@ Session::destroy () Port::PortDrop (); /* EMIT SIGNAL */ + { + Glib::Threads::Mutex::Lock lm (controllables_lock); + for (Controllables::iterator i = controllables.begin(); i != controllables.end(); ++i) { + (*i)->DropReferences (); /* EMIT SIGNAL */ + } + controllables.clear (); + } + /* clear history so that no references to objects are held any more */ _history.clear (); @@ -864,6 +872,10 @@ Session::destroy () DEBUG_TRACE (DEBUG::Destruction, "Session::destroy() done\n"); +#ifndef NDEBUG + Controllable::dump_registry (); +#endif + BOOST_SHOW_POINTERS (); } From 1d5e5b352390282a048ee7bf97349509f25d0014 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 16:14:15 +0100 Subject: [PATCH 11/16] Clean up MonitorProcessorControls As opposed to regular AutomationControls these PBD:::Controllables are not SessionObjects and don't emit a signal when the session goes away. --- libs/ardour/ardour/monitor_processor.h | 1 + libs/ardour/monitor_processor.cc | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/libs/ardour/ardour/monitor_processor.h b/libs/ardour/ardour/monitor_processor.h index 63e153e48e..5f233d9ad0 100644 --- a/libs/ardour/ardour/monitor_processor.h +++ b/libs/ardour/ardour/monitor_processor.h @@ -192,6 +192,7 @@ private: MPControl& soloed; ChannelRecord (uint32_t); + ~ChannelRecord (); }; std::vector _channels; diff --git a/libs/ardour/monitor_processor.cc b/libs/ardour/monitor_processor.cc index 56d8f879d6..b4e996a52e 100644 --- a/libs/ardour/monitor_processor.cc +++ b/libs/ardour/monitor_processor.cc @@ -78,6 +78,13 @@ MonitorProcessor::MonitorProcessor (Session& s) MonitorProcessor::~MonitorProcessor () { allocate_channels (0); + + /* special case for MPControl */ + _dim_all_control->DropReferences (); /* EMIT SIGNAL */ + _cut_all_control->DropReferences (); /* EMIT SIGNAL */ + _mono_control->DropReferences (); /* EMIT SIGNAL */ + _dim_level_control->DropReferences (); /* EMIT SIGNAL */ + _solo_boost_level_control->DropReferences (); /* EMIT SIGNAL */ } void @@ -542,5 +549,13 @@ MonitorProcessor::ChannelRecord::ChannelRecord (uint32_t chn) , polarity (*polarity_ptr) , soloed (*soloed_ptr) { - +} + +MonitorProcessor::ChannelRecord::~ChannelRecord () +{ + /* special case for MPControl */ + cut_control->DropReferences(); /* EMIT SIGNAL */ + dim_control->DropReferences(); /* EMIT SIGNAL */ + polarity_control->DropReferences(); /* EMIT SIGNAL */ + soloed_control->DropReferences(); /* EMIT SIGNAL */ } From c97116083fd51e1d3108cabf92c890c45292fbab Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 16:32:48 +0100 Subject: [PATCH 12/16] Fix generic-midi controllable race-condition Continued work after e9b36f2bea. Prefer a shared_ptr<>. MIDIControllable::write_feedback() runs in realtime context, directly from the main process-thread. Synchronizing weak-pointers and deletion across threads does not work reliably. Retaining a shared_ptr<> for controllables that are in use can solve this. --- .../surfaces/generic_midi/midicontrollable.cc | 187 ++++++++---------- libs/surfaces/generic_midi/midicontrollable.h | 7 +- 2 files changed, 91 insertions(+), 103 deletions(-) diff --git a/libs/surfaces/generic_midi/midicontrollable.cc b/libs/surfaces/generic_midi/midicontrollable.cc index a110f32904..741cd0c781 100644 --- a/libs/surfaces/generic_midi/midicontrollable.cc +++ b/libs/surfaces/generic_midi/midicontrollable.cc @@ -121,20 +121,21 @@ MIDIControllable::drop_external_control () boost::shared_ptr MIDIControllable::get_controllable () const { - return _controllable.lock (); + return _controllable; } void MIDIControllable::set_controllable (boost::shared_ptr c) { - if (c == _controllable.lock()) { + Glib::Threads::Mutex::Lock lm (controllable_lock); + if (c && c == _controllable) { return; } - controllable_death_connection.disconnect (); + controllable_death_connections.drop_connections (); if (c) { - _controllable = boost::weak_ptr (c); + _controllable = c; last_controllable_value = c->get_value(); } else { _controllable.reset(); @@ -144,8 +145,9 @@ MIDIControllable::set_controllable (boost::shared_ptr c) last_incoming = 256; if (c) { - c->Destroyed.connect_same_thread (controllable_death_connection, - boost::bind (&MIDIControllable::drop_controllable, this, _1)); + c->DropReferences.connect (controllable_death_connections, MISSING_INVALIDATOR, + boost::bind (&MIDIControllable::drop_controllable, this), + MidiControlUI::instance()); } } @@ -175,27 +177,26 @@ MIDIControllable::stop_learning () int MIDIControllable::control_to_midi (float val) { - boost::shared_ptr controllable = _controllable.lock (); - if (!controllable) { + if (!_controllable) { return 0; } - if (controllable->is_gain_like()) { - return controllable->internal_to_interface (val) * max_value_for_type (); + if (_controllable->is_gain_like()) { + return _controllable->internal_to_interface (val) * max_value_for_type (); } - float control_min = controllable->lower (); - float control_max = controllable->upper (); + float control_min = _controllable->lower (); + float control_max = _controllable->upper (); float control_range = control_max - control_min; - if (controllable->is_toggle()) { + if (_controllable->is_toggle()) { if (val >= (control_min + (control_range/2.0f))) { return max_value_for_type(); } else { return 0; } } else { - boost::shared_ptr actl = boost::dynamic_pointer_cast (controllable); + boost::shared_ptr actl = boost::dynamic_pointer_cast (_controllable); if (actl) { control_min = actl->internal_to_interface(control_min); control_max = actl->internal_to_interface(control_max); @@ -211,8 +212,7 @@ MIDIControllable::control_to_midi (float val) float MIDIControllable::midi_to_control (int val) { - boost::shared_ptr controllable = _controllable.lock (); - if (!controllable) { + if (!_controllable) { return 0; } /* fiddle with MIDI value so that we get an odd number of integer steps @@ -222,17 +222,17 @@ MIDIControllable::midi_to_control (int val) float fv = (val == 0 ? 0 : float (val - 1) / (max_value_for_type() - 1)); - if (controllable->is_gain_like()) { - return controllable->interface_to_internal (fv); + if (_controllable->is_gain_like()) { + return _controllable->interface_to_internal (fv); } DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Raw value %1 float %2\n", val, fv)); - float control_min = controllable->lower (); - float control_max = controllable->upper (); + float control_min = _controllable->lower (); + float control_max = _controllable->upper (); float control_range = control_max - control_min; DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Min %1 Max %2 Range %3\n", control_min, control_max, control_range)); - boost::shared_ptr actl = boost::dynamic_pointer_cast (controllable); + boost::shared_ptr actl = boost::dynamic_pointer_cast (_controllable); if (actl) { if (fv == 0.f) return control_min; if (fv == 1.f) return control_max; @@ -275,61 +275,56 @@ MIDIControllable::lookup_controllable() } void -MIDIControllable::drop_controllable (Controllable* c) +MIDIControllable::drop_controllable () { - boost::shared_ptr controllable = _controllable.lock (); - if (controllable && c == controllable.get()) { - set_controllable (boost::shared_ptr()); - } + set_controllable (boost::shared_ptr()); } void MIDIControllable::midi_sense_note (Parser &, EventTwoBytes *msg, bool /*is_on*/) { - if (_controllable.expired ()) { + if (!_controllable) { if (lookup_controllable()) { return; } } - boost::shared_ptr controllable = _controllable.lock (); - assert (controllable); + assert (_controllable); - _surface->maybe_start_touch (controllable); + _surface->maybe_start_touch (_controllable); - if (!controllable->is_toggle()) { + if (!_controllable->is_toggle()) { if (control_additional == msg->note_number) { - controllable->set_value (midi_to_control (msg->velocity), Controllable::UseGroup); + _controllable->set_value (midi_to_control (msg->velocity), Controllable::UseGroup); DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Note %1 value %2 %3\n", (int) msg->note_number, (float) midi_to_control (msg->velocity), current_uri() )); } } else { if (control_additional == msg->note_number) { - float new_value = controllable->get_value() > 0.5f ? 0.0f : 1.0f; - controllable->set_value (new_value, Controllable::UseGroup); + float new_value = _controllable->get_value() > 0.5f ? 0.0f : 1.0f; + _controllable->set_value (new_value, Controllable::UseGroup); DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Note %1 Value %2 %3\n", (int) msg->note_number, (float) new_value, current_uri())); } } - last_value = (MIDI::byte) (controllable->get_value() * 127.0); // to prevent feedback fights + last_value = (MIDI::byte) (_controllable->get_value() * 127.0); // to prevent feedback fights } void MIDIControllable::midi_sense_controller (Parser &, EventTwoBytes *msg) { - if (_controllable.expired ()) { + if (!_controllable) { if (lookup_controllable ()) { return; } } - boost::shared_ptr controllable = _controllable.lock (); - assert (controllable); + assert (_controllable); - _surface->maybe_start_touch (controllable); + _surface->maybe_start_touch (_controllable); if (control_additional == msg->controller_number) { - if (!controllable->is_toggle()) { + if (!_controllable->is_toggle()) { if (get_encoder() == No_enc) { float new_value = msg->value; float max_value = max(last_controllable_value, new_value); @@ -339,8 +334,8 @@ MIDIControllable::midi_sense_controller (Parser &, EventTwoBytes *msg) bool const in_sync = ( range < threshold && - controllable->get_value() <= midi_to_control(max_value) && - controllable->get_value() >= midi_to_control(min_value) + _controllable->get_value() <= midi_to_control(max_value) && + _controllable->get_value() >= midi_to_control(min_value) ); /* If the surface is not motorised, we try to prevent jumps when @@ -349,7 +344,7 @@ MIDIControllable::midi_sense_controller (Parser &, EventTwoBytes *msg) */ if (in_sync || _surface->motorised ()) { - controllable->set_value (midi_to_control (new_value), Controllable::UseGroup); + _controllable->set_value (midi_to_control (new_value), Controllable::UseGroup); } DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("MIDI CC %1 value %2 %3\n", (int) msg->controller_number, (float) midi_to_control(new_value), current_uri() )); @@ -359,30 +354,30 @@ MIDIControllable::midi_sense_controller (Parser &, EventTwoBytes *msg) switch (get_encoder()) { case Enc_L: if (msg->value > 0x40) { - controllable->set_value (midi_to_control (last_value - offset + 1), Controllable::UseGroup); + _controllable->set_value (midi_to_control (last_value - offset + 1), Controllable::UseGroup); } else { - controllable->set_value (midi_to_control (last_value + offset + 1), Controllable::UseGroup); + _controllable->set_value (midi_to_control (last_value + offset + 1), Controllable::UseGroup); } break; case Enc_R: if (msg->value > 0x40) { - controllable->set_value (midi_to_control (last_value + offset + 1), Controllable::UseGroup); + _controllable->set_value (midi_to_control (last_value + offset + 1), Controllable::UseGroup); } else { - controllable->set_value (midi_to_control (last_value - offset + 1), Controllable::UseGroup); + _controllable->set_value (midi_to_control (last_value - offset + 1), Controllable::UseGroup); } break; case Enc_2: if (msg->value > 0x40) { - controllable->set_value (midi_to_control (last_value - (0x7f - msg->value) + 1), Controllable::UseGroup); + _controllable->set_value (midi_to_control (last_value - (0x7f - msg->value) + 1), Controllable::UseGroup); } else { - controllable->set_value (midi_to_control (last_value + offset + 1), Controllable::UseGroup); + _controllable->set_value (midi_to_control (last_value + offset + 1), Controllable::UseGroup); } break; case Enc_B: if (msg->value > 0x40) { - controllable->set_value (midi_to_control (last_value + offset + 1), Controllable::UseGroup); + _controllable->set_value (midi_to_control (last_value + offset + 1), Controllable::UseGroup); } else { - controllable->set_value (midi_to_control (last_value - (0x40 - offset)), Controllable::UseGroup); + _controllable->set_value (midi_to_control (last_value - (0x40 - offset)), Controllable::UseGroup); } break; default: @@ -400,9 +395,9 @@ MIDIControllable::midi_sense_controller (Parser &, EventTwoBytes *msg) /* relax ... first incoming message */ } else { if (msg->value > last_incoming) { - controllable->set_value (1.0, Controllable::UseGroup); + _controllable->set_value (1.0, Controllable::UseGroup); } else { - controllable->set_value (0.0, Controllable::UseGroup); + _controllable->set_value (0.0, Controllable::UseGroup); } DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("dial Midi CC %1 value 1 %2\n", (int) msg->controller_number, current_uri())); } @@ -413,7 +408,7 @@ MIDIControllable::midi_sense_controller (Parser &, EventTwoBytes *msg) * time they are pressed. */ if (msg->value >= 0x40) { - controllable->set_value (controllable->get_value() >= 0.5 ? 0.0 : 1.0, Controllable::UseGroup); + _controllable->set_value (_controllable->get_value() >= 0.5 ? 0.0 : 1.0, Controllable::UseGroup); DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("toggle Midi CC %1 value 1 %2\n", (int) msg->controller_number, current_uri())); } break; @@ -422,76 +417,74 @@ MIDIControllable::midi_sense_controller (Parser &, EventTwoBytes *msg) maintain state (i.e. they know they were pressed) and then send zero the next time. */ if (msg->value >= 0x40) { - controllable->set_value (controllable->get_value() >= 0.5 ? 0.0 : 1.0, Controllable::UseGroup); + _controllable->set_value (_controllable->get_value() >= 0.5 ? 0.0 : 1.0, Controllable::UseGroup); } else { - controllable->set_value (0.0, Controllable::NoGroup); + _controllable->set_value (0.0, Controllable::NoGroup); DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Midi CC %1 value 0 %2\n", (int) msg->controller_number, current_uri())); break; } } } - last_value = (MIDI::byte) (control_to_midi(controllable->get_value())); // to prevent feedback fights + last_value = (MIDI::byte) (control_to_midi(_controllable->get_value())); // to prevent feedback fights } } void MIDIControllable::midi_sense_program_change (Parser &, MIDI::byte msg) { - if (_controllable.expired ()) { + if (!_controllable) { if (lookup_controllable ()) { return; } } - boost::shared_ptr controllable = _controllable.lock (); - assert (controllable); + assert (_controllable); - _surface->maybe_start_touch (controllable); + _surface->maybe_start_touch (_controllable); if (msg == control_additional) { - if (!controllable->is_toggle()) { - controllable->set_value (1.0, Controllable::UseGroup); + if (!_controllable->is_toggle()) { + _controllable->set_value (1.0, Controllable::UseGroup); DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("MIDI program %1 value 1.0 %3\n", (int) msg, current_uri() )); } else { - float new_value = controllable->get_value() > 0.5f ? 0.0f : 1.0f; - controllable->set_value (new_value, Controllable::UseGroup); + float new_value = _controllable->get_value() > 0.5f ? 0.0f : 1.0f; + _controllable->set_value (new_value, Controllable::UseGroup); DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("MIDI program %1 value %2 %3\n", (int) msg, (float) new_value, current_uri())); } } - last_value = (MIDI::byte) (controllable->get_value() * 127.0); // to prevent feedback fights + last_value = (MIDI::byte) (_controllable->get_value() * 127.0); // to prevent feedback fights } void MIDIControllable::midi_sense_pitchbend (Parser &, pitchbend_t pb) { - if (_controllable.expired ()) { + if (!_controllable) { if (lookup_controllable ()) { return; } } - boost::shared_ptr controllable = _controllable.lock (); - assert (controllable); + assert (_controllable); - _surface->maybe_start_touch (controllable); + _surface->maybe_start_touch (_controllable); - if (!controllable->is_toggle()) { - controllable->set_value (midi_to_control (pb), Controllable::UseGroup); + if (!_controllable->is_toggle()) { + _controllable->set_value (midi_to_control (pb), Controllable::UseGroup); DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("MIDI pitchbend %1 value %2 %3\n", (int) control_channel, (float) midi_to_control (pb), current_uri() )); } else { if (pb > 8065.0f) { - controllable->set_value (1, Controllable::UseGroup); + _controllable->set_value (1, Controllable::UseGroup); DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Midi pitchbend %1 value 1 %2\n", (int) control_channel, current_uri())); } else { - controllable->set_value (0, Controllable::UseGroup); + _controllable->set_value (0, Controllable::UseGroup); DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Midi pitchbend %1 value 0 %2\n", (int) control_channel, current_uri())); } } - last_value = control_to_midi (controllable->get_value ()); + last_value = control_to_midi (_controllable->get_value ()); } void @@ -506,9 +499,8 @@ MIDIControllable::midi_receiver (Parser &, MIDI::byte *msg, size_t /*len*/) _surface->check_used_event(msg[0], msg[1]); bind_midi ((channel_t) (msg[0] & 0xf), eventType (msg[0] & 0xF0), msg[1]); - boost::shared_ptr controllable = _controllable.lock (); - if (controllable) { - controllable->LearningFinished (); + if (_controllable) { + _controllable->LearningFinished (); } } @@ -516,9 +508,8 @@ void MIDIControllable::rpn_value_change (Parser&, uint16_t rpn, float val) { if (control_rpn == rpn) { - boost::shared_ptr controllable = _controllable.lock (); - if (controllable) { - controllable->set_value (val, Controllable::UseGroup); + if (_controllable) { + _controllable->set_value (val, Controllable::UseGroup); } } } @@ -527,9 +518,8 @@ void MIDIControllable::nrpn_value_change (Parser&, uint16_t nrpn, float val) { if (control_nrpn == nrpn) { - boost::shared_ptr controllable = _controllable.lock (); - if (controllable) { - controllable->set_value (val, Controllable::UseGroup); + if (_controllable) { + _controllable->set_value (val, Controllable::UseGroup); } } } @@ -538,10 +528,9 @@ void MIDIControllable::rpn_change (Parser&, uint16_t rpn, int dir) { if (control_rpn == rpn) { - boost::shared_ptr controllable = _controllable.lock (); - if (controllable) { + if (_controllable) { /* XXX how to increment/decrement ? */ - // controllable->set_value (val); + // _controllable->set_value (val); } } } @@ -550,10 +539,9 @@ void MIDIControllable::nrpn_change (Parser&, uint16_t nrpn, int dir) { if (control_nrpn == nrpn) { - boost::shared_ptr controllable = _controllable.lock (); - if (controllable) { + if (_controllable) { /* XXX how to increment/decrement ? */ - // controllable->set_value (val); + // _controllable->set_value (val); } } } @@ -658,14 +646,15 @@ MIDIControllable::bind_midi (channel_t chn, eventType ev, MIDI::byte additional) MIDI::byte* MIDIControllable::write_feedback (MIDI::byte* buf, int32_t& bufsize, bool /*force*/) { - if (_controllable.expired () || !_surface->get_feedback ()) { + Glib::Threads::Mutex::Lock lm (controllable_lock, Glib::Threads::TRY_LOCK); + if (!lm.locked ()) { + return buf; + } + if (!_controllable || !_surface->get_feedback ()) { return buf; } - boost::shared_ptr controllable = _controllable.lock (); - assert (controllable); - - float val = controllable->get_value (); + float val = _controllable->get_value (); /* Note that when sending RPN/NPRN we do two things: * @@ -800,15 +789,13 @@ MIDIControllable::get_state () XMLNode* node = new XMLNode ("MIDIControllable"); - boost::shared_ptr controllable = _controllable.lock (); - - if (_current_uri.empty() && controllable) { - node->set_property ("id", controllable->id ()); + if (_current_uri.empty() && _controllable) { + node->set_property ("id", _controllable->id ()); } else { node->set_property ("uri", _current_uri); } - if (controllable) { + if (_controllable) { snprintf (buf, sizeof(buf), "0x%x", (int) control_type); node->set_property ("event", (const char *)buf); node->set_property ("channel", (int16_t)control_channel); diff --git a/libs/surfaces/generic_midi/midicontrollable.h b/libs/surfaces/generic_midi/midicontrollable.h index a3f4eaabc2..47cb4f0572 100644 --- a/libs/surfaces/generic_midi/midicontrollable.h +++ b/libs/surfaces/generic_midi/midicontrollable.h @@ -115,7 +115,7 @@ private: int max_value_for_type () const; GenericMidiControlProtocol* _surface; - boost::weak_ptr _controllable; + boost::shared_ptr _controllable; std::string _current_uri; MIDI::Parser& _parser; bool setting; @@ -130,7 +130,7 @@ private: int midi_msg_id; /* controller ID or note number */ PBD::ScopedConnection midi_sense_connection[2]; PBD::ScopedConnection midi_learn_connection; - PBD::ScopedConnection controllable_death_connection; + PBD::ScopedConnectionList controllable_death_connections; /** the type of MIDI message that is used for this control */ MIDI::eventType control_type; MIDI::byte control_additional; @@ -142,7 +142,8 @@ private: std::string _what; bool _bank_relative; - void drop_controllable (PBD::Controllable*); + void drop_controllable (); + Glib::Threads::Mutex controllable_lock; void midi_receiver (MIDI::Parser &p, MIDI::byte *, size_t); void midi_sense_note (MIDI::Parser &, MIDI::EventTwoBytes *, bool is_on); From 60686a7b37707600e97b1d3b5e2e2edd7e0ebc03 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 17:20:28 +0100 Subject: [PATCH 13/16] NO-OP: whitespace --- libs/ardour/monitor_processor.cc | 424 +++++++++++++++---------------- 1 file changed, 211 insertions(+), 213 deletions(-) diff --git a/libs/ardour/monitor_processor.cc b/libs/ardour/monitor_processor.cc index b4e996a52e..42a23a7b4d 100644 --- a/libs/ardour/monitor_processor.cc +++ b/libs/ardour/monitor_processor.cc @@ -45,39 +45,39 @@ namespace ARDOUR { } MonitorProcessor::MonitorProcessor (Session& s) - : Processor (s, X_("MonitorOut")) - , solo_cnt (0) - , _monitor_active (false) + : Processor (s, X_("MonitorOut")) + , solo_cnt (0) + , _monitor_active (false) - , _dim_all_ptr (new MPControl (false, _("monitor dim"), Controllable::Toggle)) - , _cut_all_ptr (new MPControl (false, _("monitor cut"), Controllable::Toggle)) - , _mono_ptr (new MPControl (false, _("monitor mono"), Controllable::Toggle)) - , _dim_level_ptr (new MPControl - /* default is -12dB, range is -20dB to 0dB */ - (dB_to_coefficient(-12.0), _("monitor dim level"), Controllable::Flag (0), - dB_to_coefficient(-20.0), dB_to_coefficient (0.0))) - , _solo_boost_level_ptr (new MPControl - /* default is 0dB, range is 0dB to +20dB */ - (dB_to_coefficient(0.0), _("monitor solo boost level"), Controllable::Flag (0), - dB_to_coefficient(0.0), dB_to_coefficient(10.0))) - , _dim_all_control (_dim_all_ptr) - , _cut_all_control (_cut_all_ptr) - , _mono_control (_mono_ptr) - , _dim_level_control (_dim_level_ptr) - , _solo_boost_level_control (_solo_boost_level_ptr) + , _dim_all_ptr (new MPControl (false, _("monitor dim"), Controllable::Toggle)) + , _cut_all_ptr (new MPControl (false, _("monitor cut"), Controllable::Toggle)) + , _mono_ptr (new MPControl (false, _("monitor mono"), Controllable::Toggle)) + , _dim_level_ptr (new MPControl + /* default is -12dB, range is -20dB to 0dB */ + (dB_to_coefficient(-12.0), _("monitor dim level"), Controllable::Flag (0), + dB_to_coefficient(-20.0), dB_to_coefficient (0.0))) + , _solo_boost_level_ptr (new MPControl + /* default is 0dB, range is 0dB to +20dB */ + (dB_to_coefficient(0.0), _("monitor solo boost level"), Controllable::Flag (0), + dB_to_coefficient(0.0), dB_to_coefficient(10.0))) + , _dim_all_control (_dim_all_ptr) + , _cut_all_control (_cut_all_ptr) + , _mono_control (_mono_ptr) + , _dim_level_control (_dim_level_ptr) + , _solo_boost_level_control (_solo_boost_level_ptr) - , _dim_all (*_dim_all_ptr) - , _cut_all (*_cut_all_ptr) - , _mono (*_mono_ptr) - , _dim_level (*_dim_level_ptr) - , _solo_boost_level (*_solo_boost_level_ptr) + , _dim_all (*_dim_all_ptr) + , _cut_all (*_cut_all_ptr) + , _mono (*_mono_ptr) + , _dim_level (*_dim_level_ptr) + , _solo_boost_level (*_solo_boost_level_ptr) { } MonitorProcessor::~MonitorProcessor () { - allocate_channels (0); + allocate_channels (0); /* special case for MPControl */ _dim_all_control->DropReferences (); /* EMIT SIGNAL */ @@ -90,135 +90,135 @@ MonitorProcessor::~MonitorProcessor () void MonitorProcessor::allocate_channels (uint32_t size) { - while (_channels.size() > size) { - if (_channels.back()->soloed) { - if (solo_cnt > 0) { - --solo_cnt; - } - } - ChannelRecord* cr = _channels.back(); - _channels.pop_back(); - delete cr; - } + while (_channels.size() > size) { + if (_channels.back()->soloed) { + if (solo_cnt > 0) { + --solo_cnt; + } + } + ChannelRecord* cr = _channels.back(); + _channels.pop_back(); + delete cr; + } - uint32_t n = _channels.size() + 1; + uint32_t n = _channels.size() + 1; - while (_channels.size() < size) { - _channels.push_back (new ChannelRecord (n)); - } + while (_channels.size() < size) { + _channels.push_back (new ChannelRecord (n)); + } } int MonitorProcessor::set_state (const XMLNode& node, int version) { - int ret = Processor::set_state (node, version); + int ret = Processor::set_state (node, version); - if (ret != 0) { - return ret; - } + if (ret != 0) { + return ret; + } - std::string type_name; - if (!node.get_property (X_("type"), type_name)) { - error << string_compose (X_("programming error: %1"), X_("MonitorProcessor XML settings have no type information")) - << endmsg; - return -1; - } + std::string type_name; + if (!node.get_property (X_("type"), type_name)) { + error << string_compose (X_("programming error: %1"), X_("MonitorProcessor XML settings have no type information")) + << endmsg; + return -1; + } - if (type_name != X_("monitor")) { - error << string_compose (X_("programming error: %1"), X_("MonitorProcessor given unknown XML settings")) - << endmsg; - return -1; - } + if (type_name != X_("monitor")) { + error << string_compose (X_("programming error: %1"), X_("MonitorProcessor given unknown XML settings")) + << endmsg; + return -1; + } - uint32_t channels = 0; - if (!node.get_property (X_("channels"), channels)) { - error << string_compose (X_("programming error: %1"), X_("MonitorProcessor XML settings are missing a channel cnt")) - << endmsg; - return -1; - } + uint32_t channels = 0; + if (!node.get_property (X_("channels"), channels)) { + error << string_compose (X_("programming error: %1"), X_("MonitorProcessor XML settings are missing a channel cnt")) + << endmsg; + return -1; + } - allocate_channels (channels); + allocate_channels (channels); - // need to check that these conversions are working as expected - gain_t val; - if (node.get_property (X_("dim-level"), val)) { - _dim_level = val; - } + // need to check that these conversions are working as expected + gain_t val; + if (node.get_property (X_("dim-level"), val)) { + _dim_level = val; + } - if (node.get_property (X_("solo-boost-level"), val)) { - _solo_boost_level = val; - } + if (node.get_property (X_("solo-boost-level"), val)) { + _solo_boost_level = val; + } - bool bool_val; - if (node.get_property (X_("cut-all"), bool_val)) { - _cut_all = bool_val; - } + bool bool_val; + if (node.get_property (X_("cut-all"), bool_val)) { + _cut_all = bool_val; + } - if (node.get_property (X_("dim-all"), bool_val)) { - _dim_all = bool_val; - } + if (node.get_property (X_("dim-all"), bool_val)) { + _dim_all = bool_val; + } - if (node.get_property (X_("mono"), bool_val)) { - _mono = bool_val; - } + if (node.get_property (X_("mono"), bool_val)) { + _mono = bool_val; + } - for (XMLNodeList::const_iterator i = node.children().begin(); i != node.children().end(); ++i) { + for (XMLNodeList::const_iterator i = node.children().begin(); i != node.children().end(); ++i) { - if ((*i)->name() == X_("Channel")) { + if ((*i)->name() == X_("Channel")) { - uint32_t chn; - if (!(*i)->get_property (X_("id"), chn)) { - error << string_compose (X_("programming error: %1"), X_("MonitorProcessor XML settings are missing an ID")) - << endmsg; - return -1; - } + uint32_t chn; + if (!(*i)->get_property (X_("id"), chn)) { + error << string_compose (X_("programming error: %1"), X_("MonitorProcessor XML settings are missing an ID")) + << endmsg; + return -1; + } - if (chn >= _channels.size()) { - error << string_compose (X_("programming error: %1"), X_("MonitorProcessor XML settings has an illegal channel count")) - << endmsg; - return -1; - } - ChannelRecord& cr (*_channels[chn]); + if (chn >= _channels.size()) { + error << string_compose (X_("programming error: %1"), X_("MonitorProcessor XML settings has an illegal channel count")) + << endmsg; + return -1; + } + ChannelRecord& cr (*_channels[chn]); - bool gain_coeff_zero; - if ((*i)->get_property ("cut", gain_coeff_zero)) { - if (gain_coeff_zero) { - cr.cut = GAIN_COEFF_ZERO; - } else { - cr.cut = GAIN_COEFF_UNITY; - } - } + bool gain_coeff_zero; + if ((*i)->get_property ("cut", gain_coeff_zero)) { + if (gain_coeff_zero) { + cr.cut = GAIN_COEFF_ZERO; + } else { + cr.cut = GAIN_COEFF_UNITY; + } + } - bool dim; - if ((*i)->get_property ("dim", dim)) { - cr.dim = dim; - } + bool dim; + if ((*i)->get_property ("dim", dim)) { + cr.dim = dim; + } - bool invert_polarity; - if ((*i)->get_property ("invert", invert_polarity)) { - if (invert_polarity) { - cr.polarity = -1.0f; - } else { - cr.polarity = 1.0f; - } - } + bool invert_polarity; + if ((*i)->get_property ("invert", invert_polarity)) { + if (invert_polarity) { + cr.polarity = -1.0f; + } else { + cr.polarity = 1.0f; + } + } - bool soloed; - if ((*i)->get_property ("solo", soloed)) { - cr.soloed = soloed; - } - } - } + bool soloed; + if ((*i)->get_property ("solo", soloed)) { + cr.soloed = soloed; + } + } + } - /* reset solo cnt */ + /* reset solo cnt */ - solo_cnt = 0; + solo_cnt = 0; - for (vector::const_iterator x = _channels.begin(); x != _channels.end(); ++x) { - if ((*x)->soloed) { - solo_cnt++; - } - } + for (vector::const_iterator x = _channels.begin(); x != _channels.end(); ++x) { + if ((*x)->soloed) { + solo_cnt++; + } + } update_monitor_state (); return 0; @@ -246,7 +246,7 @@ MonitorProcessor::state () uint32_t chn = 0; for (vector::const_iterator x = _channels.begin (); x != _channels.end (); - ++x, ++chn) { + ++x, ++chn) { chn_node = new XMLNode (X_("Channel")); chn_node->set_property ("id", chn); @@ -266,87 +266,87 @@ MonitorProcessor::state () void MonitorProcessor::run (BufferSet& bufs, samplepos_t /*start_sample*/, samplepos_t /*end_sample*/, double /*speed*/, pframes_t nframes, bool /*result_required*/) { - uint32_t chn = 0; - gain_t target_gain; - gain_t dim_level_this_time = _dim_level; - gain_t global_cut = (_cut_all ? GAIN_COEFF_ZERO : GAIN_COEFF_UNITY); - gain_t global_dim = (_dim_all ? dim_level_this_time : GAIN_COEFF_UNITY); - gain_t solo_boost; + uint32_t chn = 0; + gain_t target_gain; + gain_t dim_level_this_time = _dim_level; + gain_t global_cut = (_cut_all ? GAIN_COEFF_ZERO : GAIN_COEFF_UNITY); + gain_t global_dim = (_dim_all ? dim_level_this_time : GAIN_COEFF_UNITY); + gain_t solo_boost; - if (_session.listening() || _session.soloing()) { - solo_boost = _solo_boost_level; - } else { - solo_boost = GAIN_COEFF_UNITY; - } + if (_session.listening() || _session.soloing()) { + solo_boost = _solo_boost_level; + } else { + solo_boost = GAIN_COEFF_UNITY; + } - for (BufferSet::audio_iterator b = bufs.audio_begin(); b != bufs.audio_end(); ++b) { + for (BufferSet::audio_iterator b = bufs.audio_begin(); b != bufs.audio_end(); ++b) { - /* don't double-scale by both track dim and global dim coefficients */ + /* don't double-scale by both track dim and global dim coefficients */ - gain_t dim_level = (global_dim == GAIN_COEFF_UNITY ? (_channels[chn]->dim ? dim_level_this_time : GAIN_COEFF_UNITY) : GAIN_COEFF_UNITY); + gain_t dim_level = (global_dim == GAIN_COEFF_UNITY ? (_channels[chn]->dim ? dim_level_this_time : GAIN_COEFF_UNITY) : GAIN_COEFF_UNITY); - if (_channels[chn]->soloed) { - target_gain = _channels[chn]->polarity * _channels[chn]->cut * dim_level * global_cut * global_dim * solo_boost; - } else { - if (solo_cnt == 0) { - target_gain = _channels[chn]->polarity * _channels[chn]->cut * dim_level * global_cut * global_dim * solo_boost; - } else { - target_gain = GAIN_COEFF_ZERO; - } - } + if (_channels[chn]->soloed) { + target_gain = _channels[chn]->polarity * _channels[chn]->cut * dim_level * global_cut * global_dim * solo_boost; + } else { + if (solo_cnt == 0) { + target_gain = _channels[chn]->polarity * _channels[chn]->cut * dim_level * global_cut * global_dim * solo_boost; + } else { + target_gain = GAIN_COEFF_ZERO; + } + } - if (target_gain != _channels[chn]->current_gain || target_gain != GAIN_COEFF_UNITY) { + if (target_gain != _channels[chn]->current_gain || target_gain != GAIN_COEFF_UNITY) { - _channels[chn]->current_gain = Amp::apply_gain (*b, _session.nominal_sample_rate(), nframes, _channels[chn]->current_gain, target_gain); - } + _channels[chn]->current_gain = Amp::apply_gain (*b, _session.nominal_sample_rate(), nframes, _channels[chn]->current_gain, target_gain); + } - ++chn; - } + ++chn; + } - if (_mono) { - DEBUG_TRACE (DEBUG::Monitor, "mono-izing\n"); + if (_mono) { + DEBUG_TRACE (DEBUG::Monitor, "mono-izing\n"); - /* chn is now the number of channels, use as a scaling factor when mixing - */ - gain_t scale = 1.f / (float)chn; - BufferSet::audio_iterator b = bufs.audio_begin(); - AudioBuffer& ab (*b); - Sample* buf = ab.data(); + /* chn is now the number of channels, use as a scaling factor when mixing + */ + gain_t scale = 1.f / (float)chn; + BufferSet::audio_iterator b = bufs.audio_begin(); + AudioBuffer& ab (*b); + Sample* buf = ab.data(); - /* scale the first channel */ + /* scale the first channel */ - for (pframes_t n = 0; n < nframes; ++n) { - buf[n] *= scale; - } + for (pframes_t n = 0; n < nframes; ++n) { + buf[n] *= scale; + } - /* add every other channel into the first channel's buffer */ + /* add every other channel into the first channel's buffer */ - ++b; - for (; b != bufs.audio_end(); ++b) { - AudioBuffer& ob (*b); - Sample* obuf = ob.data (); - for (pframes_t n = 0; n < nframes; ++n) { - buf[n] += obuf[n] * scale; - } - } + ++b; + for (; b != bufs.audio_end(); ++b) { + AudioBuffer& ob (*b); + Sample* obuf = ob.data (); + for (pframes_t n = 0; n < nframes; ++n) { + buf[n] += obuf[n] * scale; + } + } - /* copy the first channel to every other channel's buffer */ + /* copy the first channel to every other channel's buffer */ - b = bufs.audio_begin(); - ++b; - for (; b != bufs.audio_end(); ++b) { - AudioBuffer& ob (*b); - Sample* obuf = ob.data (); - memcpy (obuf, buf, sizeof (Sample) * nframes); - } - } + b = bufs.audio_begin(); + ++b; + for (; b != bufs.audio_end(); ++b) { + AudioBuffer& ob (*b); + Sample* obuf = ob.data (); + memcpy (obuf, buf, sizeof (Sample) * nframes); + } + } } bool MonitorProcessor::configure_io (ChanCount in, ChanCount out) { - allocate_channels (in.n_audio()); - return Processor::configure_io (in, out); + allocate_channels (in.n_audio()); + return Processor::configure_io (in, out); } bool @@ -426,51 +426,49 @@ MonitorProcessor::set_dim_all (bool yn) bool MonitorProcessor::display_to_user () const { - return false; + return false; } bool MonitorProcessor::soloed (uint32_t chn) const { - return _channels[chn]->soloed; + return _channels[chn]->soloed; } - bool MonitorProcessor::inverted (uint32_t chn) const { - return _channels[chn]->polarity < 0.0f; + return _channels[chn]->polarity < 0.0f; } - bool MonitorProcessor::cut (uint32_t chn) const { - return _channels[chn]->cut == GAIN_COEFF_ZERO; + return _channels[chn]->cut == GAIN_COEFF_ZERO; } bool MonitorProcessor::dimmed (uint32_t chn) const { - return _channels[chn]->dim; + return _channels[chn]->dim; } bool MonitorProcessor::mono () const { - return _mono; + return _mono; } bool MonitorProcessor::dim_all () const { - return _dim_all; + return _dim_all; } bool MonitorProcessor::cut_all () const { - return _cut_all; + return _cut_all; } void @@ -499,37 +497,37 @@ MonitorProcessor::update_monitor_state () boost::shared_ptr MonitorProcessor::channel_cut_control (uint32_t chn) const { - if (chn < _channels.size()) { - return _channels[chn]->cut_control; - } - return boost::shared_ptr(); + if (chn < _channels.size()) { + return _channels[chn]->cut_control; + } + return boost::shared_ptr(); } boost::shared_ptr MonitorProcessor::channel_dim_control (uint32_t chn) const { - if (chn < _channels.size()) { - return _channels[chn]->dim_control; - } - return boost::shared_ptr(); + if (chn < _channels.size()) { + return _channels[chn]->dim_control; + } + return boost::shared_ptr(); } boost::shared_ptr MonitorProcessor::channel_polarity_control (uint32_t chn) const { - if (chn < _channels.size()) { - return _channels[chn]->polarity_control; - } - return boost::shared_ptr(); + if (chn < _channels.size()) { + return _channels[chn]->polarity_control; + } + return boost::shared_ptr(); } boost::shared_ptr MonitorProcessor::channel_solo_control (uint32_t chn) const { - if (chn < _channels.size()) { - return _channels[chn]->soloed_control; - } - return boost::shared_ptr(); + if (chn < _channels.size()) { + return _channels[chn]->soloed_control; + } + return boost::shared_ptr(); } MonitorProcessor::ChannelRecord::ChannelRecord (uint32_t chn) From 3448f3151e52565197692512e92be7dc0c08a7c2 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 23 Mar 2019 17:35:26 +0100 Subject: [PATCH 14/16] NO-OP: whitespac --- .../generic_midi_control_protocol.cc | 20 +++++++++---------- .../generic_midi_control_protocol.h | 6 +++--- .../surfaces/generic_midi/midicontrollable.cc | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc index 44b9351554..3c0507abec 100644 --- a/libs/surfaces/generic_midi/generic_midi_control_protocol.cc +++ b/libs/surfaces/generic_midi/generic_midi_control_protocol.cc @@ -1232,11 +1232,11 @@ GenericMidiControlProtocol::create_function (const XMLNode& node) ev = MIDI::program; } else if ((prop = node.property (X_("sysex"))) != 0 || (prop = node.property (X_("msg"))) != 0) { - if (prop->name() == X_("sysex")) { - ev = MIDI::sysex; - } else { - ev = MIDI::any; - } + if (prop->name() == X_("sysex")) { + ev = MIDI::sysex; + } else { + ev = MIDI::any; + } int val; uint32_t cnt; @@ -1332,11 +1332,11 @@ GenericMidiControlProtocol::create_action (const XMLNode& node) ev = MIDI::program; } else if ((prop = node.property (X_("sysex"))) != 0 || (prop = node.property (X_("msg"))) != 0) { - if (prop->name() == X_("sysex")) { - ev = MIDI::sysex; - } else { - ev = MIDI::any; - } + if (prop->name() == X_("sysex")) { + ev = MIDI::sysex; + } else { + ev = MIDI::any; + } int val; uint32_t cnt; diff --git a/libs/surfaces/generic_midi/generic_midi_control_protocol.h b/libs/surfaces/generic_midi/generic_midi_control_protocol.h index c198969ca0..bd7e646282 100644 --- a/libs/surfaces/generic_midi/generic_midi_control_protocol.h +++ b/libs/surfaces/generic_midi/generic_midi_control_protocol.h @@ -39,7 +39,7 @@ namespace ARDOUR { } namespace MIDI { - class Port; + class Port; } class MIDIControllable; @@ -144,8 +144,8 @@ private: }; typedef std::list MIDIPendingControllables; MIDIPendingControllables pending_controllables; - Glib::Threads::Mutex controllables_lock; - Glib::Threads::Mutex pending_lock; + Glib::Threads::Mutex controllables_lock; + Glib::Threads::Mutex pending_lock; bool start_learning (boost::weak_ptr); void stop_learning (boost::weak_ptr); diff --git a/libs/surfaces/generic_midi/midicontrollable.cc b/libs/surfaces/generic_midi/midicontrollable.cc index 741cd0c781..ee08bd70a9 100644 --- a/libs/surfaces/generic_midi/midicontrollable.cc +++ b/libs/surfaces/generic_midi/midicontrollable.cc @@ -725,7 +725,7 @@ MIDIControllable::write_feedback (MIDI::byte* buf, int32_t& bufsize, bool /*forc return buf; } - DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Feedback: %1 %2\n", control_description(), current_uri())); + DEBUG_TRACE (DEBUG::GenericMidi, string_compose ("Feedback: %1 %2\n", control_description(), current_uri())); *buf++ = (0xF0 & control_type) | (0xF & control_channel); int ev_size = 3; From 60262275af54085fa290210a75ff6da7b5a04fbc Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Mon, 25 Mar 2019 17:05:19 +0100 Subject: [PATCH 15/16] Do not create automation when shifting (insert/remove time) This fixes a bug when shift() creates automation for parameters that can not have any automation (hidden parameters, Mixbus PRE). The GUI (RTAV) aborts() when it finds an automation lane for a hidden parameter. This also cleans up shift() operations in general. Empty automation lanes should be left alone, no guard-point at zero should be added. --- libs/ardour/route.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/ardour/route.cc b/libs/ardour/route.cc index 00a966aa2c..ac66e4feda 100644 --- a/libs/ardour/route.cc +++ b/libs/ardour/route.cc @@ -4224,6 +4224,9 @@ Route::shift (samplepos_t pos, samplecnt_t samples) boost::shared_ptr ac = (*i)->automation_control (*p); if (ac) { boost::shared_ptr al = ac->alist(); + if (al->empty ()) { + continue; + } XMLNode &before = al->get_state (); al->shift (pos, samples); XMLNode &after = al->get_state (); From 854de91fb0a144cb4d4b500dd092a90e485d0894 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Tue, 26 Mar 2019 15:35:36 +0100 Subject: [PATCH 16/16] Fix mingw compile (declare int64_t and int32_t) --- libs/pbd/pbd/playback_buffer.h | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/pbd/pbd/playback_buffer.h b/libs/pbd/pbd/playback_buffer.h index e5a6857b1f..d41adea2e2 100644 --- a/libs/pbd/pbd/playback_buffer.h +++ b/libs/pbd/pbd/playback_buffer.h @@ -21,6 +21,7 @@ #define playback_buffer_h #include +#include #include #include "pbd/libpbd_visibility.h"