Session::request_locate() takes a tri-valued second argument for "roll-after-locate"

This allows callers to defer logic about auto-play/current rolling state and more to TransportFSM where it
can be cnentralized and is less ambiguous

13 files changed, 104 insertions, 43 deletions

diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h
index 2b42c59973..402c55bf59 100644
--- a/libs/ardour/ardour/session.h
+++ b/libs/ardour/ardour/session.h
@@ -444,7 +444,7 @@ public:
 	void request_roll_at_and_return (samplepos_t start, samplepos_t return_to);
 	void request_bounded_roll (samplepos_t start, samplepos_t end);
 	void request_stop (bool abort = false, bool clear_state = false, TransportRequestSource origin = TRS_UI);
-	void request_locate (samplepos_t sample, bool with_roll = false, TransportRequestSource origin = TRS_UI);
+	void request_locate (samplepos_t sample, LocateTransportDisposition ltd = DoTheRightThing, TransportRequestSource origin = TRS_UI);
 
 	void request_play_loop (bool yn, bool leave_rolling = false);
 	bool get_play_loop () const { return play_loop; }
diff --git a/libs/ardour/ardour/session_event.h b/libs/ardour/ardour/session_event.h
index ddf7210617..b0af5596f6 100644
--- a/libs/ardour/ardour/session_event.h
+++ b/libs/ardour/ardour/session_event.h
@@ -91,6 +91,7 @@ public:
 	union {
 		bool second_yes_or_no;
 		double control_value;
+		LocateTransportDisposition locate_transport_disposition;
 	};
 
 	union {
diff --git a/libs/ardour/ardour/transport_fsm.h b/libs/ardour/ardour/transport_fsm.h
index 12550d92a7..54ceac2304 100644
--- a/libs/ardour/ardour/transport_fsm.h
+++ b/libs/ardour/ardour/transport_fsm.h
@@ -46,7 +46,7 @@ struct TransportFSM
 		EventType type;
 		union {
 			bool abort; /* for stop */
-			bool with_roll; /* for locate */
+			LocateTransportDisposition ltd; /* for locate */
 		};
 		union {
 			bool clear_state; /* for stop */
@@ -59,7 +59,7 @@ struct TransportFSM
 
 		Event (EventType t)
 			: type (t)
-			, with_roll (false)
+			, ltd (MustStop)
 			, with_flush (false)
 			, target (0)
 			, for_loop_end (false)
@@ -72,9 +72,9 @@ struct TransportFSM
 		{
 			assert (t == StopTransport);
 		}
-		Event (EventType t, samplepos_t pos, bool r, bool fl, bool lp, bool f4c)
+		Event (EventType t, samplepos_t pos, LocateTransportDisposition l, bool fl, bool lp, bool f4c)
 			: type (t)
-			, with_roll (r)
+			, ltd (l)
 			, with_flush (fl)
 			, target (pos)
 			, for_loop_end (lp)
@@ -175,6 +175,7 @@ struct TransportFSM
 	void defer (Event& ev);
 	void bad_transition (Event const &);
 	void set_roll_after (bool) const;
+	bool compute_should_roll (LocateTransportDisposition) const;
 };
 
 } /* end namespace ARDOUR */
diff --git a/libs/ardour/ardour/types.h b/libs/ardour/ardour/types.h
index c7453fcf1b..1504698896 100644
--- a/libs/ardour/ardour/types.h
+++ b/libs/ardour/ardour/types.h
@@ -799,6 +799,12 @@ enum OverwriteReason {
 	LoopChanged = 0x8,
 };
 
+enum LocateTransportDisposition {
+	MustRoll,
+	MustStop,
+	DoTheRightThing
+};
+
 typedef std::vector<CaptureInfo*> CaptureInfos;
 
 } // namespace ARDOUR
