possibly fix deadlocking issues with tempo map by rearranging code and adding RT...
authorPaul Davis <paul@linuxaudiosystems.com>
Thu, 5 Jan 2012 05:05:31 +0000 (05:05 +0000)
committerPaul Davis <paul@linuxaudiosystems.com>
Thu, 5 Jan 2012 05:05:31 +0000 (05:05 +0000)
git-svn-id: svn://localhost/ardour2/branches/3.0@11164 d708f5d6-7413-0410-9779-e7cbd77b26cf

libs/ardour/ardour/tempo.h
libs/ardour/location.cc
libs/ardour/region.cc
libs/ardour/session_process.cc
libs/ardour/session_time.cc
libs/ardour/tempo.cc

index 09d8a646950cdab0305a03fdae8daecd604a114c..bfdcb1738964d3388c5b8076d0ec1d8c0f7d7026 100644 (file)
@@ -231,7 +231,21 @@ class TempoMap : public PBD::StatefulDestructible
                  framepos_t start, framepos_t end);
        
        void       bbt_time (framepos_t when, Timecode::BBT_Time&);
+       /* realtime safe variant of ::bbt_time(), will throw 
+          std::logic_error if the map is not large enough
+          to provide an answer.
+       */
+       void       bbt_time_rt (framepos_t when, Timecode::BBT_Time&);
+
+
         framecnt_t frame_time (const Timecode::BBT_Time&);
+       /* realtime safe variant of ::frame_time(), will throw 
+          std::logic_error if the map is not large enough
+          to provide an answer.
+       */
+        framecnt_t frame_time_rt (const Timecode::BBT_Time&);
+
+
        framecnt_t bbt_duration_at (framepos_t, const Timecode::BBT_Time&, int dir);
 
        static const Tempo& default_tempo() { return _default_tempo; }
@@ -300,13 +314,13 @@ class TempoMap : public PBD::StatefulDestructible
        mutable Glib::RWLock lock;
        BBTPointList*       _map;
 
-       void recompute_map (bool reassign_tempo_bbt, bool use_write_lock, framepos_t end = -1);
+       void recompute_map (bool reassign_tempo_bbt, framepos_t end = -1);
         void require_map_to (framepos_t pos);
         void require_map_to (const Timecode::BBT_Time&);
 
        BBTPointList::const_iterator bbt_before_or_at (framepos_t);
+       BBTPointList::const_iterator bbt_before_or_at (const Timecode::BBT_Time&);
        BBTPointList::const_iterator bbt_after_or_at (framepos_t);
-       BBTPointList::const_iterator bbt_point_for (const Timecode::BBT_Time&);
        
        framepos_t round_to_type (framepos_t fr, int dir, BBTPointType);
        
index ed9f34ff8bc147798abe76b25ae2bc3bf8cb96cd..e909957e0d9daaf27e1b67cd34504e6fec4f111c 100644 (file)
@@ -520,8 +520,8 @@ Location::recompute_bbt_from_frames ()
                return;
        }
 
-       _session.tempo_map().bbt_time (_start, _bbt_start);
-       _session.tempo_map().bbt_time (_end, _bbt_end);
+       _session.bbt_time (_start, _bbt_start);
+       _session.bbt_time (_end, _bbt_end);
 }
 
 void
index d863a0e6fd80fb8b47d65e7508f74760c2c161ce..66c0b84ae9c201f66264614ab1760a6f32627e2d 100644 (file)
@@ -547,7 +547,7 @@ Region::set_position_lock_style (PositionLockStyle ps)
                _position_lock_style = ps;
 
                if (_position_lock_style == MusicTime) {
-                       _session.tempo_map().bbt_time (_position, _bbt_time);
+                       _session.bbt_time (_position, _bbt_time);
                }
 
                send_change (Properties::position_lock_style);
@@ -624,7 +624,7 @@ void
 Region::recompute_position_from_lock_style ()
 {
        if (_position_lock_style == MusicTime) {
-               _session.tempo_map().bbt_time (_position, _bbt_time);
+               _session.bbt_time (_position, _bbt_time);
        }
 }
 
