13
0

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.
This commit is contained in:
Robin Gareus 2019-03-23 02:09:39 +01:00
parent 16fe286ed9
commit e9b36f2bea
Signed by: rgareus
GPG Key ID: A090BCE02CF57F04
4 changed files with 92 additions and 52 deletions

View File

@ -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 ());
}

View File

@ -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*);

View File

@ -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;
}
void
MIDIControllable::set_controllable (Controllable* c)
boost::shared_ptr<PBD::Controllable>
MIDIControllable::get_controllable () const
{
if (c == controllable) {
return _controllable.lock ();
}
void
MIDIControllable::set_controllable (boost::shared_ptr<PBD::Controllable> 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<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);

View File

@ -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__