fixes to get MTC (and probably MIDI clock) slaving working again
authorPaul Davis <paul@linuxaudiosystems.com>
Tue, 13 Aug 2013 16:53:28 +0000 (12:53 -0400)
committerPaul Davis <paul@linuxaudiosystems.com>
Tue, 13 Aug 2013 16:53:28 +0000 (12:53 -0400)
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
libs/ardour/ardour/slave.h
libs/ardour/midi_clock_slave.cc
libs/ardour/midi_port.cc
libs/ardour/midiport_manager.cc
libs/ardour/mtc_slave.cc
libs/ardour/session_process.cc
libs/midi++2/mtc.cc

index 14dc6cd02123ad48e336bc9d5a71c4c9abbced92..00617d90ecb5d5d146b6d98a00b31284a3d0cdd1 100644 (file)
@@ -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,16 +58,33 @@ 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;
+        friend class PortManager;
 
-    MidiPort (const std::string& name, PortFlags);
+        MidiPort (const std::string& name, PortFlags);
 
   private:
        MidiBuffer* _buffer;
        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);
 };
index 1d8a7dd965e84ff56320c0feaf61e0260e150ab9..c60acb1df7ebcb03181fe187c2a56a5ff364e83a 100644 (file)
@@ -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)
index 752644e9f433c8700e918bd5765be6b583839815..7eaeeb9e07a5fe75551675a7502433d7c68f3c95 100644 (file)
@@ -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)
+void
+MIDIClock_Slave::rebind (MidiPort& port)
 {
-       MidiBuffer& mb (port->get_midi_buffer (nframes));
+       DEBUG_TRACE (DEBUG::MidiClock, string_compose ("MIDIClock_Slave: connecting to port %1\n", port.name()));
 
-       /* dump incoming MIDI to parser */
+       port_connections.drop_connections ();
 
-       for (MidiBuffer::iterator b = mb.begin(); b != mb.end(); ++b) {
-               uint8_t* buf = (*b).buffer();
+       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));
 
-               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)
-{
-       port = &p;
-       DEBUG_TRACE (DEBUG::MidiClock, string_compose ("MIDIClock_Slave: connecting to port %1\n", port->name()));
 }
 
 void
index 1816ddcb6ff783d8395fdc53058f90cea07d825f..eb6759f3086d4b7f49ef579570abc553ee13c5a7 100644 (file)
@@ -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;
+}
index a4d53d6530a81b1df9e552f81534a18152698018..5d4e8e37f64642e3f08e470281791c2fa7cee1d7 100644 (file)
@@ -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
index 7f68e54f4bcbe1ba5ea0f40099582f966b202cd8..e9071af61917690bc7d86d4eb25205c2ebdb2f24 100644 (file)
@@ -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));
-       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_time.connect_same_thread (port_connections,  boost::bind (&MTC_Slave::update_mtc_time, 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));
+       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; }
-       else if (engine_dll_initstate != transport_direction && last.speed != 0) {
+       if (last.timestamp == 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",
index 6a24198bec303ca1799a7ea86394548d8ac309c8..3c46a2e0a631e42e97348def144e1f008a1f21de 100644 (file)
@@ -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));
index 57c78cbfb01285577720cfbb2271a10a73cd0561..0c2d8a41b3a3b8089d55089b0dda4e585d41ae32 100644 (file)
@@ -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)