index 89dd83b9c694de4145e5bcc826925ba4b70787a3..1055a7c504397508f582d85797ab02cce05fca9d 100644 (file)
@@ -38,6 +38,7 @@
 #include "ardour/timestamps.h"
 #include "ardour/graph.h"
 #include "ardour/audio_port.h"
+#include "ardour/tempo.h"
 
 #include "midi++/manager.h"
 #include "midi++/mmc.h"
@@ -79,10 +80,14 @@ Session::process (pframes_t nframes)
        // the ticker is for sending time information like MidiClock
        framepos_t transport_frames = transport_frame();
        Timecode::BBT_Time transport_bbt;
-       bbt_time(transport_frames, transport_bbt);
-       Timecode::Time transport_timecode;
-       timecode_time(transport_frames, transport_timecode);
-       tick (transport_frames, transport_bbt, transport_timecode); /* EMIT SIGNAL */
+       try {
+               _tempo_map->bbt_time_rt (transport_frames, transport_bbt);
+               Timecode::Time transport_timecode;
+               timecode_time(transport_frames, transport_timecode);
+               tick (transport_frames, transport_bbt, transport_timecode); /* EMIT SIGNAL */
+       } catch (...) {
+               warning << _("Missed MIDI Clock tick due to problems with tempo map") << endmsg;
+       }
 
        SendFeedback (); /* EMIT SIGNAL */
 
index bf7a43845b168d94ddec62b6eed97e7fc1008abc..38eb0b4b28859005a5f42f9a440e136169db7a1d 100644 (file)
@@ -485,20 +485,25 @@ Session::jack_timebase_callback (jack_transport_state_t /*state*/,
 
                TempoMetric metric (_tempo_map->metric_at (_transport_frame));
 
-               _tempo_map->bbt_time (_transport_frame, bbt);
-
-               pos->bar = bbt.bars;
-               pos->beat = bbt.beats;
-               pos->tick = bbt.ticks;
-
-               // XXX still need to set bar_start_tick
-
-               pos->beats_per_bar = metric.meter().divisions_per_bar();
-               pos->beat_type = metric.meter().note_divisor();
-               pos->ticks_per_beat = Timecode::BBT_Time::ticks_per_bar_division;
-               pos->beats_per_minute = metric.tempo().beats_per_minute();
-
-               pos->valid = jack_position_bits_t (pos->valid | JackPositionBBT);
+               try {
+                       _tempo_map->bbt_time_rt (_transport_frame, bbt);
+
+                       pos->bar = bbt.bars;
+                       pos->beat = bbt.beats;
+                       pos->tick = bbt.ticks;
+                       
+                       // XXX still need to set bar_start_tick
+                       
+                       pos->beats_per_bar = metric.meter().divisions_per_bar();
+                       pos->beat_type = metric.meter().note_divisor();
+                       pos->ticks_per_beat = Timecode::BBT_Time::ticks_per_bar_division;
+                       pos->beats_per_minute = metric.tempo().beats_per_minute();
+                       
+                       pos->valid = jack_position_bits_t (pos->valid | JackPositionBBT);
+
+               } catch (...) {
+                       warning << _("failed to set tempo map information for JACK due to issues with tempo map") << endmsg;
+               }
        }
 
 #ifdef HAVE_JACK_VIDEO_SUPPORT
