fixes to get MTC (and probably MIDI clock) slaving working again

incoming MIDI data has to be parsed EVERY process cycle, not just when Slave::speed_and_position() is called.
The private MIDI::Parser owned by the MTC and MClck slaves was irrelevant, since the port has its own.
See comments in midi_port.h on the strangled inheritance heirarchy.

libs/ardour/ardour/midi_port.h

@@ -21,6 +21,8 @@
 #ifndef __ardour_midi_port_h__
 #define __ardour_midi_port_h__
 
+#include "midi++/parser.h"
+
 #include "ardour/port.h"
 #include "ardour/midi_buffer.h"
 #include "ardour/midi_state_tracker.h"
@@ -56,6 +58,9 @@ class MidiPort : public Port {
 
 	MidiBuffer& get_midi_buffer (pframes_t nframes);
 
+        void set_always_parse (bool yn);
+        MIDI::Parser& self_parser() { return _self_parser; }
+
   protected:
         friend class PortManager;
 
@@ -66,6 +71,20 @@ class MidiPort : public Port {
 	bool        _has_been_mixed_down;
 	bool        _resolve_required;
 	bool        _input_active;
+        bool        _always_parse;
+
+    /* Naming this is tricky. AsyncMIDIPort inherits (for now, aug 2013) from
+     * both MIDI::Port, which has _parser, and this (ARDOUR::MidiPort). We
+     * need parsing support in this object, independently of what the
+     * MIDI::Port/AsyncMIDIPort stuff does. Rather than risk errors coming
+     * from not explicitly naming which _parser we want, we will call this
+     * _self_parser for now.
+     *
+     * Ultimately, MIDI::Port should probably go away or be fully integrated
+     * into this object, somehow.
+     */
+
+        MIDI::Parser _self_parser;
 
 	void resolve_notes (void* buffer, MidiBuffer::TimeType when);
 };

libs/ardour/ardour/slave.h

@@ -64,15 +64,6 @@ class Slave {
 	Slave() { }
 	virtual ~Slave() {}
 
-        /** The slave should read any incoming information in this method
-	 *  and use it adjust its current idea of reality. If no such
-	 *  processing is required, it does need to be implemented.
-	 *
-	 * @param nframes specifies the number of frames-worth of data that
-	 * can be read from any ports used by the slave.
-	 */
-         virtual int process (pframes_t) { return 0; }
-
 	/**
 	 * This is the most important function to implement:
 	 * Each process cycle, Session::follow_slave will call this method.
@@ -263,7 +254,6 @@ class MTC_Slave : public TimecodeSlave {
 	~MTC_Slave ();
 
 	void rebind (MidiPort&);
-        int process (pframes_t);
 	bool speed_and_position (double&, framepos_t&);
 
 	bool locked() const;
@@ -282,7 +272,6 @@ class MTC_Slave : public TimecodeSlave {
   private:
 	Session&    session;
 	MidiPort*   port;
-        MIDI::Parser    parser;
 	PBD::ScopedConnectionList port_connections;
 	PBD::ScopedConnection     config_connection;
 	bool        can_notify_on_unknown_rate;
@@ -420,7 +409,6 @@ class MIDIClock_Slave : public Slave {
 	~MIDIClock_Slave ();
 
 	void rebind (MidiPort&);
-        int process (pframes_t);
 	bool speed_and_position (double&, framepos_t&);
 
 	bool locked() const;
@@ -436,8 +424,6 @@ class MIDIClock_Slave : public Slave {
 
   protected:
 	ISlaveSessionProxy* session;
-	MidiPort* port;
-        MIDI::Parser parser;
 	PBD::ScopedConnectionList port_connections;
 
 	/// pulses per quarter note for one MIDI clock frame (default 24)

libs/ardour/midi_clock_slave.cc

@@ -50,13 +50,6 @@ MIDIClock_Slave::MIDIClock_Slave (Session& s, MidiPort& p, int ppqn)
 	session = (ISlaveSessionProxy *) new SlaveSessionProxy(s);
 	rebind (p);
 	reset ();
-
-	parser.timing.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::update_midi_clock, this, _1, _2));
-	parser.start.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::start, this, _1, _2));
-	parser.contineu.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::contineu, this, _1, _2));
-	parser.stop.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::stop, this, _1, _2));
-	parser.position.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::position, this, _1, _2, 3));
-
 }
 
 MIDIClock_Slave::MIDIClock_Slave (ISlaveSessionProxy* session_proxy, int ppqn)
@@ -72,33 +65,19 @@ MIDIClock_Slave::~MIDIClock_Slave()
 	delete session;
 }
 
-int
-MIDIClock_Slave::process (pframes_t nframes)
-{
-	MidiBuffer& mb (port->get_midi_buffer (nframes));
-
-	/* dump incoming MIDI to parser */
-
-	for (MidiBuffer::iterator b = mb.begin(); b != mb.end(); ++b) {
-		uint8_t* buf = (*b).buffer();
-
-		parser.set_timestamp ((*b).time());
-
-		uint32_t limit = (*b).size();
-
-		for (size_t n = 0; n < limit; ++n) {
-			parser.scanner (buf[n]);
-		}
-	}
-
-	return 0;
-}
-
 void
-MIDIClock_Slave::rebind (MidiPort& p)
+MIDIClock_Slave::rebind (MidiPort& port)
 {
-	port = &p;
+	DEBUG_TRACE (DEBUG::MidiClock, string_compose ("MIDIClock_Slave: connecting to port %1\n", port.name()));
-	DEBUG_TRACE (DEBUG::MidiClock, string_compose ("MIDIClock_Slave: connecting to port %1\n", port->name()));
+
+	port_connections.drop_connections ();
+
+	port.self_parser().timing.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::update_midi_clock, this, _1, _2));
+	port.self_parser().start.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::start, this, _1, _2));
+	port.self_parser().contineu.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::contineu, this, _1, _2));
+	port.self_parser().stop.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::stop, this, _1, _2));
+	port.self_parser().position.connect_same_thread (port_connections, boost::bind (&MIDIClock_Slave::position, this, _1, _2, 3));
+
 }
 
 void

