* wrong calculation of frames_moved in Session::process_*, resulting in drift against any Slaves when transport speed != 1.0

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

6 files changed, 85 insertions, 9 deletions

diff --git a/libs/ardour/ardour/audio_diskstream.h b/libs/ardour/ardour/audio_diskstream.h
index f918655f98..0db44709f8 100644
--- a/libs/ardour/ardour/audio_diskstream.h
+++ b/libs/ardour/ardour/audio_diskstream.h
@@ -150,6 +150,48 @@ class AudioDiskstream : public Diskstream
 
 	XMLNode* deprecated_io_node;
 
+	/**
+	 * Calculate the playback distance during varispeed playback.
+	 * Important for Session::process to know exactly, how many frames
+	 * were passed by.
+	 */
+	static nframes_t calculate_varispeed_playback_distance(
+			nframes_t nframes, 
+			uint64_t& the_last_phase, 
+			uint64_t& the_phi,
+			uint64_t& the_target_phi)
+	{
+		// calculate playback distance in the same way 
+		// as in AudioDiskstream::process_varispeed_playback
+		uint64_t    phase = the_last_phase;
+		int64_t     phi_delta;
+		nframes_t   i = 0;
+		
+		const int64_t fractional_part_mask  = 0xFFFFFF;
+		const Sample  binary_scaling_factor = 16777216.0f;
+
+		if (the_phi != the_target_phi) {
+			phi_delta = ((int64_t)(the_target_phi - the_phi)) / nframes;
+		} else {
+			phi_delta = 0;
+		}
+
+		Sample fractional_phase_part;
+
+		i = 0;
+		phase = the_last_phase;
+
+		for (nframes_t outsample = 0; outsample < nframes; ++outsample) {
+			i = phase >> 24;
+			fractional_phase_part = (phase & fractional_part_mask) / binary_scaling_factor;
+			phase += the_phi + phi_delta;
+		}
+
+		the_last_phase = (phase & fractional_part_mask);
+		the_phi = the_target_phi;
+		return i; // + 1;
+	}
+	
   protected:
 	friend class Session;
 
diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h
index 9d12aa79b8..8b45dad958 100644
--- a/libs/ardour/ardour/session.h
+++ b/libs/ardour/ardour/session.h
@@ -1025,6 +1025,12 @@ class Session : public PBD::StatefulDestructible
 	bool                    _silent;
 	volatile double         _transport_speed;
 	double                  _last_transport_speed;
+	// fixed point transport speed for varispeed playback
+	uint64_t                phi; 
+	// fixed point target transport speed for varispeed playback when tempo changes
+	uint64_t                target_phi;
+	// fixed point phase for varispeed playback
+	uint64_t                phase;
 	bool                     auto_play_legal;
 	nframes_t               _last_slave_transport_frame;
 	nframes_t                maximum_output_latency;
diff --git a/libs/ardour/session_click.cc b/libs/ardour/session_click.cc
index fcbf1e1b03..fde698803c 100644
--- a/libs/ardour/session_click.cc
+++ b/libs/ardour/session_click.cc
@@ -53,7 +53,7 @@ Session::click (nframes_t start, nframes_t nframes, nframes_t offset)
 		return;
 	} 
 
-	const nframes_t end = start + (nframes_t)floor(nframes * _transport_speed);
+	const nframes_t end = start + nframes;
 
 	BufferSet& bufs = get_scratch_buffers(ChanCount(DataType::AUDIO, 1));
 	buf = bufs.get_audio(0).data();
diff --git a/libs/ardour/session_process.cc b/libs/ardour/session_process.cc
index 6247a4d654..dfb070b61c 100644
--- a/libs/ardour/session_process.cc
+++ b/libs/ardour/session_process.cc
@@ -271,11 +271,11 @@ void
 Session::process_with_events (nframes_t nframes)
 {
 	Event*         ev;
-	nframes_t this_nframes;
-	nframes_t end_frame;
-	nframes_t offset;
-	bool session_needs_butler = false;
-	nframes_t stop_limit;
+	nframes_t      this_nframes;
+	nframes_t      end_frame;
+	nframes_t      offset;
+	bool           session_needs_butler = false;
+	nframes_t      stop_limit;
 	long           frames_moved;
 	
 	/* make sure the auditioner is silent */
@@ -319,7 +319,17 @@ Session::process_with_events (nframes_t nframes)
 		return;
 	}
 
