Declick before the end of seamless loops, not after the end, so that loops are rendered accurately (#4213, #4593).

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

libs/ardour/ardour/session.h

@@ -871,10 +871,12 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 	void destroy ();
 
 	enum SubState {
-		PendingDeclickIn   = 0x1,
+		PendingDeclickIn      = 0x1,  ///< pending de-click fade-in for start
-		PendingDeclickOut  = 0x2,
+		PendingDeclickOut     = 0x2,  ///< pending de-click fade-out for stop
-		StopPendingCapture = 0x4,
+		StopPendingCapture    = 0x4,
-		PendingLocate      = 0x20,
+		PendingLoopDeclickIn  = 0x8,  ///< pending de-click fade-in at the start of a loop
+		PendingLoopDeclickOut = 0x10, ///< pending de-click fade-out at the end of a loop
+		PendingLocate         = 0x20,
 	};
 
 	/* stuff used in process() should be close together to
@@ -999,7 +1001,16 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 			transport_sub_state &= ~PendingDeclickIn;
 			return 1;
 		} else if (transport_sub_state & PendingDeclickOut) {
+			/* XXX: not entirely sure why we don't clear this */
 			return -1;
+		} else if (transport_sub_state & PendingLoopDeclickOut) {
+			/* Return the declick out first ... */
+			transport_sub_state &= ~PendingLoopDeclickOut;
+			return -1;
+		} else if (transport_sub_state & PendingLoopDeclickIn) {
+			/* ... then the declick in on the next call */
+			transport_sub_state &= ~PendingLoopDeclickIn;
+			return 1;
 		} else {
 			return 0;
 		}
@@ -1089,6 +1100,7 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 
 	PBD::ScopedConnectionList loop_connections;
 	void             auto_loop_changed (Location *);
+	void             auto_loop_declick_range (Location *, framepos_t &, framepos_t &);
 
 	void first_stage_init (std::string path, std::string snapshot_name);
 	int  second_stage_init ();

libs/ardour/ardour/session_event.h

@@ -42,7 +42,8 @@ public:
 		/* only one of each of these events can be queued at any one time */
 
 		StopOnce,
-		AutoLoop
+		AutoLoop,
+		AutoLoopDeclick,
 	};
 
 	enum Action {

libs/ardour/session.cc

@@ -929,10 +929,25 @@ Session::auto_punch_changed (Location* location)
 	replace_event (SessionEvent::PunchOut, when_to_stop);
 }
 
+/** @param loc A loop location.
+ *  @param pos Filled in with the start time of the required fade-out (in session frames).
+ *  @param length Filled in with the length of the required fade-out.
+ */
+void
+Session::auto_loop_declick_range (Location* loc, framepos_t & pos, framepos_t & length)
+{
+	pos = max (loc->start(), loc->end() - 64);
+	length = loc->end() - pos;
+}
+
 void
 Session::auto_loop_changed (Location* location)
 {
 	replace_event (SessionEvent::AutoLoop, location->end(), location->start());
+	framepos_t dcp;
+	framecnt_t dcl;
+	auto_loop_declick_range (location, dcp, dcl);
+	replace_event (SessionEvent::AutoLoopDeclick, dcp, dcl);
 
 	if (transport_rolling() && play_loop) {
 
@@ -1010,6 +1025,10 @@ Session::set_auto_loop_location (Location* location)
 		loop_connections.drop_connections ();
 		existing->set_auto_loop (false, this);
 		remove_event (existing->end(), SessionEvent::AutoLoop);
+		framepos_t dcp;
+		framecnt_t dcl;
+		auto_loop_declick_range (existing, dcp, dcl);
+		remove_event (dcp, SessionEvent::AutoLoopDeclick);
 		auto_loop_location_changed (0);
 	}
 

libs/ardour/session_events.cc

@@ -173,6 +173,7 @@ SessionEventManager::merge_event (SessionEvent* ev)
 
 	switch (ev->type) {
 	case SessionEvent::AutoLoop:
+	case SessionEvent::AutoLoopDeclick:
 	case SessionEvent::StopOnce:
 		_clear_event_type (ev->type);
 		break;

libs/ardour/session_process.cc

@@ -1021,6 +1021,17 @@ Session::process_event (SessionEvent* ev)
 		del = false;
 		break;
 
+	case SessionEvent::AutoLoopDeclick:
+		if (play_loop) {
+			/* Request a declick fade-out and a fade-in; the fade-out will happen
+			   at the end of the loop, and the fade-in at the start.
+			*/
+			transport_sub_state |= (PendingLoopDeclickOut | PendingLoopDeclickIn);
+		}
+		remove = false;
+		del = false;
+		break;
+
 	case SessionEvent::Locate:
 		if (ev->yes_or_no) {
 			// cerr << "forced locate to " << ev->target_frame << endl;

libs/ardour/session_transport.cc

@@ -662,7 +662,7 @@ Session::check_declick_out ()
 
 	/* this is called after a process() iteration. if PendingDeclickOut was set,
 	   it means that we were waiting to declick the output (which has just been
-	   done) before doing something else. this is where we do that "something else".
+	   done) before maybe doing something else. this is where we do that "something else".
 
 	   note: called from the audio thread.
 	*/
@@ -676,6 +676,10 @@ Session::check_declick_out ()
 			stop_transport (pending_abort);
 			transport_sub_state &= ~(PendingDeclickOut|PendingLocate);
 		}
+
+	} else if (transport_sub_state & PendingLoopDeclickOut) {
+		/* Nothing else to do here; we've declicked, and the loop event will be along shortly */
+		transport_sub_state &= ~PendingLoopDeclickOut;
 	}
 }
 
@@ -684,6 +688,7 @@ Session::unset_play_loop ()
 {
 	play_loop = false;
 	clear_events (SessionEvent::AutoLoop);
+	clear_events (SessionEvent::AutoLoopDeclick);
 
 	// set all tracks to NOT use internal looping
 	boost::shared_ptr<RouteList> rl = routes.reader ();
@@ -744,10 +749,16 @@ Session::set_play_loop (bool yn)
 				}
 			}
 
-			/* put the loop event into the event list */
+			/* Put the delick and loop events in into the event list.  The declick event will
+			   cause a de-clicking fade-out just before the end of the loop, and it will also result
+			   in a fade-in when the loop restarts.  The AutoLoop event will peform the actual loop.
+			*/
 
-			SessionEvent* event = new SessionEvent (SessionEvent::AutoLoop, SessionEvent::Replace, loc->end(), loc->start(), 0.0f);
+			framepos_t dcp;
-			merge_event (event);
+			framecnt_t dcl;
+			auto_loop_declick_range (loc, dcp, dcl);
+			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.
@@ -841,8 +852,11 @@ Session::locate (framepos_t target_frame, bool with_roll, bool with_flush, bool
 		return;
 	}
 
-	if (_transport_speed) {
+	if (_transport_speed && !with_loop) {
-		/* schedule a declick. we'll be called again when its done */
+		/* 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.
+		*/
 
 		if (!(transport_sub_state & PendingDeclickOut)) {
 			transport_sub_state |= (PendingDeclickOut|PendingLocate);