libs/ardour/midi_port.cc

@@ -33,6 +33,7 @@ MidiPort::MidiPort (const std::string& name, PortFlags flags)
 	, _has_been_mixed_down (false)
 	, _resolve_required (false)
 	, _input_active (true)
+	, _always_parse (false)
 {
 	_buffer = new MidiBuffer (AudioEngine::instance()->raw_buffer_size (DataType::MIDI));
 }
@@ -45,6 +46,8 @@ MidiPort::~MidiPort()
 void
 MidiPort::cycle_start (pframes_t nframes)
 {
+	framepos_t now = AudioEngine::instance()->sample_time_at_cycle_start();
+
 	Port::cycle_start (nframes);
 
 	_buffer->clear ();
@@ -52,6 +55,24 @@ MidiPort::cycle_start (pframes_t nframes)
 	if (sends_output ()) {
 		port_engine.midi_clear (port_engine.get_buffer (_port_handle, nframes));
 	}
+
+	if (_always_parse) {
+		MidiBuffer& mb (get_midi_buffer (nframes));
+
+		/* dump incoming MIDI to parser */
+		
+		for (MidiBuffer::iterator b = mb.begin(); b != mb.end(); ++b) {
+			uint8_t* buf = (*b).buffer();
+			
+			_self_parser.set_timestamp (now + (*b).time());
+			
+			uint32_t limit = (*b).size();
+			
+			for (size_t n = 0; n < limit; ++n) {
+				_self_parser.scanner (buf[n]);
+			}
+		}
+	}
 }
 
 MidiBuffer &
@@ -72,8 +93,6 @@ MidiPort::get_midi_buffer (pframes_t nframes)
 			   into our MidiBuffer
 			*/
 
-			cerr << "grabbing " << event_count << " events\n";
-
 			for (pframes_t i = 0; i < event_count; ++i) {
 				
 				pframes_t timestamp;
@@ -217,3 +236,9 @@ MidiPort::set_input_active (bool yn)
 {
 	_input_active = yn;
 }
+
+void
+MidiPort::set_always_parse (bool yn)
+{
+	_always_parse = yn;
+}

libs/ardour/midiport_manager.cc

@@ -107,6 +107,15 @@ MidiPortManager::create_ports ()
 	_midi_clock_input_port = boost::dynamic_pointer_cast<MidiPort> (p);
 	p = AudioEngine::instance()->register_output_port (DataType::MIDI, _("MIDI Clock out"));
 	_midi_clock_output_port= boost::dynamic_pointer_cast<MidiPort> (p);
+
+	/* These ports all need their incoming data handled in
+	 * Port::cycle_start() and so ...
+	 */
+
+	_mtc_input_port->set_always_parse (true);
+	_mtc_output_port->set_always_parse (true);
+	_midi_clock_input_port->set_always_parse (true);
+	_midi_clock_output_port->set_always_parse (true);
 }
 
 void

