new approach to handling Transport Masters when engine is restarted

Trust that ::reset() works for all transport masters, and call it when engine is stopped. This way
the transport masters are ready to be called again as soon as the engine restarts.

14 files changed, 139 insertions, 50 deletions

diff --git a/libs/ardour/ardour/transport_master.h b/libs/ardour/ardour/transport_master.h
index c70757647b..98f8efae88 100644
--- a/libs/ardour/ardour/transport_master.h
+++ b/libs/ardour/ardour/transport_master.h
@@ -33,7 +33,7 @@
 #include "pbd/i18n.h"
 #include "pbd/properties.h"
 #include "pbd/signals.h"
-#include "pbd/stateful.h"
+#include "pbd/statefuldestructible.h"
 
 #include "temporal/time.h"
 
@@ -214,6 +214,8 @@ class LIBARDOUR_API TransportMaster : public PBD::Stateful {
 	 */
 	virtual bool speed_and_position (double& speed, samplepos_t& position, samplepos_t & lp, samplepos_t & when, samplepos_t now);
 
+	virtual void reset (bool with_position) = 0;
+
 	/**
 	 * reports to ARDOUR whether the TransportMaster is currently synced to its external
 	 * time source.
@@ -346,6 +348,8 @@ class LIBARDOUR_API TransportMaster : public PBD::Stateful {
 
 	std::string display_name (bool sh/*ort*/ = true) const;
 
+	virtual void unregister_port ();
+
   protected:
 	SyncSource      _type;
 	PBD::Property<std::string>   _name;
@@ -423,6 +427,9 @@ class LIBARDOUR_API MTC_TransportMaster : public TimecodeTransportMaster, public
 
 	void pre_process (pframes_t nframes, samplepos_t now, boost::optional<samplepos_t>);
 
+	void unregister_port ();
+
+	void reset (bool with_pos);
 	bool locked() const;
 	bool ok() const;
 	void handle_locate (const MIDI::byte*);
@@ -467,7 +474,6 @@ class LIBARDOUR_API MTC_TransportMaster : public TimecodeTransportMaster, public
 	Timecode::Time timecode;
 	bool           printed_timecode_warning;
 
-	void reset (bool with_pos);
 	void queue_reset (bool with_pos);
 	void maybe_reset ();
 
@@ -490,6 +496,7 @@ public:
 
 	void pre_process (pframes_t nframes, samplepos_t now, boost::optional<samplepos_t>);
 
+	void reset (bool with_pos);
 	bool locked() const;
 	bool ok() const;
 
@@ -510,7 +517,6 @@ public:
 	bool detect_discontinuity(LTCFrameExt *, int, bool);
 	bool detect_ltc_fps(int, bool);
 	bool equal_ltc_sample_time(LTCFrame *a, LTCFrame *b);
-	void reset (bool with_ts = true);
 	void resync_xrun();
 	void resync_latency();
 	void parse_timecode_offset();
@@ -550,10 +556,13 @@ class LIBARDOUR_API MIDIClock_TransportMaster : public TransportMaster, public T
 
 	void set_session (Session*);
 
+	void unregister_port ();
+
 	void pre_process (pframes_t nframes, samplepos_t now, boost::optional<samplepos_t>);
 
 	void rebind (MidiPort&);
 
+	void reset (bool with_pos);
 	bool locked() const;
 	bool ok() const;
 	bool starting() const;
@@ -594,7 +603,6 @@ class LIBARDOUR_API MIDIClock_TransportMaster : public TransportMaster, public T
 	bool _running;
 	double _bpm;
 
-	void reset ();
 	void start (MIDI::Parser& parser, samplepos_t timestamp);
 	void contineu (MIDI::Parser& parser, samplepos_t timestamp);
 	void stop (MIDI::Parser& parser, samplepos_t timestamp);
@@ -616,6 +624,7 @@ class LIBARDOUR_API Engine_TransportMaster : public TransportMaster
 	bool speed_and_position (double& speed, samplepos_t& pos, samplepos_t &, samplepos_t &, samplepos_t);
 
 	bool starting() const { return _starting; }
+	void reset (bool with_position);
 	bool locked() const;
 	bool ok() const;
 	samplecnt_t update_interval () const;
diff --git a/libs/ardour/ardour/transport_master_manager.h b/libs/ardour/ardour/transport_master_manager.h
index ac61a97a9f..b0de99619a 100644
--- a/libs/ardour/ardour/transport_master_manager.h
+++ b/libs/ardour/ardour/transport_master_manager.h
@@ -36,6 +36,8 @@ class LIBARDOUR_API TransportMasterManager : public boost::noncopyable
 	~TransportMasterManager ();
 
 	int set_default_configuration ();