diff --git a/libs/ardour/disk_reader.cc b/libs/ardour/disk_reader.cc
index 73dede5a96..2ba941eab0 100644
--- a/libs/ardour/disk_reader.cc
+++ b/libs/ardour/disk_reader.cc
@@ -492,7 +492,9 @@ DiskReader::run (BufferSet& bufs, samplepos_t start_sample, samplepos_t end_samp
 		_need_butler = butler_required;
 	}
 
-	DEBUG_TRACE (DEBUG::Butler, string_compose ("%1 reader run, needs butler = %2\n", name(), _need_butler));
+	if (_need_butler) {
+		DEBUG_TRACE (DEBUG::Butler, string_compose ("%1 reader run, needs butler = %2\n", name(), _need_butler));
+	}
 }
 
 bool
@@ -1127,6 +1129,9 @@ DiskReader::refill_audio (Sample* sum_buffer, Sample* mixdown_buffer, float* gai
 
 	samplepos_t file_sample_tmp = ffa;
 
+	// int64_t before = g_get_monotonic_time ();
+	// int64_t elapsed;
+
 	for (chan_n = 0, i = c->begin(); i != c->end(); ++i, ++chan_n) {
 
 		ChannelInfo* chan (*i);
diff --git a/libs/ardour/enums.cc b/libs/ardour/enums.cc
index f9eda73b7e..d8a442150c 100644
--- a/libs/ardour/enums.cc
+++ b/libs/ardour/enums.cc
@@ -157,7 +157,8 @@ setup_enum_writer ()
 	TransportFSM::ButlerState _TransportFSM_ButlerState;
 	LoopFadeChoice _LoopFadeChooice;
 	TransportState _TransportState;
-
+	LocateTransportDisposition _LocateTransportDisposition;
+	
 #define REGISTER(e) enum_writer.register_distinct (typeid(e).name(), i, s); i.clear(); s.clear()
 #define REGISTER_BITS(e) enum_writer.register_bits (typeid(e).name(), i, s); i.clear(); s.clear()
 #define REGISTER_ENUM(e) i.push_back (e); s.push_back (#e)
@@ -833,6 +834,11 @@ setup_enum_writer ()
 	REGISTER_ENUM (TransportLooping);
 	REGISTER_ENUM (TransportStarting);
 	REGISTER (_TransportState);
+
+	REGISTER_ENUM (MustStop);
+	REGISTER_ENUM (MustRoll);
+	REGISTER_ENUM (DoTheRightThing);
+	REGISTER (_LocateTransportDisposition);
 }
 
 } /* namespace ARDOUR */
diff --git a/libs/ardour/mtc_slave.cc b/libs/ardour/mtc_slave.cc
index eded0e6dc2..c4af3f9bc7 100644
--- a/libs/ardour/mtc_slave.cc
+++ b/libs/ardour/mtc_slave.cc
@@ -431,7 +431,7 @@ MTC_TransportMaster::update_mtc_time (const MIDI::byte *msg, bool was_full, samp
 		DEBUG_TRACE (DEBUG::MTC, string_compose ("update_mtc_time: full TC %1 or outside window %2 MTC %3\n", was_full, outside_window (mtc_frame), mtc_frame));
 		_session->set_requested_return_sample (-1);
 		_session->request_transport_speed (0, TRS_MTC);
-		_session->request_locate (mtc_frame, false, TRS_MTC);
+		_session->request_locate (mtc_frame, MustStop, TRS_MTC);
 		update_mtc_status (MIDI::MTC_Stopped);
 		reset (false);
 		reset_window (mtc_frame);
diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc
index 62660c9fa2..454a9065ed 100644
--- a/libs/ardour/session.cc
+++ b/libs/ardour/session.cc
@@ -1443,7 +1443,7 @@ Session::auto_loop_changed (Location* location)
 				 */
 
 				loop_changing = true;
-				request_locate (location->start(), true);
+				request_locate (location->start(), MustRoll);
 
 			} else {
 
@@ -6083,9 +6083,9 @@ void
 Session::goto_end ()
 {
 	if (_session_range_location) {
-		request_locate (_session_range_location->end(), false);
+		request_locate (_session_range_location->end(), MustStop);
 	} else {
-		request_locate (0, false);
+		request_locate (0, MustStop);
 	}
 }
 
@@ -6093,9 +6093,9 @@ void
 Session::goto_start (bool and_roll)
 {
 	if (_session_range_location) {
-		request_locate (_session_range_location->start(), and_roll);
+		request_locate (_session_range_location->start(), and_roll ? MustRoll : DoTheRightThing);
 	} else {
-		request_locate (0, and_roll);
+		request_locate (0, and_roll ? MustRoll : DoTheRightThing);
 	}
 }
 
diff --git a/libs/ardour/session_export.cc b/libs/ardour/session_export.cc
index 3ec32e1d6f..c86d55e248 100644
--- a/libs/ardour/session_export.cc
+++ b/libs/ardour/session_export.cc
@@ -346,6 +346,6 @@ Session::finalize_audio_export (TransportRequestSource trs)
 	if (post_export_sync) {
 		config.set_external_sync (true);
 	} else {
-		request_locate (post_export_position, false, trs);
+		request_locate (post_export_position, MustStop, trs);
 	}
 }
