Patch for OSC cue observers backported for 8.10

I’ve been having issues with OSC crashing when using foldback mixes. Ardour would crash (on both Windows and Linux) in certain situations and would always crash when disabling OSC in preferences after foldback cues were started.

Robin (x42) was kind enough to resolve this but it’s obviously only available in the v9 master branch. I’ve backported all the necesary patches for 8.10 in case it’s of use to anyone.

OSC observer heap buffer-overflow/shared_ptr fix backport for 8.10

This combines commits:-

84027120cc - Fix OSC observer heap buffer-overflow

86a4447805 - OSC: Never, ever bind a shared_ptr<T> to a signal
This fixes crashes when controllable are destroyed, or
OSC surface is disabled (signals retain a reference).

2816c85324 - Fix OSC observer heap buffer-overflow
OSCCueObserver::send_init may populate a sparse map, rather
than a contiguously indexed vector

backported for 8.10 which fixes crashes when using OSC foldback
mixes and also stops a crash when disabling the OSC protocol in
preferences when OSC Cue observer is in use.
diff --git a/libs/surfaces/osc/osc_cue_observer.cc b/libs/surfaces/osc/osc_cue_observer.cc
index 169587751c..c5b16dec9f 100644
--- a/libs/surfaces/osc/osc_cue_observer.cc
+++ b/libs/surfaces/osc/osc_cue_observer.cc
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2017-2018 Len Ovens <len@ovenwerks.net>
+ * Copyright (C) 2024 Robin Gareus <robin@gareus.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -93,8 +94,8 @@ OSCCueObserver::refresh_strip (std::shared_ptr<ARDOUR::Stripable> new_strip, Sor
 	_strip->mute_control()->Changed.connect (strip_connections, MISSING_INVALIDATOR, boost::bind (&OSCCueObserver::send_change_message, this, X_("/cue/mute"), 0, _strip->mute_control()), OSC::instance());
 	send_change_message (X_("/cue/mute"), 0, _strip->mute_control());
 
-	gain_timeout.push_back (0);
-	_last_gain.push_back (-1.0);
+	gain_timeout[0] = 0;
+	_last_gain[0] = -1.0; // unused
 	_strip->gain_control()->Changed.connect (strip_connections, MISSING_INVALIDATOR, boost::bind (&OSCCueObserver::send_gain_message, this, 0, _strip->gain_control(), false), OSC::instance());
 	send_gain_message (0, _strip->gain_control(), true);
 
@@ -161,15 +162,16 @@ OSCCueObserver::send_init()
 
 
 			if (send->gain_control()) {
-				gain_timeout.push_back (0);
-				_last_gain.push_back (-1.0);
-				send->gain_control()->Changed.connect (send_connections, MISSING_INVALIDATOR, boost::bind (&OSCCueObserver::send_gain_message, this, i + 1, send->gain_control(), false), OSC::instance());
+				gain_timeout[i + 1] = 0;
+				_last_gain[i + 1] = -1.0;
+				send->gain_control()->Changed.connect (send_connections, MISSING_INVALIDATOR, boost::bind (&OSCCueObserver::send_gain_message, this, i + 1, std::weak_ptr<Controllable>(send->gain_control()), false), OSC::instance());
 				send_gain_message (i + 1, send->gain_control(), true);
 			}
 
 			std::shared_ptr<Processor> proc = std::dynamic_pointer_cast<Processor> (send);
-				proc->ActiveChanged.connect (send_connections, MISSING_INVALIDATOR, boost::bind (&OSCCueObserver::send_enabled_message, this, X_("/cue/send/enable"), i + 1, proc), OSC::instance());
-				send_enabled_message (X_("/cue/send/enable"), i + 1, proc);
+			std::weak_ptr<Processor> wproc (proc);
+			proc->ActiveChanged.connect (send_connections, MISSING_INVALIDATOR, boost::bind (&OSCCueObserver::send_enabled_message, this, X_("/cue/send/enable"), i + 1, wproc), OSC::instance());
+			send_enabled_message (X_("/cue/send/enable"), i + 1, wproc);
 		}
 	}
 
@@ -218,8 +220,12 @@ OSCCueObserver::name_changed (const PBD::PropertyChange& what_changed, uint32_t
 }
 
 void