+	void restart ();
+	void engine_stopped ();
 
 	static TransportMasterManager& instance();
 	/* this method is not thread-safe and is intended to be used only
diff --git a/libs/ardour/audioengine.cc b/libs/ardour/audioengine.cc
index 5e9948ef8d..3fbc7635e1 100644
--- a/libs/ardour/audioengine.cc
+++ b/libs/ardour/audioengine.cc
@@ -1002,6 +1002,8 @@ AudioEngine::stop (bool for_latency)
 	}
 
 	if (stop_engine) {
+		TransportMasterManager& tmm (TransportMasterManager::instance());
+		tmm.engine_stopped ();
 		Stopped (); /* EMIT SIGNAL */
 	}
 
diff --git a/libs/ardour/engine_slave.cc b/libs/ardour/engine_slave.cc
index 451356536a..870b2a1757 100644
--- a/libs/ardour/engine_slave.cc
+++ b/libs/ardour/engine_slave.cc
@@ -57,6 +57,12 @@ Engine_TransportMaster::check_backend()
 	}
 }
 
+void
+Engine_TransportMaster::reset (bool)
+{
+	_starting = false;
+}
+
 bool
 Engine_TransportMaster::locked() const
 {
diff --git a/libs/ardour/globals.cc b/libs/ardour/globals.cc
index b40db9466c..665277388a 100644
--- a/libs/ardour/globals.cc
+++ b/libs/ardour/globals.cc
@@ -561,7 +561,7 @@ ARDOUR::init (bool use_windows_vst, bool try_optimization, const char* localedir
 
 	ARDOUR::AudioEngine::create ();
 
-	/* This will run only once in whatever thread calls AudioEngine::start() */
+	/* This will run in whatever thread calls AudioEngine::start() */
 	ARDOUR::AudioEngine::instance()->Running.connect_same_thread (engine_startup_connection, ARDOUR::init_post_engine);
 
 	/* it is unfortunate that we need to include reserved names here that
@@ -599,39 +599,30 @@ ARDOUR::init (bool use_windows_vst, bool try_optimization, const char* localedir
 }
 
 void
-ARDOUR::init_post_engine (uint32_t /* ignored */)
+ARDOUR::init_post_engine (uint32_t start_cnt)
 {
 	XMLNode* node;
 
-	if ((node = Config->control_protocol_state()) != 0) {
-		ControlProtocolManager::instance().set_state (*node, 0 /* here: global-config state */);
-	}
+	std::cerr << "Engine stated, libardour inits, cnt = " << start_cnt << std::endl;
 
-	if ((node = Config->transport_master_state()) != 0) {
-		if (TransportMasterManager::instance().set_state (*node, Stateful::loading_state_version)) {
-			error << _("Cannot restore transport master manager") << endmsg;
-			/* XXX now what? */
-		}
-	} else {
-		if (TransportMasterManager::instance().set_default_configuration ()) {
-			error << _("Cannot initialize transport master manager") << endmsg;
-			/* XXX now what? */
-		}
-	}
+	if (start_cnt == 0) {
 
-	/* find plugins */
+		/* find plugins */
 
-	ARDOUR::PluginManager::instance().refresh (!Config->get_discover_vst_on_start());
+		ARDOUR::PluginManager::instance().refresh (!Config->get_discover_vst_on_start());
+	}
 
-	/* Don't do this again - we are only meant to execute on the first
-	 * engine start, not any subsequence starts
-	 */
+	if ((node = Config->control_protocol_state()) != 0) {
+		ControlProtocolManager::instance().set_state (*node, 0 /* here: global-config state */);
+	}
 
-	engine_startup_connection.disconnect ();
+	if (start_cnt == 0) {
+		TransportMasterManager::instance().restart ();
+	}
 }
 
 void
-	ARDOUR::cleanup ()
+ARDOUR::cleanup ()
 {
 	if (!libardour_initialized) {
 		return;
diff --git a/libs/ardour/ltc_slave.cc b/libs/ardour/ltc_slave.cc
index 9f8411103f..9fe2635446 100644
--- a/libs/ardour/ltc_slave.cc
+++ b/libs/ardour/ltc_slave.cc
@@ -104,7 +104,7 @@ LTC_TransportMaster::set_session (Session *s)
 		decoder = ltc_decoder_create((int) samples_per_ltc_frame, 128 /*queue size*/);
 
 		parse_timecode_offset();
-		reset();
+		reset (true);
 
 		_session->config.ParameterChanged.connect_same_thread (config_connection, boost::bind (&LTC_TransportMaster::parameter_changed, this, _1));
 	}
@@ -189,10 +189,10 @@ LTC_TransportMaster::resync_latency()
 }
 
 void
-LTC_TransportMaster::reset (bool with_ts)
+LTC_TransportMaster::reset (bool with_position)
 {
 	DEBUG_TRACE (DEBUG::LTC, "LTC reset()\n");
-	if (with_ts) {
+	if (with_position) {
 		current.update (current.position, 0, current.speed);
 		_current_delta = 0;
 	}
@@ -381,7 +381,7 @@ LTC_TransportMaster::process_ltc(samplepos_t const now)
 		}
 
 		if (!ltc_is_stationary && detect_ltc_fps (stime.frame, (sample.ltc.dfbit)? true : false)) {
-			reset();
+			reset(true);
 			fps_detected=true;
 		}
 
@@ -528,7 +528,7 @@ LTC_TransportMaster::pre_process (ARDOUR::pframes_t nframes, samplepos_t now, bo
 	} else if (skip != 0) {
 		/* this should never happen. it may if monotonic_cnt, now overflow on 64bit */
 		DEBUG_TRACE (DEBUG::LTC, string_compose("engine skipped %1 samples\n", skip));
-		reset();
+		reset(true);
 	}
 
 	/* Now feed the incoming LTC signal into the decoder */
@@ -553,7 +553,7 @@ LTC_TransportMaster::pre_process (ARDOUR::pframes_t nframes, samplepos_t now, bo
 
 	if (abs (now - current.timestamp) > FLYWHEEL_TIMEOUT) {
 		DEBUG_TRACE (DEBUG::LTC, "flywheel timeout\n");
-		reset();
+		reset(true);
 		/* don't change position from last known */
 
 		return;
diff --git a/libs/ardour/midi_clock_slave.cc b/libs/ardour/midi_clock_slave.cc
index 706032887d..a6d0af2c1f 100644
--- a/libs/ardour/midi_clock_slave.cc
+++ b/libs/ardour/midi_clock_slave.cc
@@ -88,7 +88,7 @@ MIDIClock_TransportMaster::set_session (Session *session)
 		parser.stop.connect_same_thread (port_connections, boost::bind (&MIDIClock_TransportMaster::stop, this, _1, _2));
 		parser.position.connect_same_thread (port_connections, boost::bind (&MIDIClock_TransportMaster::position, this, _1, _2, _3, _4));
 
-		reset ();
+		reset (true);
 	}
 }
 
@@ -284,18 +284,22 @@ MIDIClock_TransportMaster::start (Parser& /*parser*/, samplepos_t timestamp)
 	DEBUG_TRACE (DEBUG::MidiClock, string_compose ("MIDIClock_TransportMaster got start message at time %1 engine time %2 transport_sample %3\n", timestamp, ENGINE->sample_time(), _session->transport_sample()));
 
 	if (!_running) {
-		reset();
+		reset(true);
 		_running = true;
 		current.update (_session->transport_sample(), timestamp, 0);
 	}
 }
 
 void
-MIDIClock_TransportMaster::reset ()
+MIDIClock_TransportMaster::reset (bool with_position)
 {
 	DEBUG_TRACE (DEBUG::MidiClock, string_compose ("MidiClock Master reset(): calculated filter for period size %2\n", ENGINE->samples_per_cycle()));
 
-	current.update (_session->transport_sample(), 0, 0);
+	if (with_position) {
+		current.update (_session->transport_sample(), 0, 0);
+	} else {
+		current.update (0, 0, 0);
+	}
 
 	_running = false;
 	_current_delta = 0;
@@ -409,3 +413,11 @@ MIDIClock_TransportMaster::delta_string() const
 	}
 	return std::string(delta);
 }