diff --git a/libs/ardour/session_midi.cc b/libs/ardour/session_midi.cc
index 3048303c7d..c08abe95f5 100644
--- a/libs/ardour/session_midi.cc
+++ b/libs/ardour/session_midi.cc
@@ -306,7 +306,7 @@ Session::mmc_locate (MIDI::MachineControl &/*mmc*/, const MIDI::byte* mmc_tc)
 		mtcs->handle_locate (mmc_tc);
 	} else {
 		// cerr << "Locate without MTC slave\n";
-		request_locate (target_sample, false);
+		request_locate (target_sample, MustStop);
 	}
 }
 
diff --git a/libs/ardour/session_process.cc b/libs/ardour/session_process.cc
index bd8bd8d08b..a4429c3609 100644
--- a/libs/ardour/session_process.cc
+++ b/libs/ardour/session_process.cc
@@ -61,7 +61,7 @@ using namespace std;
 
 #define TFSM_EVENT(evtype) { _transport_fsm->enqueue (new TransportFSM::Event (evtype)); }
 #define TFSM_STOP(abort,clear) { _transport_fsm->enqueue (new TransportFSM::Event (TransportFSM::StopTransport,abort,clear)); }
-#define TFSM_LOCATE(target,roll,flush,loop,force) { _transport_fsm->enqueue (new TransportFSM::Event (TransportFSM::Locate,target,roll,flush,loop,force)); }
+#define TFSM_LOCATE(target,ltd,flush,loop,force) { _transport_fsm->enqueue (new TransportFSM::Event (TransportFSM::Locate,target,ltd,flush,loop,force)); }
 
 
 /** Called by the audio engine when there is work to be done with JACK.
@@ -859,7 +859,7 @@ Session::process_event (SessionEvent* ev)
 		if (play_loop) {
 			/* roll after locate, do not flush, set "for loop end" true
 			*/
-			TFSM_LOCATE (ev->target_sample, true, false, true, false);
+			TFSM_LOCATE (ev->target_sample, MustRoll, false, true, false);
 		}
 		remove = false;
 		del = false;
@@ -867,19 +867,19 @@ Session::process_event (SessionEvent* ev)
 
 	case SessionEvent::Locate:
 		/* args: do not roll after locate, clear state, not for loop, force */
-		TFSM_LOCATE (ev->target_sample, false, true, false, ev->yes_or_no);
+		TFSM_LOCATE (ev->target_sample, ev->locate_transport_disposition, true, false, ev->yes_or_no);
 		_send_timecode_update = true;
 		break;
 
 	case SessionEvent::LocateRoll:
 		/* args: roll after locate, clear state if not looping, not for loop, force */