-OSCCueObserver::send_change_message (string path, uint32_t id, std::shared_ptr<Controllable> controllable)
+OSCCueObserver::send_change_message (string path, uint32_t id, std::weak_ptr<Controllable> weak_controllable)
 {
+	std::shared_ptr<Controllable> controllable = weak_controllable.lock ();
+	if (!controllable) {
+		return;
+	}
 	if (id) {
 		path = string_compose("%1/%2", path, id);
 	}
@@ -228,18 +234,21 @@ OSCCueObserver::send_change_message (string path, uint32_t id, std::shared_ptr<C
 }
 
 void
-OSCCueObserver::send_gain_message (uint32_t id,  std::shared_ptr<Controllable> controllable, bool force)
+OSCCueObserver::send_gain_message (uint32_t id, std::weak_ptr<Controllable> weak_controllable, bool force)
 {
+	std::shared_ptr<Controllable> controllable = weak_controllable.lock ();
+	if (!controllable) {
+		return;
+	}
+
 	if (_last_gain[id] != controllable->get_value()) {
 		_last_gain[id] = controllable->get_value();
 	} else {
 		return;
 	}
 	if (id) {
-		_osc.text_message_with_id (X_("/cue/send/name"), id, string_compose ("%1%2%3", std::fixed, std::setprecision(2), accurate_coefficient_to_dB (controllable->get_value())), true, addr);
 		_osc.float_message_with_id (X_("/cue/send/fader"), id, controllable->internal_to_interface (controllable->get_value()), true, addr);
 	} else {
-		_osc.text_message (X_("/cue/name"), string_compose ("%1%2%3", std::fixed, std::setprecision(2), accurate_coefficient_to_dB (controllable->get_value())), addr);
 		_osc.float_message (X_("/cue/fader"), controllable->internal_to_interface (controllable->get_value()), addr);
 	}
 
@@ -247,8 +256,12 @@ OSCCueObserver::send_gain_message (uint32_t id,  std::shared_ptr<Controllable> c
 }
 
 void
-OSCCueObserver::send_enabled_message (std::string path, uint32_t id, std::shared_ptr<ARDOUR::Processor> proc)
+OSCCueObserver::send_enabled_message (std::string path, uint32_t id, std::weak_ptr<ARDOUR::Processor> weak_proc)
 {
+	std::shared_ptr<ARDOUR::Processor> proc = weak_proc.lock ();
+	if (!proc) {
+		return;
+	}
 	if (id) {
 		_osc.float_message_with_id (path, id, (float) proc->enabled(), true, addr);
 	} else {
diff --git a/libs/surfaces/osc/osc_cue_observer.h b/libs/surfaces/osc/osc_cue_observer.h
index dfe13651da..123976c04b 100644
--- a/libs/surfaces/osc/osc_cue_observer.h
+++ b/libs/surfaces/osc/osc_cue_observer.h
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2017-2018 Len Ovens <len@ovenwerks.net>
+ * Copyright (C) 2024 Robin Gareus <robin@gareus.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -57,14 +58,14 @@ class OSCCueObserver
 	ArdourSurface::OSC::OSCSurface* sur;
 	float _last_meter;
 	float _last_signal;
-	std::vector<uint32_t> gain_timeout;
+	std::map<uint32_t,float> gain_timeout;
 	bool tick_enable;
-	std::vector<float> _last_gain;
+	std::map<uint32_t,float> _last_gain;
 
 	void name_changed (const PBD::PropertyChange& what_changed, uint32_t id);
-	void send_change_message (std::string path, uint32_t id, std::shared_ptr<PBD::Controllable> controllable);
-	void send_gain_message (uint32_t id, std::shared_ptr<PBD::Controllable> controllable, bool force);
-	void send_enabled_message (std::string path, uint32_t id, std::shared_ptr<ARDOUR::Processor> proc);
+	void send_change_message (std::string path, uint32_t id, std::weak_ptr<PBD::Controllable> controllable);
+	void send_gain_message (uint32_t id, std::weak_ptr<PBD::Controllable> controllable, bool force);
+	void send_enabled_message (std::string path, uint32_t id, std::weak_ptr<ARDOUR::Processor> proc);
 	void send_init (void);
 	void send_end (uint32_t new_sends_size);
 	void send_restart (void);
1 Like