+
+void
+MIDIClock_TransportMaster::unregister_port ()
+{
+	_midi_port.reset ();
+	TransportMaster::unregister_port ();
+}
+
diff --git a/libs/ardour/mtc_slave.cc b/libs/ardour/mtc_slave.cc
index f8afe66d93..28a51f96e7 100644
--- a/libs/ardour/mtc_slave.cc
+++ b/libs/ardour/mtc_slave.cc
@@ -587,3 +587,10 @@ MTC_TransportMaster::delta_string () const
 	}
 	return std::string(delta);
 }
+
+void
+MTC_TransportMaster::unregister_port ()
+{
+	_midi_port.reset ();
+	TransportMaster::unregister_port ();
+}
diff --git a/libs/ardour/session_ltc.cc b/libs/ardour/session_ltc.cc
index c2cd0ef681..094b502d52 100644
--- a/libs/ardour/session_ltc.cc
+++ b/libs/ardour/session_ltc.cc
@@ -184,7 +184,12 @@ Session::ltc_tx_send_time_code_for_cycle (samplepos_t start_sample, samplepos_t
 		return;
 	}
 
+	if (!TransportMasterManager::instance().current()) {
+		return;
+	}
+
 	SyncSource sync_src = TransportMasterManager::instance().current()->type();
