* made MidiClock_Slave conform more to to the Spec by starting transport
authorHans Baier <hansfbaier@googlemail.com>
Thu, 1 Jan 2009 06:52:18 +0000 (06:52 +0000)
committerHans Baier <hansfbaier@googlemail.com>
Thu, 1 Jan 2009 06:52:18 +0000 (06:52 +0000)
  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

libs/ardour/ardour/slave.h
libs/ardour/midi_clock_slave.cc
libs/ardour/session_process.cc
libs/ardour/ticker.cc

index ffeacd30177b4bfc437ba5aaf2ff59d815f74c1d..db66e428be1cf74e591e36f63e7d2acfe864b8d0 100644 (file)
@@ -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;
 };
 
index e51fe764751db3d20682b61a1b0b690372ba3fa0..e6639446d58a78d82a7a992e4a7fe9a43c876bf7 100644 (file)
@@ -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,22 +171,33 @@ 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;
        _starting = true;
 }
 
+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
index c2e78df643e4203cca9abe80ffdfd7ac9949ac10..5ca77f02f176770b53751123c8b553aff69972fb 100644 (file)
@@ -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;
 }
index 0012d47abfc3f60c9c2598b03f58aa9a5d52e3b3..4fe8fab520e3894e0830d06935faf310e61771b2 100644 (file)
@@ -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);
 }