-		TFSM_LOCATE (ev->target_sample, true, !play_loop, false, ev->yes_or_no);
+		TFSM_LOCATE (ev->target_sample, ev->locate_transport_disposition, !play_loop, false, ev->yes_or_no);
 		_send_timecode_update = true;
 		break;
 
 	case SessionEvent::Skip:
 		if (Config->get_skip_playback()) {
-			TFSM_LOCATE (ev->target_sample, true, true, false, false);
+			TFSM_LOCATE (ev->target_sample, MustRoll, true, false, false);
 			_send_timecode_update = true;
 		}
 		remove = false;
@@ -889,7 +889,7 @@ Session::process_event (SessionEvent* ev)
 	case SessionEvent::LocateRollLocate:
 		// locate is handled by ::request_roll_at_and_return()
 		_requested_return_sample = ev->target_sample;
-		TFSM_LOCATE (ev->target2_sample, true, true, false, false);
+		TFSM_LOCATE (ev->target2_sample, MustRoll, true, false, false);
 		_send_timecode_update = true;
 		break;
 
@@ -928,7 +928,7 @@ Session::process_event (SessionEvent* ev)
 
 	case SessionEvent::RangeLocate:
 		/* args: roll after locate, do flush, not with loop */
-		TFSM_LOCATE (ev->target_sample, true, true, false, false);
+		TFSM_LOCATE (ev->target_sample, MustRoll, true, false, false);
 		remove = false;
 		del = false;
 		break;
@@ -1126,7 +1126,7 @@ Session::follow_transport_master (pframes_t nframes)
 						const samplepos_t locate_target = master_transport_sample + wlp;
 						DEBUG_TRACE (DEBUG::Slave, string_compose ("JACK transport: jump to master position %1 by locating to %2\n", master_transport_sample, locate_target));
 						/* for JACK transport always stop after the locate (2nd argument == false) */
-						TFSM_LOCATE (locate_target, false, true, false, false);
+						TFSM_LOCATE (locate_target, MustStop, true, false, false);
 
 					} else {
 						DEBUG_TRACE (DEBUG::Slave, string_compose ("JACK Transport: locate already in process, sts = %1\n", master_transport_sample));
@@ -1166,7 +1166,7 @@ Session::follow_transport_master (pframes_t nframes)
 				/* note that for non-JACK transport masters, we assume that the transport state (rolling,stopped) after the locate
 				 * remains unchanged (2nd argument, "roll-after-locate")
 				 */
-				TFSM_LOCATE (master_transport_sample, master_speed != 0, true, false, false);
+				TFSM_LOCATE (master_transport_sample, (master_speed != 0) ? MustRoll : MustStop, true, false, false);
 			}
 
 			return true;
diff --git a/libs/ardour/session_transport.cc b/libs/ardour/session_transport.cc
index c62e42d5d9..9e5117f9d1 100644
--- a/libs/ardour/session_transport.cc
+++ b/libs/ardour/session_transport.cc
@@ -85,7 +85,7 @@ using namespace PBD;
 
 #define TFSM_EVENT(evtype) { _transport_fsm->enqueue (new TransportFSM::Event (evtype)); }
 #define TFSM_STOP(abort,clear) { _transport_fsm->enqueue (new TransportFSM::Event (TransportFSM::StopTransport,abort,clear)); }
-#define TFSM_LOCATE(target,roll,flush,loop,force) { _transport_fsm->enqueue (new TransportFSM::Event (TransportFSM::Locate,target,roll,flush,loop,force)); }
+#define TFSM_LOCATE(target,ltd,flush,loop,force) { _transport_fsm->enqueue (new TransportFSM::Event (TransportFSM::Locate,target,ltd,flush,loop,force)); }
 
 /* *****************************************************************************
  * REALTIME ACTIONS (to be called on state transitions)
@@ -470,7 +470,7 @@ Session::set_transport_speed (double speed, samplepos_t destination_sample, bool
 
 					/* jump to start and then roll from there */
 
-					request_locate (location->start(), true);
+					request_locate (location->start(), MustRoll);
 					return;
 				}
 			}
@@ -910,7 +910,7 @@ Session::request_stop (bool abort, bool clear_state, TransportRequestSource orig
 }
 
 void