+
 	if (engine().freewheeling() || !Config->get_send_ltc()
 	    /* TODO
 	     * decide which time-sources we can generated LTC from.
@@ -194,7 +199,7 @@ Session::ltc_tx_send_time_code_for_cycle (samplepos_t start_sample, samplepos_t
 	     || (config.get_external_sync() && sync_src == LTC)
 	     || (config.get_external_sync() && sync_src == MTC)
 	    */
-	     ||(config.get_external_sync() && sync_src == MIDIClock)
+	    ||(config.get_external_sync() && sync_src == MIDIClock)
 		) {
 		return;
 	}
diff --git a/libs/ardour/session_process.cc b/libs/ardour/session_process.cc
index 1cc9a03e02..bece787fd7 100644
--- a/libs/ardour/session_process.cc
+++ b/libs/ardour/session_process.cc
@@ -861,6 +861,7 @@ Session::process_event (SessionEvent* ev)
 		break;
 
 	case SessionEvent::SetTransportMaster:
+		cerr << "Process TMM current request\n";
 		TransportMasterManager::instance().set_current (ev->transport_master);
 		break;
 
diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc
index 8809ab388c..a8f9d6022f 100644
--- a/libs/ardour/session_state.cc
+++ b/libs/ardour/session_state.cc
@@ -4098,6 +4098,7 @@ Session::config_changed (std::string p, bool ours)
 		first_file_data_format_reset = false;
 
 	} else if (p == "external-sync") {
+		std::cerr << "param change, rss to " << TransportMasterManager::instance().current() << std::endl;
 		request_sync_source (TransportMasterManager::instance().current());
 	}  else if (p == "denormal-model") {
 		setup_fpu ();
diff --git a/libs/ardour/session_transport.cc b/libs/ardour/session_transport.cc
index a79de32c1e..7fe8f15e61 100644
--- a/libs/ardour/session_transport.cc
+++ b/libs/ardour/session_transport.cc
@@ -1975,7 +1975,7 @@ Session::transport_master() const
 bool
 Session::transport_master_is_external () const
 {
-	return config.get_external_sync();
+	return TransportMasterManager::instance().current() && config.get_external_sync();
 }
 
 void
diff --git a/libs/ardour/transport_master.cc b/libs/ardour/transport_master.cc
index 2564f87c5d..8799d51486 100644
--- a/libs/ardour/transport_master.cc
+++ b/libs/ardour/transport_master.cc
@@ -19,7 +19,9 @@
 
 #include <vector>
 
+#include "pbd/boost_debug.h"
 #include "pbd/debug.h"
+#include "pbd/i18n.h"
 
 #include "ardour/audioengine.h"
 #include "ardour/debug.h"
@@ -30,7 +32,6 @@
 #include "ardour/types_convert.h"
 #include "ardour/utils.h"
 
-#include "pbd/i18n.h"
 
 namespace ARDOUR {
 	namespace Properties {
@@ -83,6 +84,9 @@ TransportMaster::TransportMaster (SyncSource t, std::string const & name)
 
 TransportMaster::~TransportMaster()
 {
+	DEBUG_TRACE (DEBUG::Destruction, string_compose ("destroying transport master \"%1\" along with port %2\n", name(), (_port ? _port->name() : std::string ("no port"))));
+
+	unregister_port ();
 }
 
 bool
@@ -431,6 +435,15 @@ TransportMaster::display_name (bool sh) const
 	return S_("SyncSource|JACK");
 }
 
+void
+TransportMaster::unregister_port ()
+{
+	if (_port) {
+		AudioEngine::instance()->unregister_port (_port);
+		_port.reset ();
+	}
+}
+
 boost::shared_ptr<Port>
 TransportMasterViaMIDI::create_midi_port (std::string const & port_name)
 {
diff --git a/libs/ardour/transport_master_manager.cc b/libs/ardour/transport_master_manager.cc
index fafbf6042b..508d677a8a 100644
--- a/libs/ardour/transport_master_manager.cc
+++ b/libs/ardour/transport_master_manager.cc
@@ -22,6 +22,7 @@
 #include "ardour/session.h"
 #include "ardour/transport_master_manager.h"
 
+#include "pbd/boost_debug.cc"
 #include "pbd/i18n.h"
 
 #if __cplusplus > 199711L
@@ -54,19 +55,24 @@ int
 TransportMasterManager::set_default_configuration ()
 {
 	try {
+
+		clear ();
+
 		/* setup default transport masters. Most people will never need any
 		   others
 		*/
+
 		add (Engine, X_("JACK Transport"), false);
 		add (MTC, X_("MTC"), false);
 		add (LTC, X_("LTC"), false);
 		add (MIDIClock, X_("MIDI Clock"), false);
+
 	} catch (...) {
 		return -1;
 	}
 
 	_current_master = _transport_masters.back();
-
+	cerr << "default current master (back) is " << _current_master->name() << endl;
 	return 0;
 }
 
@@ -324,6 +330,7 @@ TransportMasterManager::add (SyncSource type, std::string const & name, bool rem
 		}
 
 		tm = TransportMaster::factory (type, name, removeable);
+		boost_debug_shared_ptr_mark_interesting (tm.get(), "tm");
 		ret = add_locked (tm);
 	}
 
@@ -382,9 +389,11 @@ TransportMasterManager::remove (std::string const & name)
 int
 TransportMasterManager::set_current_locked (boost::shared_ptr<TransportMaster> c)
 {
-	if (find (_transport_masters.begin(), _transport_masters.end(), c) == _transport_masters.end()) {
-		warning << string_compose (X_("programming error: attempt to use unknown transport master named \"%1\"\n"), c->name());
-		return -1;
+	if (c) {
+		if (find (_transport_masters.begin(), _transport_masters.end(), c) == _transport_masters.end()) {
+			warning << string_compose (X_("programming error: attempt to use unknown transport master \"%1\"\n"), c->name());
+			return -1;
+		}
 	}
 
 	_current_master = c;
@@ -393,7 +402,7 @@ TransportMasterManager::set_current_locked (boost::shared_ptr<TransportMaster> c
 
 	master_dll_initstate = 0;
 
-	DEBUG_TRACE (DEBUG::Slave, string_compose ("current transport master set to %1\n", c->name()));
+	DEBUG_TRACE (DEBUG::Slave, string_compose ("current transport master set to %1\n", (c ? c->name() : string ("none"))));
 
 	return 0;
 }
@@ -471,6 +480,7 @@ TransportMasterManager::clear ()
 {
 	{
 		Glib::Threads::RWLock::WriterLock lm (lock);
+		_current_master.reset ();
 		_transport_masters.clear ();
 	}
 
@@ -484,18 +494,18 @@ TransportMasterManager::set_state (XMLNode const & node, int version)
 
 	XMLNodeList const & children = node.children();
 
-
-	if (!children.empty()) {
-		_transport_masters.clear ();
-	}
-
 	{
 		Glib::Threads::RWLock::WriterLock lm (lock);
 
+		_current_master.reset ();
+		boost_debug_list_ptrs ();
+
+		_transport_masters.clear ();
 
 		for (XMLNodeList::const_iterator c = children.begin(); c != children.end(); ++c) {
 
 			boost::shared_ptr<TransportMaster> tm = TransportMaster::factory (**c);
+			boost_debug_shared_ptr_mark_interesting (tm.get(), "tm");
 
 			if (add_locked (tm)) {
 				continue;
@@ -548,3 +558,33 @@ TransportMasterManager::master_by_type (SyncSource src) const
 
 	return boost::shared_ptr<TransportMaster> ();
 }
+
+void
+TransportMasterManager::engine_stopped ()
+{
+	{
+		Glib::Threads::RWLock::ReaderLock lm (lock);
+
+		for (TransportMasters::const_iterator tm = _transport_masters.begin(); tm != _transport_masters.end(); ++tm) {
+			(*tm)->reset (false);
+		}
+	}
+}
+
+void
+TransportMasterManager::restart ()
+{
+	XMLNode* node;
+
+	if ((node = Config->transport_master_state()) != 0) {
+		if (TransportMasterManager::instance().set_state (*node, Stateful::loading_state_version)) {
+			error << _("Cannot restore transport master manager") << endmsg;
+			/* XXX now what? */
+		}
+	} else {
+		if (TransportMasterManager::instance().set_default_configuration ()) {
+			error << _("Cannot initialize transport master manager") << endmsg;
+			/* XXX now what? */
+		}
+	}
+}