Fix dropped MIDI events, especially with record enabled.

I am not precisely sure why the cached iterator was causing this problem, it
shouldn't be invalidated, and the times make sense.  It may be some lock
related issue since the iterator holds a lock on the source.

In any case, this cached iterator was just to avoid repeated linear search of
the model, but since the model has a logarithmic search, instead just scrap all
this problematic persistent state and search for the appropriate start time
every read.  No need to be careful about invalidating when anything changes.

diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h
index 07a32c5bfcdd5d0c7c17f86ce615d9102b4076a5..411c76eeea5106cbc6ec0cf32932bf4b80cee70c 100644 (file)
--- a/libs/ardour/ardour/midi_source.h
+++ b/libs/ardour/ardour/midi_source.h
@@ -143,11 +143,6 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
        virtual void load_model(bool lock=true, bool force_reload=false) = 0;
        virtual void destroy_model() = 0;
 
-       /** This must be called with the source lock held whenever the
-        *  source/model contents have been changed (reset iterators/cache/etc).
-        */
-       void invalidate();
-
        void set_note_mode(NoteMode mode);
 
        boost::shared_ptr<MidiModel> model() { return _model; }
@@ -194,11 +189,7 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
        boost::shared_ptr<MidiModel> _model;
        bool                         _writing;
 
-       mutable Evoral::Sequence<Evoral::MusicalTime>::const_iterator _model_iter;
-       mutable bool                                                  _model_iter_valid;
-
-       mutable double     _length_beats;
-       mutable framepos_t _last_read_end;
+       mutable double _length_beats;
 
        /** The total duration of the current capture. */
        framepos_t _capture_length;

diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc
index 259a04bc0ffa261191e663e7649ed34cf442c157..8e17e1d3ec7778bddd617c02b4d2a752fc45fcdb 100644 (file)
--- a/libs/ardour/midi_model.cc
+++ b/libs/ardour/midi_model.cc
@@ -1630,7 +1630,6 @@ MidiModel::edit_lock()
        assert (ms);
 
        Glib::Threads::Mutex::Lock* source_lock = new Glib::Threads::Mutex::Lock (ms->mutex());
-       ms->invalidate(); // Release cached iterator's read lock on model
        return WriteLock(new WriteLockImpl(source_lock, _lock, _control_lock));
 }
 