-Session::request_locate (samplepos_t target_sample, bool with_roll, TransportRequestSource origin)
+Session::request_locate (samplepos_t target_sample, LocateTransportDisposition ltd, TransportRequestSource origin)
 {
 	if (synced_to_engine()) {
 		_engine.transport_locate (target_sample);
@@ -921,8 +921,27 @@ Session::request_locate (samplepos_t target_sample, bool with_roll, TransportReq
 		return;
 	}
 
-	SessionEvent *ev = new SessionEvent (with_roll ? SessionEvent::LocateRoll : SessionEvent::Locate, SessionEvent::Add, SessionEvent::Immediate, target_sample, 0, false);
-	DEBUG_TRACE (DEBUG::Transport, string_compose ("Request locate to %1\n", target_sample));
+	SessionEvent::Type type;
+
+	switch (ltd) {
+	case MustRoll:
+		type = SessionEvent::LocateRoll;
+		break;
+	case MustStop:
+		type = SessionEvent::Locate;
+		break;
+	case DoTheRightThing:
+		if (config.get_auto_play()) {
+			type = SessionEvent::LocateRoll;
+		} else {
+			type = SessionEvent::Locate;
+		}
+		break;
+	}
+
+	SessionEvent *ev = new SessionEvent (type, SessionEvent::Add, SessionEvent::Immediate, target_sample, 0, false);
+	ev->locate_transport_disposition = ltd;
+	DEBUG_TRACE (DEBUG::Transport, string_compose ("Request locate to %1 ltd = %2\n", target_sample, enum_2_string (ltd)));
 	queue_event (ev);
 }
 
@@ -954,7 +973,7 @@ Session::request_preroll_record_trim (samplepos_t rec_in, samplecnt_t preroll)
 	samplepos_t pos = std::max ((samplepos_t)0, rec_in - preroll);
 	_preroll_record_trim_len = preroll;
 	maybe_enable_record ();
-	request_locate (pos, true);
+	request_locate (pos, MustRoll);
 	set_requested_return_sample (rec_in);
 }
 
@@ -1367,7 +1386,7 @@ Session::non_realtime_stop (bool abort, int on_entry, bool& finished)
 		_have_captured = true;
 	}
 
-	DEBUG_TRACE (DEBUG::Transport, X_("Butler PTW: DS stop\n"));
+	DEBUG_TRACE (DEBUG::Transport, X_("Butler post-transport-work, non realtime stop\n"));
 
 	if (abort && did_record) {
 		/* no reason to save the session file when we remove sources
@@ -1612,10 +1631,10 @@ Session::set_play_loop (bool yn, bool change_transport_state)
 				   crude mechanism. Got a better idea?
 				*/
 				loop_changing = true;
-				TFSM_LOCATE (loc->start(), true, true, false, true);
+				TFSM_LOCATE (loc->start(), MustRoll, true, false, true);
 			} else if (!transport_rolling()) {
 				/* loop-is-mode: not rolling, just locate to loop start */
-				TFSM_LOCATE (loc->start(), false, true, false, true);
+				TFSM_LOCATE (loc->start(), MustStop, true, false, true);
 			}
 		}
 
diff --git a/libs/ardour/transport_fsm.cc b/libs/ardour/transport_fsm.cc
index bd3ac986b2..a06e2159c9 100644
--- a/libs/ardour/transport_fsm.cc
+++ b/libs/ardour/transport_fsm.cc
@@ -253,8 +253,8 @@ TransportFSM::process_event (Event& ev, bool already_deferred, bool& deferred)
 		break;
 
 	case Locate:
