* made MidiClock_Slave conform more to to the Spec by starting transport

  on the first MidiClock Message after the start Message
* removed debugging output from session_process.c
* fixed bug: calculate a more sensible speed value on transport start
* fixed typos in Slave docs
* refactored MidiClock_Slave for better readability
* made MidiClock_Slave react to continue messages


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

4 files changed, 106 insertions, 63 deletions

diff --git a/libs/ardour/ardour/slave.h b/libs/ardour/ardour/slave.h
index ffeacd3017..db66e428be 100644
--- a/libs/ardour/ardour/slave.h
+++ b/libs/ardour/ardour/slave.h
@@ -39,13 +39,13 @@ class Session;
 /**
  * @class Slave
  * 
- * @brief The class Slave can be used to sync ardours tempo to an external source
+ * @brief The Slave interface can be used to sync ARDOURs tempo to an external source
  * like MTC, MIDI Clock, etc.
  * 
- * The name of the class may be a bit misleading: A subclass of Slave actually
- * acts as a master for Ardour, that means Ardour will try to follow the
+ * The name of the interface may be a bit misleading: A subclass of Slave actually
+ * acts as a time master for ARDOUR, that means ARDOUR will try to follow the
  * speed and transport position of the implementation of Slave.
- * Therefor it is rather that class, that makes Ardour a slave by connecting it
+ * Therefore it is rather that class, that makes ARDOUR a slave by connecting it
  * to its external time master.
  */
 class Slave {
@@ -62,8 +62,8 @@ class Slave {
 	 * <emph>position</emph> using a delay locked loop (DLL),
 	 * starting with the first given transport speed.
 	 * If the values of speed and position contradict each other,
-	 * ardour will always follow the position and disregard the speed.
-	 * Although, a correct speed is important so that ardour
+	 * ARDOUR will always follow the position and disregard the speed.
+	 * Although, a correct speed is important so that ARDOUR
 	 * can sync to the master time source quickly.
 	 * 
 	 * For background information on delay locked loops, 
@@ -85,7 +85,7 @@ class Slave {
 	 * <ul>
 	 * 	  <li>
 	 *       The first position value on transport start should be 0,
-	 *       otherwise ardour will try to locate to the new position 
+	 *       otherwise ARDOUR will try to locate to the new position 
 	 *       rather than move to it
 	 *    </li>
 	 * 	  <li>
@@ -95,7 +95,7 @@ class Slave {
 	 *    </li>
 	 *   <li>
 	 *     Slave::resolution() should be greater than the maximum distance of 
-	 *     ardours transport position to the slaves requested transport position.
+	 *     ARDOURs transport position to the slaves requested transport position.
 	 *   </li>
 	 * 	 <li>Slave::locked() should return true, otherwise Session::no_roll will be called</li>
 	 * 	 <li>Slave::starting() should be false, otherwise the transport will not move until it becomes true</li>	 *   
@@ -107,7 +107,7 @@ class Slave {
 	virtual bool speed_and_position (float& speed, nframes_t& position) = 0;
 	
 	/**
-	 * reports to ardour whether the Slave is currently synced to its external 
+	 * reports to ARDOUR whether the Slave is currently synced to its external 
 	 * time source.
 	 * 
 	 * @return - when returning false, the transport will stop rolling
@@ -115,15 +115,15 @@ class Slave {
 	virtual bool locked() const = 0;
 	
 	/**
-	 * reports to ardour whether the slave is in a sane state
+	 * reports to ARDOUR whether the slave is in a sane state
 	 * 
 	 * @return - when returning false, the transport will be stopped and the slave 
-	 * disconnected from ardour.
+	 * disconnected from ARDOUR.
 	 */
 	virtual bool ok() const = 0;
 	
 	/**
-	 * reports to ardour whether the slave is in the process of starting
+	 * reports to ARDOUR whether the slave is in the process of starting
 	 * to roll
 	 * 
 	 * @return - when returning false, transport will not move until this method returns true
@@ -131,19 +131,19 @@ class Slave {
 	virtual bool starting() const { return false; }
 	
 	/**
-	 * @return - the timing resolution of the Slave - If the distance of ardours transport
+	 * @return - the timing resolution of the Slave - If the distance of ARDOURs transport
 	 * to the slave becomes greater than the resolution, sound will stop
 	 */
 	virtual nframes_t resolution() const = 0;
 	
 	/**
-	 * @return - when returning true, ardour will wait for one second before transport
+	 * @return - when returning true, ARDOUR will wait for one second before transport
 	 * starts rolling
 	 */
 	virtual bool requires_seekahead () const = 0;
 	
 	/**
-	 * @return - when returning true, ardour will use transport speed 1.0 no matter what 
+	 * @return - when returning true, ARDOUR will use transport speed 1.0 no matter what 
 	 *           the slave returns
 	 */
 	virtual bool is_always_synced() const { return false; }
@@ -214,7 +214,7 @@ class MIDIClock_Slave : public Slave, public sigc::trackable {
 
 	bool locked() const;
 	bool ok() const;
-	bool starting() const { return false; }
+	bool starting() const;
 
 	nframes_t resolution() const;
 	bool requires_seekahead () const { return false; }
@@ -252,14 +252,18 @@ class MIDIClock_Slave : public Slave, public sigc::trackable {
 	void reset ();
 	void start (MIDI::Parser& parser, nframes_t timestamp);
 	void stop (MIDI::Parser& parser, nframes_t timestamp);
+	// we can't use continue because it is a C++ keyword
+	void contineu (MIDI::Parser& parser, nframes_t timestamp);
+	void calculate_one_ppqn_in_frames_at(nframes_t time);
 	void update_midi_clock (MIDI::Parser& parser, nframes_t timestamp);
 	void read_current (SafeTime *) const;
+	bool stop_if_no_more_clock_events(nframes_t& pos, nframes_t now, SafeTime& last);
 
 	/// whether transport should be rolling
 	bool _started;
 	
 	/// is true if the MIDI Start message has just been received until
-	/// the first call of speed_and_position(...)
+	/// the first MIDI Clock Event
 	bool _starting;
 };
 
diff --git a/libs/ardour/midi_clock_slave.cc b/libs/ardour/midi_clock_slave.cc
index e51fe76475..e6639446d5 100644
--- a/libs/ardour/midi_clock_slave.cc
+++ b/libs/ardour/midi_clock_slave.cc
@@ -67,23 +67,21 @@ MIDIClock_Slave::rebind (MIDI::Port& p)
 
 	port = &p;
 
+#ifdef DEBUG_MIDI_CLOCK		
 	std::cerr << "MIDIClock_Slave: connecting to port " << port->name() << std::endl;
+#endif
 
-	connections.push_back (port->input()->timing.connect (mem_fun (*this, &MIDIClock_Slave::update_midi_clock)));
-	connections.push_back (port->input()->start.connect  (mem_fun (*this, &MIDIClock_Slave::start)));
-	connections.push_back (port->input()->stop.connect   (mem_fun (*this, &MIDIClock_Slave::stop)));
+	connections.push_back (port->input()->timing.connect   (mem_fun (*this, &MIDIClock_Slave::update_midi_clock)));
+	connections.push_back (port->input()->start.connect    (mem_fun (*this, &MIDIClock_Slave::start)));
+	connections.push_back (port->input()->contineu.connect (mem_fun (*this, &MIDIClock_Slave::contineu)));
+	connections.push_back (port->input()->stop.connect     (mem_fun (*this, &MIDIClock_Slave::stop)));
 }
 
-void
-MIDIClock_Slave::update_midi_clock (Parser& parser, nframes_t timestamp)
-{	
-	nframes_t now = timestamp;
-
-	SafeTime last;
-	read_current (&last);
-		
-	const Tempo& current_tempo = session.tempo_map().tempo_at(now);
-	const Meter& current_meter = session.tempo_map().meter_at(now);
+void 
+MIDIClock_Slave::calculate_one_ppqn_in_frames_at(nframes_t time)
+{
+	const Tempo& current_tempo = session.tempo_map().tempo_at(time);
+	const Meter& current_meter = session.tempo_map().meter_at(time);
 	double frames_per_beat =
 		current_tempo.frames_per_beat(session.nominal_frame_rate(),
 		                              current_meter);
@@ -92,8 +90,25 @@ MIDIClock_Slave::update_midi_clock (Parser& parser, nframes_t timestamp)
 	double frames_per_quarter_note = frames_per_beat / quarter_notes_per_beat;
 
 	one_ppqn_in_frames = frames_per_quarter_note / double (ppqn);
+}
+
+void
+MIDIClock_Slave::update_midi_clock (Parser& parser, nframes_t timestamp)
+{	
+	nframes_t now = timestamp;
+
+	SafeTime last;
+	read_current (&last);
+	
+	if (_starting) {
+		assert(last.timestamp == 0);
+		// let ardour go after first MIDI Clock Event
+		_starting = false;
+	}
+		
+	calculate_one_ppqn_in_frames_at(now);
 	
-	// for the first MIDI clock event we dont have any past
+	// for the first MIDI clock event we don't have any past
 	// data, so we assume a sane tempo
 	if(last.timestamp == 0) {
 		current_midi_clock_frame_duration = one_ppqn_in_frames;
@@ -111,8 +126,9 @@ MIDIClock_Slave::update_midi_clock (Parser& parser, nframes_t timestamp)
 		average_midi_clock_frame_duration += accumulator[i];
 	average_midi_clock_frame_duration /= double(accumulator_size);
 	
+#ifdef DEBUG_MIDI_CLOCK		
 #ifdef WITH_JACK_MIDI
-	JACK_MidiPort *jack_port = dynamic_cast<JACK_MidiPort *>(port);
+	JACK_MidiPort* jack_port = dynamic_cast<JACK_MidiPort*>(port);
 	pthread_t process_thread_id = 0;
 	if(jack_port) {
 		process_thread_id = jack_port->get_process_thread();
@@ -126,6 +142,7 @@ MIDIClock_Slave::update_midi_clock (Parser& parser, nframes_t timestamp)
 	          << " reference: " << one_ppqn_in_frames
 	          << " average: " << average_midi_clock_frame_duration
 	          << std::endl;
+#endif // DEBUG_MIDI_CLOCK
 #endif // WITH_JACK_MIDI
 	
 	current.guard1++;
@@ -143,8 +160,10 @@ MIDIClock_Slave::start (Parser& parser, nframes_t timestamp)
 	
 	nframes_t now = timestamp;
 	
+#ifdef DEBUG_MIDI_CLOCK	
 	cerr << "MIDIClock_Slave got start message at time "  <<  now << " session time: " << session.engine().frame_time() << endl;
-
+#endif
+	
 	if(!locked()) {
 		cerr << "Did not start because not locked!" << endl;
 		return;
@@ -152,11 +171,10 @@ MIDIClock_Slave::start (Parser& parser, nframes_t timestamp)
 	
 	current_midi_clock_frame_duration = 0;
 	
-	session.request_transport_speed(one_ppqn_in_frames / double(average_midi_clock_frame_duration));
 	current.guard1++;
 	current.position = 0;
 	current_position = 0;	
-	current.timestamp = now;
+	current.timestamp = 0;
 	current.guard2++;
 	
 	_started = true;
@@ -164,10 +182,22 @@ MIDIClock_Slave::start (Parser& parser, nframes_t timestamp)
 }
 
 void
+MIDIClock_Slave::contineu (Parser& parser, nframes_t timestamp)
+{
+#ifdef DEBUG_MIDI_CLOCK	
+	std::cerr << "MIDIClock_Slave got continue message" << endl;
+#endif
+	start(parser, timestamp);
+}
+
+
+void
 MIDIClock_Slave::stop (Parser& parser, nframes_t timestamp)
 {
+#ifdef DEBUG_MIDI_CLOCK	
 	std::cerr << "MIDIClock_Slave got stop message" << endl;
-
+#endif
+	
 	current_midi_clock_frame_duration = 0;
 
 	current.guard1++;
@@ -210,31 +240,48 @@ MIDIClock_Slave::ok() const
 }
 
 bool
-MIDIClock_Slave::speed_and_position (float& speed, nframes_t& pos)
+MIDIClock_Slave::starting() const
 {
-	if (!_started) {
-		speed = 0.0;
-		pos = 0;
-		return true;
-	}
-		
-	nframes_t now = session.engine().frame_time();
-	SafeTime last;
-	read_current (&last);
+	return _starting;
+}
 
+bool
+MIDIClock_Slave::stop_if_no_more_clock_events(nframes_t& pos, nframes_t now, SafeTime& last)
+{
 	/* no timecode for 1/4 second ? conclude that its stopped */
 	if (last_inbound_frame && 
 	    now > last_inbound_frame && 
 	    now - last_inbound_frame > session.frame_rate() / 4) {
-		//cerr << "No MIDI Clock frames received for some time, stopping!" << endl;
+#ifdef DEBUG_MIDI_CLOCK			
+		cerr << "No MIDI Clock frames received for some time, stopping!" << endl;
+#endif		
 		pos = last.position;
 		session.request_locate (pos, false);
 		session.request_transport_speed (0);
 		this->stop(*port->input(), now);
 		reset();
+		return true;
+	} else {
 		return false;
 	}
+}
+
+bool
+MIDIClock_Slave::speed_and_position (float& speed, nframes_t& pos)
+{
+	if (!_started) {
+		speed = 0.0;
+		pos = 0;
+		return true;
+	}
+		
+	nframes_t now = session.engine().frame_time();
+	SafeTime last;
+	read_current (&last);
 
+	if (stop_if_no_more_clock_events(pos, now, last)) {
+		return false;
+	}
 	//cerr << " now: " << now << " last: " << last.timestamp;
 
 	// calculate speed
@@ -259,19 +306,14 @@ MIDIClock_Slave::speed_and_position (float& speed, nframes_t& pos)
    cerr << endl;
    */
    
-   // we want start on frame 0 on the first call after a MIDI start
-   if (_starting) {
-	   pos = 0;
-	   _starting = false;
-   }
-   
 	return true;
 }
 
 ARDOUR::nframes_t
 MIDIClock_Slave::resolution() const
 {
-	return (nframes_t) one_ppqn_in_frames * 24;
+	// one beat
+	return (nframes_t) one_ppqn_in_frames * ppqn;
 }
 
 void
diff --git a/libs/ardour/session_process.cc b/libs/ardour/session_process.cc
index c2e78df643..5ca77f02f1 100644
--- a/libs/ardour/session_process.cc
+++ b/libs/ardour/session_process.cc
@@ -743,7 +743,6 @@ Session::follow_slave (nframes_t nframes, nframes_t offset)
 
   noroll:
 	/* don't move at all */
-  cerr << "********* noroll" << endl;
 	no_roll (nframes, 0);
 	return false;
 }
diff --git a/libs/ardour/ticker.cc b/libs/ardour/ticker.cc
index 0012d47abf..4fe8fab520 100644
--- a/libs/ardour/ticker.cc
+++ b/libs/ardour/ticker.cc
@@ -22,8 +22,6 @@
 #include "ardour/session.h"
 #include "ardour/tempo.h"
 
-#define DEBUG_TICKER 0
-
 namespace ARDOUR
 {
 
@@ -60,7 +58,7 @@ void MidiClockTicker::transport_state_changed()
 {
 	float     speed     = _session->transport_speed();
 	nframes_t position  = _session->transport_frame();
-#if DEBUG_TICKER	
+#ifdef DEBUG_MIDI_CLOCK	
 	cerr << "Transport state change, speed:" << speed << "position:" << position<< " play loop " << _session->get_play_loop() << endl;
 #endif	
 	if (speed == 1.0f) {
@@ -96,7 +94,7 @@ void MidiClockTicker::transport_state_changed()
 
 void MidiClockTicker::position_changed(nframes_t position)
 {
-#if DEBUG_TICKER	
+#ifdef DEBUG_MIDI_CLOCK	
 	cerr << "Position changed:" << position << endl;
 #endif	
 	_last_tick = position;
@@ -107,7 +105,7 @@ void MidiClockTicker::transport_looped()
 	Location* loop_location = _session->locations()->auto_loop_location();
 	assert(loop_location);
 
-#if DEBUG_TICKER	
+#ifdef DEBUG_MIDI_CLOCK	
 	cerr << "Transport looped, position:" <<  _session->transport_frame()
 	     << " loop start " << loop_location->start( )
 	     << " loop end " << loop_location->end( )
@@ -134,7 +132,7 @@ void MidiClockTicker::tick(const nframes_t& transport_frames, const BBT_Time& tr
 		double next_tick = _last_tick + one_ppqn_in_frames(transport_frames);
 		nframes_t next_tick_offset = nframes_t(next_tick) - transport_frames;
 		
-#if DEBUG_TICKER	
+#ifdef DEBUG_MIDI_CLOCK	
 		cerr << "Transport:" << transport_frames 
 			 << ":Last tick time:" << _last_tick << ":" 
 			 << ":Next tick time:" << next_tick << ":" 
@@ -172,9 +170,9 @@ void MidiClockTicker::send_midi_clock_event(nframes_t offset)
 #ifdef WITH_JACK_MIDI
 	assert (MIDI::JACK_MidiPort::is_process_thread());
 #endif // WITH_JACK_MIDI
-#if DEBUG_TICKER	
+#ifdef DEBUG_MIDI_CLOCK	
 	cerr << "Tick with offset " << offset << endl;
-#endif // DEBUG_TICKER
+#endif // DEBUG_MIDI_CLOCK
 	static uint8_t _midi_clock_tick[1] = { MIDI_CMD_COMMON_CLOCK };
 	_midi_port->write(_midi_clock_tick, 1, offset);
 }