@@ -1854,10 +1853,6 @@ MidiModel::set_midi_source (boost::shared_ptr<MidiSource> s)
 {
        boost::shared_ptr<MidiSource> old = _midi_source.lock ();
 
-       if (old) {
-               old->invalidate ();
-       }
-
        _midi_source_connections.drop_connections ();
 
        _midi_source = s;

diff --git a/libs/ardour/midi_region.cc b/libs/ardour/midi_region.cc
index 03db3eaaa76d7d59ed45152f9db5f17db474ce16..cb0d103b1e58d0f45f2674cb296d06579b6c1b26 100644 (file)
--- a/libs/ardour/midi_region.cc
+++ b/libs/ardour/midi_region.cc
@@ -414,20 +414,13 @@ MidiRegion::model_automation_state_changed (Evoral::Parameter const & p)
        /* Update our filtered parameters list after a change to a parameter's AutoState */
 
        boost::shared_ptr<AutomationControl> ac = model()->automation_control (p);
-       assert (ac);
-
-       if (ac->alist()->automation_state() == Play) {
+       if (!ac || ac->alist()->automation_state() == Play) {
+               /* It should be "impossible" for ac to be NULL, but if it is, don't
+                  filter the parameter so events aren't lost. */
                _filtered_parameters.erase (p);
        } else {
                _filtered_parameters.insert (p);
        }
-
-       /* the source will have an iterator into the model, and that iterator will have been set up
-          for a given set of filtered_parameters, so now that we've changed that list we must invalidate
-          the iterator.
-       */
-       Glib::Threads::Mutex::Lock lm (midi_source(0)->mutex());
-       midi_source(0)->invalidate ();
 }
 
 /** This is called when a trim drag has resulted in a -ve _start time for this region.

diff --git a/libs/ardour/midi_source.cc b/libs/ardour/midi_source.cc
index 655222413aa0a5f5d836c6121c5511a99d8569c0..52c15f94cd36e95941545d01670400d9931b4efc 100644 (file)
--- a/libs/ardour/midi_source.cc
+++ b/libs/ardour/midi_source.cc
@@ -56,9 +56,7 @@ PBD::Signal1<void,MidiSource*> MidiSource::MidiSourceCreated;
 MidiSource::MidiSource (Session& s, string name, Source::Flag flags)
        : Source(s, DataType::MIDI, name, flags)
        , _writing(false)
-       , _model_iter_valid(false)
        , _length_beats(0.0)
-       , _last_read_end(0)
        , _capture_length(0)
        , _capture_loop_length(0)
 {
@@ -67,9 +65,7 @@ MidiSource::MidiSource (Session& s, string name, Source::Flag flags)
 MidiSource::MidiSource (Session& s, const XMLNode& node)
        : Source(s, node)
        , _writing(false)
-       , _model_iter_valid(false)
        , _length_beats(0.0)
-       , _last_read_end(0)
        , _capture_length(0)
        , _capture_loop_length(0)
 {
@@ -183,13 +179,6 @@ MidiSource::update_length (framecnt_t)
        // You're not the boss of me!
 }
 
-void
-MidiSource::invalidate ()
-{
-       _model_iter_valid = false;
-       _model_iter.invalidate();
-}
-
 /** @param filtered A set of parameters whose MIDI messages will not be returned */
 framecnt_t
 MidiSource::midi_read (Evoral::EventSink<framepos_t>& dst, framepos_t source_start,
@@ -205,26 +194,11 @@ MidiSource::midi_read (Evoral::EventSink<framepos_t>& dst, framepos_t source_sta
                                                          source_start, start, cnt, tracker, name()));
 
        if (_model) {
-               Evoral::Sequence<double>::const_iterator& i = _model_iter;
-
-               // If the cached iterator is invalid, search for the first event past start
-               if (_last_read_end == 0 || start != _last_read_end || !_model_iter_valid) {
-                       DEBUG_TRACE (DEBUG::MidiSourceIO, string_compose ("*** %1 search for relevant iterator for %1 / %2\n", _name, source_start, start));
-                       for (i = _model->begin(0, false, filtered); i != _model->end(); ++i) {
-                               if (converter.to(i->time()) >= start) {
-                                       DEBUG_TRACE (DEBUG::MidiSourceIO, string_compose ("***\tstop iterator search @ %1\n", i->time()));
-                                       break;
-                               }
-                       }
-                       _model_iter_valid = true;
-               } else {
-                       DEBUG_TRACE (DEBUG::MidiSourceIO, string_compose ("*** %1 use cachediterator for %1 / %2\n", _name, source_start, start));
-               }
-
-               _last_read_end = start + cnt;
-
                // Read events up to end
-               for (; i != _model->end(); ++i) {
+               const double start_beats = converter.from(start);
+               for (Evoral::Sequence<double>::const_iterator i = _model->begin(start_beats, false, filtered);
+                    i != _model->end();
+                    ++i) {
                        const framecnt_t time_frames = converter.to(i->time());
                        if (time_frames < start + cnt) {
                                /* convert event times to session frames by adding on the source start position in session frames */
@@ -265,9 +239,7 @@ MidiSource::midi_write (MidiRingBuffer<framepos_t>& source,
 
        const framecnt_t ret = write_unlocked (source, source_start, cnt);
 
-       if (cnt == max_framecnt) {
-               _last_read_end = 0;
-       } else {
+       if (cnt != max_framecnt) {
                _capture_length += cnt;
        }
 

diff --git a/libs/ardour/smf_source.cc b/libs/ardour/smf_source.cc
index 8a865c39578abe4915f1999b5013436f77723df7..3f22028e6265fa01ec90686e16665682e0a33785 100644 (file)
--- a/libs/ardour/smf_source.cc
+++ b/libs/ardour/smf_source.cc
@@ -702,8 +702,6 @@ SMFSource::load_model (bool lock, bool force_reload)
        _model->end_write (Evoral::Sequence<Evoral::MusicalTime>::ResolveStuckNotes, _length_beats);
        _model->set_edited (false);
 
-       _model_iter = _model->begin();
-
        free(buf);
 }