-		DEBUG_TRACE (DEBUG::TFSMEvents, string_compose ("locate, with roll = %1 flush = %2 target = %3 loop %4 force %5\n",
-		                                                ev.with_roll,
+		DEBUG_TRACE (DEBUG::TFSMEvents, string_compose ("locate, ltd = %1 flush = %2 target = %3 loop %4 force %5\n",
+		                                                enum_2_string (ev.ltd),
 		                                                ev.with_flush,
 		                                                ev.target,
 		                                                ev.for_loop_end,
@@ -277,7 +277,7 @@ TransportFSM::process_event (Event& ev, bool already_deferred, bool& deferred)
 				 * disk I/O is required - the loop is
 				 * automically present in buffers already.
 				 *
-				 * Note that ev.with_roll is ignored and
+				 * Note that ev.ltd is ignored and
 				 * assumed to be true because we're looping.
 				 */
 				transition (WaitingForLocate);
@@ -389,11 +389,12 @@ void
 TransportFSM::start_declick_for_locate (Event const & l)
 {
 	assert (l.type == Locate);
-	DEBUG_TRACE (DEBUG::TFSMEvents, string_compose ("start_declick_for_locate, crals %1 with_roll %2 speed %3 sral %4\n", (bool) current_roll_after_locate_status, l.with_roll, api->speed(), api->should_roll_after_locate()));
+	DEBUG_TRACE (DEBUG::TFSMEvents, string_compose ("start_declick_for_locate, crals %1 ltd %2 speed %3 sral %4\n", (bool) current_roll_after_locate_status,
+	                                                enum_2_string (l.ltd), api->speed(), api->should_roll_after_locate()));
 	_last_locate = l;
 
 	if (!current_roll_after_locate_status) {
-		set_roll_after (l.with_roll);
+		set_roll_after (compute_should_roll (l.ltd));
 	}
 	api->stop_transport (false, false);
 }
@@ -404,28 +405,50 @@ TransportFSM::start_locate_while_stopped (Event const & l) const
 	assert (l.type == Locate);
 	DEBUG_TRACE (DEBUG::TFSMEvents, "start_locate_while_stopped\n");
 
-	set_roll_after (l.with_roll || api->should_roll_after_locate());
+	set_roll_after (compute_should_roll (l.ltd));
 
 	api->locate (l.target, current_roll_after_locate_status.get(), l.with_flush, l.for_loop_end, l.force);
 }
 
+bool
+TransportFSM::compute_should_roll (LocateTransportDisposition ltd) const
+{
+	switch (ltd) {
+	case MustRoll:
+		return true;
+	case MustStop:
+		return false;
+	case DoTheRightThing:
+		if (rolling()) {
+			return true;
+		} else {
+			return api->should_roll_after_locate ();
+		}
+		break;
+	}
+	/*NOTREACHED*/
+	return true;
+}
+
 void
 TransportFSM::locate_for_loop (Event const & l)
 {
 	assert (l.type == Locate);
 	DEBUG_TRACE (DEBUG::TFSMEvents, string_compose ("locate_for_loop, wl = %1\n", l.for_loop_end));
-	set_roll_after (l.with_roll);
+
+	const bool should_roll = compute_should_roll (l.ltd);
+
 	_last_locate = l;
-	api->locate (l.target, l.with_roll, l.with_flush, l.for_loop_end, l.force);
+	api->locate (l.target, should_roll, l.with_flush, l.for_loop_end, l.force);
 }
 
 void
 TransportFSM::start_locate_after_declick () const
 {
 	DEBUG_TRACE (DEBUG::TFSMEvents, string_compose ("start_locate_after_declick, have crals ? %1 roll will be %2\n", (bool) current_roll_after_locate_status,
-	                                                current_roll_after_locate_status ? current_roll_after_locate_status.get() : _last_locate.with_roll));
+	                                                current_roll_after_locate_status ? current_roll_after_locate_status.get() : compute_should_roll (_last_locate.ltd)));
 
-	const bool roll = current_roll_after_locate_status ? current_roll_after_locate_status.get() : _last_locate.with_roll;
+	const bool roll = current_roll_after_locate_status ? current_roll_after_locate_status.get() : compute_should_roll (_last_locate.ltd);
 	api->locate (_last_locate.target, roll, _last_locate.with_flush, _last_locate.for_loop_end, _last_locate.force);
 }