fix reset of transport speed when seamless looping; add a few comments and tidy-ups to related transport code

git-svn-id: svn://localhost/ardour2/branches/3.0@12818 d708f5d6-7413-0410-9779-e7cbd77b26cf

4 files changed, 61 insertions, 34 deletions

diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h
index 204474d611..f89c43ac16 100644
--- a/libs/ardour/ardour/session.h
+++ b/libs/ardour/ardour/session.h
@@ -329,7 +329,7 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 	void goto_start ();
 	void use_rf_shuttle_speed ();
 	void allow_auto_play (bool yn);
-	void request_transport_speed (double speed);
+        void request_transport_speed (double speed);
 	void request_transport_speed_nonzero (double);
 	void request_overwrite_buffer (Track *);
 	void adjust_playback_buffering();
@@ -1210,7 +1210,7 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 	void start_locate (framepos_t, bool with_roll, bool with_flush, bool with_loop=false, bool force=false);
 	void force_locate (framepos_t frame, bool with_roll = false);
 	void set_track_speed (Track *, double speed);
-	void set_transport_speed (double speed, bool abort = false, bool clear_state = false);
+        void set_transport_speed (double speed, bool abort = false, bool clear_state = false);
 	void stop_transport (bool abort = false, bool clear_state = false);
 	void start_transport ();
 	void realtime_stop (bool abort, bool clear_state);
diff --git a/libs/ardour/ardour/session_event.h b/libs/ardour/ardour/session_event.h
index d5a1b0c255..c917e4b3cf 100644
--- a/libs/ardour/ardour/session_event.h
+++ b/libs/ardour/ardour/session_event.h
@@ -85,7 +85,7 @@ public:
 
 	boost::shared_ptr<Region> region;
 
-	SessionEvent (Type t, Action a, framepos_t when, framepos_t where, double spd, bool yn = false, bool yn2 = false)
+    SessionEvent (Type t, Action a, framepos_t when, framepos_t where, double spd, bool yn = false, bool yn2 = false)
 		: type (t)
 		, action (a)
 		, action_frame (when)
diff --git a/libs/ardour/session_process.cc b/libs/ardour/session_process.cc
index 585d0927f2..0c3b0a57ea 100644
--- a/libs/ardour/session_process.cc
+++ b/libs/ardour/session_process.cc
@@ -1015,6 +1015,9 @@ Session::process_event (SessionEvent* ev)
 
 	case SessionEvent::AutoLoop:
 		if (play_loop) {
+			/* roll after locate, do not flush, set "with loop"
+			   true only if we are seamless looping
+			*/
 			start_locate (ev->target_frame, true, false, Config->get_seamless_loop());
 		}
 		remove = false;
@@ -1034,10 +1037,10 @@ Session::process_event (SessionEvent* ev)
 
 	case SessionEvent::Locate:
 		if (ev->yes_or_no) {
-			// cerr << "forced locate to " << ev->target_frame << endl;
+			/* args: do not roll after locate, do flush, not with loop */
 			locate (ev->target_frame, false, true, false);
 		} else {
-			// cerr << "soft locate to " << ev->target_frame << endl;
+			/* args: do not roll after locate, do flush, not with loop */
 			start_locate (ev->target_frame, false, true, false);
 		}
 		_send_timecode_update = true;
@@ -1045,10 +1048,10 @@ Session::process_event (SessionEvent* ev)
 
 	case SessionEvent::LocateRoll:
 		if (ev->yes_or_no) {
-			// cerr << "forced locate to+roll " << ev->target_frame << endl;
+			/* args: roll after locate, do flush, not with loop */
 			locate (ev->target_frame, true, true, false);
 		} else {
-			// cerr << "soft locate to+roll " << ev->target_frame << endl;
+			/* args: roll after locate, do flush, not with loop */
 			start_locate (ev->target_frame, true, true, false);
 		}
 		_send_timecode_update = true;
@@ -1101,6 +1104,7 @@ Session::process_event (SessionEvent* ev)
 		break;
 
 	case SessionEvent::RangeLocate:
+		/* args: roll after locate, do flush, not with loop */
 		start_locate (ev->target_frame, true, true, false);
 		remove = false;
 		del = false;
diff --git a/libs/ardour/session_transport.cc b/libs/ardour/session_transport.cc
index ace5e8e3e1..20ac737575 100644
--- a/libs/ardour/session_transport.cc
+++ b/libs/ardour/session_transport.cc
@@ -760,8 +760,9 @@ Session::set_play_loop (bool yn)
 			merge_event (new SessionEvent (SessionEvent::AutoLoopDeclick, SessionEvent::Replace, dcp, dcl, 0.0f));
 			merge_event (new SessionEvent (SessionEvent::AutoLoop, SessionEvent::Replace, loc->end(), loc->start(), 0.0f));
 
-			/* locate to start of loop and roll. If doing seamless loop, force a
-			   locate+buffer refill even if we are positioned there already.
+			/* locate to start of loop and roll. 
+
+			   args: positition, roll=true, flush=true, with_loop=false, force buffer refill if seamless looping
 			*/
 
 			start_locate (loc->start(), true, true, false, Config->get_seamless_loop());
@@ -837,13 +838,21 @@ Session::micro_locate (framecnt_t distance)
 
 /** @param with_mmc true to send a MMC locate command when the locate is done */
 void