libs/ardour/mtc_slave.cc

@@ -73,10 +73,9 @@ MTC_Slave::MTC_Slave (Session& s, MidiPort& p)
 	parse_timecode_offset();
 	reset (true);
 
-	parser.mtc_time.connect_same_thread (port_connections,  boost::bind (&MTC_Slave::update_mtc_time, this, _1, _2, _3));
+	port->self_parser().mtc_time.connect_same_thread (port_connections,  boost::bind (&MTC_Slave::update_mtc_time, this, _1, _2, _3));
-	parser.mtc_qtr.connect_same_thread (port_connections, boost::bind (&MTC_Slave::update_mtc_qtr, this, _1, _2, _3));
+	port->self_parser().mtc_qtr.connect_same_thread (port_connections, boost::bind (&MTC_Slave::update_mtc_qtr, this, _1, _2, _3));
-	parser.mtc_status.connect_same_thread (port_connections, boost::bind (&MTC_Slave::update_mtc_status, this, _1));
+	port->self_parser().mtc_status.connect_same_thread (port_connections, boost::bind (&MTC_Slave::update_mtc_status, this, _1));
-	
 }
 
 MTC_Slave::~MTC_Slave()
@@ -99,34 +98,6 @@ MTC_Slave::~MTC_Slave()
 	}
 }
 
-int
-MTC_Slave::process (pframes_t nframes)
-{
-	MidiBuffer& mb (port->get_midi_buffer (nframes));
-
-	/* dump incoming MIDI to parser */
-
-	cerr << "\n\n\n<<<< MTC slave, process " << mb.size() << endl;
-
-	for (MidiBuffer::iterator b = mb.begin(); b != mb.end(); ++b) {
-		uint8_t* buf = (*b).buffer();
-
-		parser.set_timestamp ((*b).time());
-
-		uint32_t limit = (*b).size();
-
-		cerr << "msg of " << limit << " bytes\n";
-
-		for (size_t n = 0; n < limit; ++n) {
-			parser.scanner (buf[n]);
-		}
-	}
-
-	cerr << ">>>> MTC slave, done processing\n\n\n";
-
-	return 0;
-}
-
 void
 MTC_Slave::rebind (MidiPort& p)
 {
@@ -185,7 +156,8 @@ MTC_Slave::outside_window (framepos_t pos) const
 bool
 MTC_Slave::locked () const
 {
-	return parser.mtc_locked() && last_inbound_frame !=0 && engine_dll_initstate !=0;
+	DEBUG_TRACE (DEBUG::MTC, string_compose ("locked ? %1 last %2 initstate %3\n", port->self_parser().mtc_locked(), last_inbound_frame, engine_dll_initstate));
+	return port->self_parser().mtc_locked() && last_inbound_frame !=0 && engine_dll_initstate !=0;
 }
 
 bool
@@ -286,7 +258,6 @@ MTC_Slave::init_mtc_dll(framepos_t tme, double qtr)
 	DEBUG_TRACE (DEBUG::MTC, string_compose ("[re-]init MTC DLL %1 %2 %3\n", t0, t1, e2));
 }
 
-
 /* called from MIDI parser */
 void
 MTC_Slave::update_mtc_qtr (Parser& /*p*/, int which_qtr, framepos_t now)
