diff options
author | Robin Gareus <robin@gareus.org> | 2019-03-23 02:09:39 +0100 |
---|---|---|
committer | Robin Gareus <robin@gareus.org> | 2019-03-23 02:09:39 +0100 |
commit | e9b36f2beab7d7c22d321291e5cfe39f5b0a4349 (patch) | |
tree | f39fcf5c0b867fcd373aa15c9f71b218793eb0b4 /libs/surfaces/generic_midi | |
parent | 16fe286ed97e89a6768e0eb1e983ab55cc396eaf (diff) |
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.
Diffstat (limited to 'libs/surfaces/generic_midi')
4 files changed, 91 insertions, 51 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 <Controllable> wc) { - if (c == 0) { + boost::shared_ptr<Controllable> 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<PBD::Controllable> wc) { + boost::shared_ptr<Controllable> 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<PBD::Controllable> 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> controllable) { - AutomationControl *actl = dynamic_cast<AutomationControl*> (controllable); + boost::shared_ptr<AutomationControl> actl = boost::dynamic_pointer_cast<AutomationControl> (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<PBD::Controllable> lookup_controllable (std::string const &) const; - void maybe_start_touch (PBD::Controllable*); + void maybe_start_touch (boost::shared_ptr<PBD::Controllable>); XMLNode& get_state (); int set_state (const XMLNode&, int version); @@ -110,7 +110,7 @@ class GenericMidiControlProtocol : public ARDOUR::ControlProtocol { PBD::Signal0<void> ConnectionChange; - private: +private: boost::shared_ptr<ARDOUR::Bundle> _input_bundle; boost::shared_ptr<ARDOUR::Bundle> _output_bundle; boost::shared_ptr<ARDOUR::AsyncMIDIPort> _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<PBD::Controllable>); + void stop_learning (boost::weak_ptr<PBD::Controllable>); 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<PBD::Controllable> 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; } +boost::shared_ptr<PBD::Controllable> +MIDIControllable::get_controllable () const +{ + return _controllable.lock (); +} + void -MIDIControllable::set_controllable (Controllable* c) +MIDIControllable::set_controllable (boost::shared_ptr<PBD::Controllable> c) { - if (c == controllable) { + 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<PBD::Controllable> (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 = _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<AutomationControl*> (controllable); + boost::shared_ptr<AutomationControl> actl = boost::dynamic_pointer_cast<AutomationControl> (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 = _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<AutomationControl*> (controllable); + boost::shared_ptr<AutomationControl> actl = boost::dynamic_pointer_cast<AutomationControl> (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 = _controllable.lock (); + if (controllable && c == controllable.get()) { + set_controllable (boost::shared_ptr<PBD::Controllable>()); } } 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 = _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 = _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 = _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 = _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 = _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 = _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 = _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 = _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 = _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 = _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 = _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<PBD::Controllable>, 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<PBD::Controllable>); + boost::shared_ptr<PBD::Controllable> 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<PBD::Controllable> _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__ - |