-Session::locate (framepos_t target_frame, bool with_roll, bool with_flush, bool with_loop, bool force, bool with_mmc)
+Session::locate (framepos_t target_frame, bool with_roll, bool with_flush, bool for_seamless_loop, bool force, bool with_mmc)
 {
-	if (actively_recording() && !with_loop) {
+	/* Locates for seamless looping are fairly different from other
+	 * locates. They assume that the diskstream buffers for each track
+	 * already have the correct data in them, and thus there is no need to
+	 * actually tell the tracks to locate. What does need to be done,
+	 * though, is all the housekeeping that is associated with non-linear
+	 * changes in the value of _transport_frame. 
+	 */
+	 
+	if (actively_recording() && !for_seamless_loop) {
 		return;
 	}
 
-	if (!force && _transport_frame == target_frame && !loop_changing && !with_loop) {
+	if (!force && _transport_frame == target_frame && !loop_changing && !for_seamless_loop) {
 		if (with_roll) {
 			set_transport_speed (1.0, false);
 		}
@@ -852,10 +861,10 @@ Session::locate (framepos_t target_frame, bool with_roll, bool with_flush, bool
 		return;
 	}
 
-	if (_transport_speed && !with_loop) {
+	if (_transport_speed && !for_seamless_loop) {
 		/* Schedule a declick.  We'll be called again when its done.
 		   We only do it this way for ordinary locates, not those
-		   due to loops.
+		   due to **seamless** loops.
 		*/
 
 		if (!(transport_sub_state & PendingDeclickOut)) {
@@ -883,18 +892,21 @@ Session::locate (framepos_t target_frame, bool with_roll, bool with_flush, bool
          *
 	 */
 
-	if (transport_rolling() && (!auto_play_legal || !config.get_auto_play()) && !with_roll && !(synced_to_jack() && play_loop)) {
+	bool transport_was_stopped = !transport_rolling();
+
+	if (transport_was_stopped && (!auto_play_legal || !config.get_auto_play()) && !with_roll && !(synced_to_jack() && play_loop)) {
 		realtime_stop (false, true); // XXX paul - check if the 2nd arg is really correct
+		transport_was_stopped = true;
 	} else {
 		/* otherwise tell the world that we located */
 		realtime_locate ();
 	}
 
-	if (force || !with_loop || loop_changing) {
+	if (force || !for_seamless_loop || loop_changing) {
 
 		PostTransportWork todo = PostTransportLocate;
 
-		if (with_roll) {
+		if (with_roll && transport_was_stopped) {
 			todo = PostTransportWork (todo | PostTransportRoll);
 		}
 
@@ -931,27 +943,39 @@ Session::locate (framepos_t target_frame, bool with_roll, bool with_flush, bool
 
 	/* cancel looped playback if transport pos outside of loop range */
 	if (play_loop) {
+
 		Location* al = _locations->auto_loop_location();
 
-		if (al && (_transport_frame < al->start() || _transport_frame > al->end())) {
-			// cancel looping directly, this is called from event handling context
-			set_play_loop (false);
-		}
-		else if (al && _transport_frame == al->start()) {
-			if (with_loop) {
-				// this is only necessary for seamless looping
+		if (al) {
+			if (_transport_frame < al->start() || _transport_frame > al->end()) {
 
-				boost::shared_ptr<RouteList> rl = routes.reader();
-				for (RouteList::iterator i = rl->begin(); i != rl->end(); ++i) {
-					boost::shared_ptr<Track> tr = boost::dynamic_pointer_cast<Track> (*i);
-					if (tr && tr->record_enabled ()) {
-						// tell it we've looped, so it can deal with the record state
-						tr->transport_looped(_transport_frame);
+				// located outside the loop: cancel looping directly, this is called from event handling context
+
+				set_play_loop (false);
+				
+			} else if (_transport_frame == al->start()) {
+
+				// located to start of loop - this is looping, basically
+
+				if (for_seamless_loop) {
+
+					// this is only necessary for seamless looping
+					
+					boost::shared_ptr<RouteList> rl = routes.reader();
+
+					for (RouteList::iterator i = rl->begin(); i != rl->end(); ++i) {
+						boost::shared_ptr<Track> tr = boost::dynamic_pointer_cast<Track> (*i);
+
+						if (tr && tr->record_enabled ()) {
+							// tell it we've looped, so it can deal with the record state
+							tr->transport_looped (_transport_frame);
+						}
 					}
 				}
+
+				have_looped = true;
+				TransportLooped(); // EMIT SIGNAL
 			}
-			have_looped = true;
-			TransportLooped(); // EMIT SIGNAL
 		}
 	}
 
@@ -1190,7 +1214,7 @@ Session::start_transport ()
 	transport_sub_state |= PendingDeclickIn;
 
 	_transport_speed = 1.0;
-	_target_transport_speed = 1.0;
+	_target_transport_speed = _transport_speed;
 
 	boost::shared_ptr<RouteList> rl = routes.reader();
 	for (RouteList::iterator i = rl->begin(); i != rl->end(); ++i) {
@@ -1237,7 +1261,6 @@ Session::post_transport ()
 
 		if (((!config.get_external_sync() && (auto_play_legal && config.get_auto_play())) && !_exporting) || (ptw & PostTransportRoll)) {
 			start_transport ();
-
 		} else {
 			transport_sub_state = 0;
 		}