@@ -453,7 +424,7 @@ MTC_Slave::update_mtc_time (const byte *msg, bool was_full, framepos_t now)
 						 now, timecode, mtc_frame, was_full, speedup_due_to_tc_mismatch));
 
 	if (was_full || outside_window (mtc_frame)) {
-		DEBUG_TRACE (DEBUG::MTC, string_compose ("update_mtc_time: full TC or outside window. - TID:%1\n", ::pthread_self()));
+		DEBUG_TRACE (DEBUG::MTC, string_compose ("update_mtc_time: full TC %1 or outside window %2\n", was_full, outside_window (mtc_frame)));
 		session.request_locate (mtc_frame, false);
 		session.request_transport_speed (0);
 		update_mtc_status (MIDI::MTC_Stopped);
@@ -477,7 +448,7 @@ MTC_Slave::update_mtc_time (const byte *msg, bool was_full, framepos_t now)
 		DEBUG_TRACE (DEBUG::MTC, string_compose ("new mtc_frame: %1 | MTC-FpT: %2 A3-FpT:%3\n",
 							 mtc_frame, (4.0*qtr), session.frames_per_timecode_frame()));
 
-		switch (parser.mtc_running()) {
+		switch (port->self_parser().mtc_running()) {
 		case MTC_Backward:
 			mtc_frame -= mtc_off;
 			qtr *= -1.0;
@@ -557,9 +528,10 @@ MTC_Slave::reset_window (framepos_t root)
 	   of acceptable MTC frames wide open. otherwise, shrink it down to just 2 video frames
 	   ahead of the window root (taking direction into account).
 	*/
+
 	framecnt_t const d = (quarter_frame_duration * 4 * frame_tolerance);
 
-	switch (parser.mtc_running()) {
+	switch (port->self_parser().mtc_running()) {
 	case MTC_Forward:
 		window_begin = root;
 		transport_direction = 1;
@@ -582,7 +554,7 @@ MTC_Slave::reset_window (framepos_t root)
 		break;
 	}
 
-	DEBUG_TRACE (DEBUG::MTC, string_compose ("legal MTC window now %1 .. %2\n", window_begin, window_end));
+	DEBUG_TRACE (DEBUG::MTC, string_compose ("reset MTC window @ %3, now %1 .. %2\n", window_begin, window_end, root));
 }
 
 void
@@ -620,9 +592,19 @@ MTC_Slave::speed_and_position (double& speed, framepos_t& pos)
 
 	read_current (&last);
 
+	DEBUG_TRACE (DEBUG::MTC, string_compose ("speed&pos: timestamp %1 speed %2 initstate %3 dir %4 tpos %5 now %6 last-in %7\n",
+						 last.timestamp, 
+						 last.speed,
+						 engine_dll_initstate,
+						 transport_direction,
+						 sess_pos,
+						 now,
+						 last_inbound_frame));
+
 	/* re-init engine DLL here when state changed (direction, first_mtc_timestamp) */
-	if (last.timestamp == 0) { engine_dll_initstate = 0; }
+	if (last.timestamp == 0) { 
-	else if (engine_dll_initstate != transport_direction && last.speed != 0) {
+		engine_dll_initstate = 0; 
+	} else if (engine_dll_initstate != transport_direction && last.speed != 0) {
 		engine_dll_initstate = transport_direction;
 		init_engine_dll(last.position, session.engine().samples_per_cycle());
 		engine_dll_reinitialized = true;
@@ -656,9 +638,7 @@ MTC_Slave::speed_and_position (double& speed, framepos_t& pos)
 	/* interpolate position according to speed and time since last quarter-frame*/
 	if (speed_flt == 0.0f) {
 		elapsed = 0;
-	}
+	} else {
-	else
-	{
 		/* scale elapsed time by the current MTC speed */
 		elapsed = (framecnt_t) rint (speed_flt * (now - last.timestamp));
 		if (give_slave_full_control_over_transport_speed() && !engine_dll_reinitialized) {
@@ -688,14 +668,13 @@ MTC_Slave::speed_and_position (double& speed, framepos_t& pos)
 	 */
 	if (!session.actively_recording()
 	    && speed != 0
-			&& ( (pos < 0) || (labs(pos - sess_pos) > 3 * session.frame_rate()) )
+	    && ((pos < 0) || (labs(pos - sess_pos) > 3 * session.frame_rate()))) {
-	    ) {
 		engine_dll_initstate = 0;
 		queue_reset (false);
 	}
 
 	/* provide a .1% deadzone to lock the speed */
-	if (fabs(speed - 1.0) <= 0.001)
+	if (fabs (speed - 1.0) <= 0.001)
 	        speed = 1.0;
 
 	DEBUG_TRACE (DEBUG::MTC, string_compose ("MTCsync spd: %1 pos: %2 | last-pos: %3 elapsed: %4 delta: %5\n",

libs/ardour/session_process.cc

@@ -491,7 +491,6 @@ Session::follow_slave (pframes_t nframes)
 		goto noroll;
 	}
 
-	_slave->process (nframes);
 	_slave->speed_and_position (slave_speed, slave_transport_frame);
 
 	DEBUG_TRACE (DEBUG::Slave, string_compose ("Slave position %1 speed %2\n", slave_transport_frame, slave_speed));

libs/midi++2/mtc.cc

@@ -33,7 +33,7 @@ using namespace std;
 using namespace sigc;
 using namespace MIDI;
 
-#define DEBUG_MTC
+#undef DEBUG_MTC
 
 bool
 Parser::possible_mtc (byte *sysex_buf, size_t msglen)