-	end_frame = _transport_frame + (nframes_t)abs(floor(nframes * _transport_speed));
+	/// TODO: Figure out what happens to phi and phase, if transport speed momentarily becomes
+	/// 1.0 eg. during the adjustments of a slave. If that is a bug, then AudioDiskstream::process
+	/// is very likely broken too
+	if (_transport_speed == 1.0) {
+		frames_moved = (long) nframes;
+	} else {		
+		frames_moved = (long) AudioDiskstream::
+			calculate_varispeed_playback_distance(nframes, phase, phi, target_phi); 
+	}
+
+	end_frame = _transport_frame + (nframes_t)frames_moved;
 
 	{
 		Event* this_event;
@@ -372,7 +382,6 @@ Session::process_with_events (nframes_t nframes)
 		while (nframes) {
 
 			this_nframes = nframes; /* real (jack) time relative */
-			frames_moved = (long) floor (_transport_speed * nframes); /* transport relative */
 
 			/* running an event, position transport precisely to its time */
 			if (this_event && this_event->action_frame <= end_frame && this_event->action_frame >= _transport_frame) {
@@ -836,7 +845,15 @@ Session::process_without_events (nframes_t nframes)
 
 	prepare_diskstreams ();
 	
-	frames_moved = (long) floor (_transport_speed * nframes);
+	/// TODO: Figure out what happens to phi and phase, if transport speed momentarily becomes
+	/// 1.0 eg. during the adjustments of a slave. If that is a bug, then AudioDiskstream::process
+	/// is very likely broken too
+	if (_transport_speed == 1.0) {
+		frames_moved = (long) nframes;
+	} else {		
+		frames_moved = (long) AudioDiskstream::
+			calculate_varispeed_playback_distance(nframes, phase, phi, target_phi); 
+	}
 
 	if (process_routes (nframes, offset)) {
 		no_roll (nframes, offset);
diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc
index a3bede221a..0fcd1c95a9 100644
--- a/libs/ardour/session_state.cc
+++ b/libs/ardour/session_state.cc
@@ -155,6 +155,8 @@ Session::first_stage_init (string fullpath, string snapshot_name)
 	insert_cnt = 0;
 	_transport_speed = 0;
 	_last_transport_speed = 0;
+	phi = (uint64_t) (0x1000000);
+	target_phi = phi;
 	auto_play_legal = false;
 	transport_sub_state = 0;
 	_transport_frame = 0;
diff --git a/libs/ardour/session_transport.cc b/libs/ardour/session_transport.cc
index 0649d16848..5b2c351b88 100644
--- a/libs/ardour/session_transport.cc
+++ b/libs/ardour/session_transport.cc
@@ -176,6 +176,9 @@ Session::realtime_stop (bool abort)
 	reset_slave_state ();
 
 	_transport_speed = 0;
+	phi = 0;
+	target_phi = 0;
+	phase = 0;
 
 	if (Config->get_use_video_sync()) {
 		waiting_for_sync_offset = true;
@@ -805,6 +808,8 @@ Session::set_transport_speed (double speed, bool abort)
 		return;
 	}
 
+	target_phi = (uint64_t) (0x1000000 * fabs(speed));
+	
 	if (speed > 0) {
 		speed = min (8.0, speed);
 	} else if (speed < 0) {
@@ -981,7 +986,11 @@ Session::start_transport ()
 	}
 
 	transport_sub_state |= PendingDeclickIn;
+	
 	_transport_speed = 1.0;
+	target_phi       = 0x1000000; // speed = 1
+	phi              = target_phi;
+	phase            = 0;
 
 	boost::shared_ptr<DiskstreamList> dsl = diskstreams.reader();
 	for (DiskstreamList::iterator i = dsl->begin(); i != dsl->end(); ++i) {