@@ -554,7 +559,7 @@ Session::convert_to_frames (AnyTime const & position)
 
        switch (position.type) {
        case AnyTime::BBT:
-               return _tempo_map->frame_time (position.bbt);
+               return _tempo_map->frame_time_rt (position.bbt);
                break;
 
        case AnyTime::Timecode:
index 23dd34c9622edb51d48088139116e655956c442a..8f7c4b587506d2c9b365f044c588aee1c566ce12 100644 (file)
@@ -600,7 +600,7 @@ TempoMap::change_initial_tempo (double beats_per_minute, double note_type)
 
        for (Metrics::iterator i = metrics->begin(); i != metrics->end(); ++i) {
                if ((t = dynamic_cast<TempoSection*> (*i)) != 0) {
-                       {
+                       { 
                                Glib::RWLock::WriterLock lm (lock);
                                *((Tempo*) t) = newtempo;
                                recompute_map (false, false);
@@ -695,27 +695,39 @@ TempoMap::first_tempo () const
 void
 TempoMap::require_map_to (framepos_t pos)
 {
-       /* CALLER MUST HOLD READ LOCK AND MAY NOT HOLD WRITE LOCK */
+       Glib::RWLock::WriterLock lm (lock);
 
        if (_map->empty() || _map->back().frame < pos) {
-               recompute_map (false, true, pos);
+               recompute_map (false, pos);
        }
 }
 
 void
 TempoMap::require_map_to (const BBT_Time& bbt)
 {
-       /* CALLER MUST HOLD READ LOCK AND MAY NOT HOLD WRITE LOCK */
+       Glib::RWLock::WriterLock lm (lock);
+
+       /* since we have no idea where BBT is if its off the map, see the last
+        * point in the map is past BBT, and if not add an arbitrary amount of
+        * time until it is.
+        */
+
+       int additional_minutes = 1;
        
-       if (_map->empty() || _map->back().bbt() < bbt) {
-               recompute_map (false, true, 99);
+       while (1) {
+               if (!_map->empty() && _map->back().bar >= (bbt.bars + 1)) {
+                       break;
+               }
+               /* add some more distance, using bigger steps each time */
+               recompute_map (false, _map->back().frame + (_frame_rate * 60 * additional_minutes));
+               additional_minutes *= 2;
        }
 }
 
 void
-TempoMap::recompute_map (bool reassign_tempo_bbt, bool use_write_lock, framepos_t end)
+TempoMap::recompute_map (bool reassign_tempo_bbt, framepos_t end)
 {
-       /* CALLER MUST HOLD READ LOCK AND MAY HOLD WRITE LOCK */
+       /* CALLER MUST HOLD WRITE LOCK */
 
        MeterSection* meter;
        TempoSection* tempo;
@@ -736,6 +748,11 @@ TempoMap::recompute_map (bool reassign_tempo_bbt, bool use_write_lock, framepos_
                } else {
                        end = _map->back().frame;
                }
+       } else {
+               if (!_map->empty ()) {
+                       /* never allow the map to be shortened */
+                       end = max (end, _map->back().frame);
+               }
        }
 
        DEBUG_TRACE (DEBUG::TempoMath, string_compose ("recomputing tempo map, zero to %1\n", end));
@@ -907,13 +924,7 @@ TempoMap::recompute_map (bool reassign_tempo_bbt, bool use_write_lock, framepos_
                }
        }
 
-       if (use_write_lock) {
-               Glib::RWLock::WriterLock lm (lock);
-               swap (_map, new_map);
-       } else {
-               swap (_map, new_map);
-       }
-
+       swap (_map, new_map);
        delete new_map;
 }
 
@@ -993,19 +1004,26 @@ TempoMap::metric_at (BBT_Time bbt) const
 void
 TempoMap::bbt_time (framepos_t frame, BBT_Time& bbt)
 {
-       Glib::RWLock::ReaderLock lm (lock);
+       require_map_to (frame);
 
-       const BBTPointList::const_iterator& i = bbt_before_or_at (frame);
+       Glib::RWLock::ReaderLock lm (lock);
+       return bbt_time (frame, bbt, bbt_before_or_at (frame));
+}
 
-       bbt.bars = (*i).bar;
-       bbt.beats = (*i).beat;
+void
+TempoMap::bbt_time_rt (framepos_t frame, BBT_Time& bbt)
+{
+       Glib::RWLock::ReaderLock lm (lock, Glib::TRY_LOCK);
 
-       if ((*i).frame == frame) {
-               bbt.ticks = 0;
-       } else {
-               bbt.ticks = llrint (((frame - (*i).frame) / (*i).meter->frames_per_division(*((*i).tempo), _frame_rate)) *
-                                   BBT_Time::ticks_per_bar_division);
+       if (!lm.locked()) {
+               throw std::logic_error ("TempoMap::bbt_time_rt() could not lock tempo map");
+       }
+       
+       if (_map->empty() || _map->back().frame < frame) {
+               throw std::logic_error (string_compose ("map not long enough to reach %1", frame));
        }
+
+       return bbt_time (frame, bbt, bbt_before_or_at (frame));
 }
 
 void
@@ -1027,10 +1045,36 @@ TempoMap::bbt_time (framepos_t frame, BBT_Time& bbt, const BBTPointList::const_i
 framepos_t
 TempoMap::frame_time (const BBT_Time& bbt)
 {
+       require_map_to (bbt);
+
        Glib::RWLock::ReaderLock lm (lock);
 
-       BBTPointList::const_iterator s = bbt_point_for (BBT_Time (1, 1, 0));
-       BBTPointList::const_iterator e = bbt_point_for (BBT_Time (bbt.bars, bbt.beats, 0));
+       BBTPointList::const_iterator s = bbt_before_or_at (BBT_Time (1, 1, 0));
+       BBTPointList::const_iterator e = bbt_before_or_at (BBT_Time (bbt.bars, bbt.beats, 0));
+
+       if (bbt.ticks != 0) {
+               return ((*e).frame - (*s).frame) + 
+                       llrint ((*e).meter->frames_per_division (*(*e).tempo, _frame_rate) * (bbt.ticks/BBT_Time::ticks_per_bar_division));
+       } else {
+               return ((*e).frame - (*s).frame);
+       }
+}
+
+framepos_t
+TempoMap::frame_time_rt (const BBT_Time& bbt)
+{
+       Glib::RWLock::ReaderLock lm (lock, Glib::TRY_LOCK);
+
+       if (!lm.locked()) {
+               throw std::logic_error ("TempoMap::bbt_time_rt() could not lock tempo map");
+       }
+
+       if (_map->empty() || _map->back().bar < bbt.bars || (_map->back().bar == bbt.bars && _map->back().beat < bbt.beats)) {
+               throw std::logic_error (string_compose ("map not long enough to reach %1", bbt));
+       }
+
+       BBTPointList::const_iterator s = bbt_before_or_at (BBT_Time (1, 1, 0));
+       BBTPointList::const_iterator e = bbt_before_or_at (BBT_Time (bbt.bars, bbt.beats, 0));
 
        if (bbt.ticks != 0) {
                return ((*e).frame - (*s).frame) + 
@@ -1061,7 +1105,7 @@ TempoMap::bbt_duration_at_unlocked (const BBT_Time& when, const BBT_Time& bbt, i
        }
 
        /* round back to the previous precise beat */
-       BBTPointList::const_iterator wi = bbt_point_for (BBT_Time (when.bars, when.beats, 0));
+       BBTPointList::const_iterator wi = bbt_before_or_at (BBT_Time (when.bars, when.beats, 0));
        BBTPointList::const_iterator start (wi);
        double tick_frames = 0;
 
@@ -1114,6 +1158,8 @@ TempoMap::round_to_beat (framepos_t fr, int dir)
 framepos_t
 TempoMap::round_to_beat_subdivision (framepos_t fr, int sub_num, int dir)
 {
+       require_map_to (fr);
+
        Glib::RWLock::ReaderLock lm (lock);
        BBTPointList::const_iterator i = bbt_before_or_at (fr);
        BBT_Time the_beat;
@@ -1231,6 +1277,8 @@ TempoMap::round_to_beat_subdivision (framepos_t fr, int sub_num, int dir)
 framepos_t
 TempoMap::round_to_type (framepos_t frame, int dir, BBTPointType type)
 {
+       require_map_to (frame);
+
        Glib::RWLock::ReaderLock lm (lock);
        BBTPointList::const_iterator fi;
 
@@ -1366,8 +1414,13 @@ TempoMap::map (TempoMap::BBTPointList::const_iterator& begin,
               TempoMap::BBTPointList::const_iterator& end, 
               framepos_t lower, framepos_t upper) 
 {
-       Glib::RWLock::ReaderLock lm (lock);
-       require_map_to (upper);
+       { 
+               Glib::RWLock::WriterLock lm (lock);
+               if (_map->empty() || (_map->back().frame < upper)) {
+                       recompute_map (false, upper);
+               }
+       }
+
        begin = lower_bound (_map->begin(), _map->end(), lower);
        end = upper_bound (_map->begin(), _map->end(), upper);
 }
@@ -1759,7 +1812,7 @@ TempoMap::framepos_plus_bbt (framepos_t pos, BBT_Time op)
                /* we hit the end of the map before finish the bbt walk.
                 */
 
-               require_map_to (pos + (_frame_rate * 60 * additional_minutes));
+               recompute_map (pos + (_frame_rate * 60 * additional_minutes));
                additional_minutes *= 2;
 
                /* go back and try again */
@@ -1783,13 +1836,14 @@ TempoMap::framepos_plus_bbt (framepos_t pos, BBT_Time op)
 Evoral::MusicalTime
 TempoMap::framewalk_to_beats (framepos_t pos, framecnt_t distance)
 {
-       Glib::RWLock::ReaderLock lm (lock);
-       BBTPointList::const_iterator i = bbt_after_or_at (pos);
-       Evoral::MusicalTime beats = 0;
        framepos_t end = pos + distance;
 
        require_map_to (end);
 
+       Glib::RWLock::ReaderLock lm (lock);
+       BBTPointList::const_iterator i = bbt_after_or_at (pos);
+       Evoral::MusicalTime beats = 0;
+
        /* if our starting BBTPoint is after pos, add a fractional beat
           to represent that distance.
        */
@@ -1802,6 +1856,7 @@ TempoMap::framewalk_to_beats (framepos_t pos, framecnt_t distance)
                ++i;
                beats++;
        }
+
        assert (i != _map->end());
        
        /* if our ending BBTPoint is after the end, subtract a fractional beat
@@ -1819,9 +1874,9 @@ TempoMap::BBTPointList::const_iterator
 TempoMap::bbt_before_or_at (framepos_t pos)
 {
        /* CALLER MUST HOLD READ LOCK */
+
        BBTPointList::const_iterator i;
 
-       require_map_to (pos);
        i = lower_bound (_map->begin(), _map->end(), pos);
        assert (i != _map->end());
        if ((*i).frame > pos) {
@@ -1831,52 +1886,46 @@ TempoMap::bbt_before_or_at (framepos_t pos)
        return i;
 }
 
+struct bbtcmp {
+    bool operator() (const BBT_Time& a, const BBT_Time& b) {
+           return a < b;
+    }
+};
+
 TempoMap::BBTPointList::const_iterator
-TempoMap::bbt_after_or_at (framepos_t pos)
+TempoMap::bbt_before_or_at (const BBT_Time& bbt)
 {
-       /* CALLER MUST HOLD READ LOCK */
-
        BBTPointList::const_iterator i;
+       bbtcmp cmp;
 
-       require_map_to (pos);
-       i = upper_bound (_map->begin(), _map->end(), pos);
+       i = lower_bound (_map->begin(), _map->end(), bbt, cmp);
        assert (i != _map->end());
+       if ((*i).bar > bbt.bars || (*i).beat > bbt.beats) {
+               assert (i != _map->begin());
+               --i;
+       }
        return i;
 }
 
-struct bbtcmp {
-    bool operator() (const BBT_Time& a, const BBT_Time& b) {
-           return a < b;
-    }
-};
-
 TempoMap::BBTPointList::const_iterator
-TempoMap::bbt_point_for (const BBT_Time& bbt)
+TempoMap::bbt_after_or_at (framepos_t pos) 
 {
        /* CALLER MUST HOLD READ LOCK */
 
        BBTPointList::const_iterator i;
-       bbtcmp cmp;
-       int additional_minutes = 1;
 
-       while (1) {
-               {
-                       if (!_map->empty() && _map->back().bar >= (bbt.bars + 1)) {
-                               break;
-                       }
-               }
-               /* add some more distance, using bigger steps each time */
-               require_map_to (_map->back().frame + (_frame_rate * 60 * additional_minutes));
-               additional_minutes *= 2;
+       if (_map->back().frame == pos) {
+               i = _map->end();
+               assert (i != _map->begin());
+               --i;
+               return i;
        }
 
-       i = lower_bound (_map->begin(), _map->end(), bbt, cmp);
+       i = upper_bound (_map->begin(), _map->end(), pos);
        assert (i != _map->end());
-
        return i;
 }
 
-
 /** Compare the time of this with that of another MetricSection.
  *  @param with_bbt True to compare using start(), false to use frame().
  *  @return -1 for less than, 0 for equal, 1 for greater than.