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.

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;

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

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 */
 	}
 

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
 {

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;
 
+	std::cerr << "Engine stated, libardour inits, cnt = " << start_cnt << std::endl;
+
+	if (start_cnt == 0) {
+
+		/* find plugins */
+
+		ARDOUR::PluginManager::instance().refresh (!Config->get_discover_vst_on_start());
+	}
+
 	if ((node = Config->control_protocol_state()) != 0) {
 		ControlProtocolManager::instance().set_state (*node, 0 /* here: global-config state */);
 	}
 
-	if ((node = Config->transport_master_state()) != 0) {
+	if (start_cnt == 0) {
-		if (TransportMasterManager::instance().set_state (*node, Stateful::loading_state_version)) {
+		TransportMasterManager::instance().restart ();
-			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? */
-		}
 	}
-
-	/* find plugins */
-
-	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
-	 */
-
-	engine_startup_connection.disconnect ();
 }
 
 void
-	ARDOUR::cleanup ()
+ARDOUR::cleanup ()
 {
 	if (!libardour_initialized) {
 		return;

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;

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

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

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;
 	}

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;
 

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

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

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)
 {

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()) {
+	if (c) {
-		warning << string_compose (X_("programming error: attempt to use unknown transport master named \"%1\"\n"), c->name());
+		if (find (_transport_masters.begin(), _transport_masters.end(), c) == _transport_masters.end()) {
-		return -1;
+			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? */
+		}
+	}
+}