Fix lost MIDI note offs and controllers.

Fix initial read of discrete MIDI controllers.

Fix spurious note offs when starting to play in the middle of a note.

Faster search for initial event when cached iterator is invalid.

So much for dropping the cached iterator.  The iterator is responsible for
handling note offs, so that doesn't work.  This design means we have some stuck
note issues at the source read level, but they should be taken care of by the
state tracker anyway.

diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h
index 364b220ebb748665ffa91606e5d8cb7fd8c78010..65e382c4b7efe5c0f6f0105703a7052f3a98cba2 100644 (file)
--- a/libs/ardour/ardour/midi_source.h
+++ b/libs/ardour/ardour/midi_source.h
@@ -140,6 +140,11 @@ 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; }
@@ -186,7 +191,11 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
        boost::shared_ptr<MidiModel> _model;
        bool                         _writing;
 
-       mutable double _length_beats;
+       mutable Evoral::Sequence<Evoral::MusicalTime>::const_iterator _model_iter;
+       mutable bool                                                  _model_iter_valid;
+
+       mutable double     _length_beats;
+       mutable framepos_t _last_read_end;
 
        /** 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 8e17e1d3ec7778bddd617c02b4d2a752fc45fcdb..259a04bc0ffa261191e663e7649ed34cf442c157 100644 (file)
--- a/libs/ardour/midi_model.cc
+++ b/libs/ardour/midi_model.cc
@@ -1630,6 +1630,7 @@ 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));
 }
 
@@ -1853,6 +1854,10 @@ 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 cb0d103b1e58d0f45f2674cb296d06579b6c1b26..71fd796b81a103e22296cd99a70d1a8d005b96f7 100644 (file)
--- a/libs/ardour/midi_region.cc
+++ b/libs/ardour/midi_region.cc
@@ -421,6 +421,13 @@ MidiRegion::model_automation_state_changed (Evoral::Parameter const & 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 494bb513fe58b9c690fa486c2558d347d8492628..823ca9dc5c9482c41f2b5d500d602005ef8abc94 100644 (file)
--- a/libs/ardour/midi_source.cc
+++ b/libs/ardour/midi_source.cc
@@ -56,7 +56,9 @@ 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)
 {
@@ -65,7 +67,9 @@ 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)
 {
@@ -169,6 +173,13 @@ MidiSource::update_length (framecnt_t)
        // You're not the boss of me!
 }
 
+void
+MidiSource::invalidate ()
+{
+       _model_iter_valid = false;
+       _model_iter.invalidate();
+}
+
 framecnt_t
 MidiSource::midi_read (Evoral::EventSink<framepos_t>&     dst,
                        framepos_t                         source_start,
@@ -186,11 +197,18 @@ MidiSource::midi_read (Evoral::EventSink<framepos_t>&     dst,
                                     source_start, start, cnt, tracker, name()));
 
        if (_model) {
-               // Read events up to end
-               const double start_beats = converter.from(start);
-               for (Evoral::Sequence<double>::const_iterator i = _model->begin(start_beats, false, filtered);
-                    i != _model->end();
-                    ++i) {
+               // Find appropriate model iterator
+               Evoral::Sequence<double>::const_iterator& i = _model_iter;
+               if (_last_read_end == 0 || start != _last_read_end || !_model_iter_valid) {
+                       // Cached iterator is invalid, search for the first event past start
+                       i                 = _model->begin(converter.from(start), false, filtered);
+                       _model_iter_valid = true;
+               }
+
+               _last_read_end = start + cnt;
+
+               // Copy events in [start, start + cnt) into dst
+               for (; i != _model->end(); ++i) {
                        const framecnt_t time_frames = converter.to(i->time());
                        if (time_frames < start + cnt) {
                                // Offset by source start to convert event time to session time
@@ -225,7 +243,10 @@ MidiSource::midi_write (MidiRingBuffer<framepos_t>& source,
 
        const framecnt_t ret = write_unlocked (source, source_start, cnt);
 
-       if (cnt != max_framecnt) {
+       if (cnt == max_framecnt) {
+               _last_read_end = 0;
+               invalidate();
+       } else {
                _capture_length += cnt;
        }
 

diff --git a/libs/ardour/smf_source.cc b/libs/ardour/smf_source.cc
index 3f22028e6265fa01ec90686e16665682e0a33785..8a956495a555d17f6d28c9ca506cb64d601bae12 100644 (file)
--- a/libs/ardour/smf_source.cc
+++ b/libs/ardour/smf_source.cc
@@ -701,6 +701,7 @@ SMFSource::load_model (bool lock, bool force_reload)
 
        _model->end_write (Evoral::Sequence<Evoral::MusicalTime>::ResolveStuckNotes, _length_beats);
        _model->set_edited (false);
+       invalidate();
 
        free(buf);
 }
@@ -724,6 +725,8 @@ SMFSource::flush_midi ()
        Evoral::SMF::end_write ();
        /* data in the file means its no longer removable */
        mark_nonremovable ();
+
+       invalidate();
 }
 
 void

diff --git a/libs/evoral/src/Sequence.cpp b/libs/evoral/src/Sequence.cpp
index 1cc8ff6e0fb3cadca14e78a5f74f10817fcd43dd..082f8f24ea82732bc8365bb67207da705dc7f06d 100644 (file)
--- a/libs/evoral/src/Sequence.cpp
+++ b/libs/evoral/src/Sequence.cpp
@@ -61,10 +61,13 @@ namespace Evoral {
 template<typename Time>
 Sequence<Time>::const_iterator::const_iterator()
        : _seq(NULL)
+       , _event(boost::shared_ptr< Event<Time> >(new Event<Time>()))
+       , _active_patch_change_message (0)
+       , _type(NIL)
        , _is_end(true)
        , _control_iter(_control_iters.end())
+       , _force_discrete(false)
 {
-       _event = boost::shared_ptr< Event<Time> >(new Event<Time>());
 }
 
 /** @param force_discrete true to force ControlLists to use discrete evaluation, otherwise false to get them to use their configured mode */
@@ -125,7 +128,7 @@ Sequence<Time>::const_iterator::const_iterator(const Sequence<Time>& seq, Time t
                DEBUG_TRACE (DEBUG::Sequence, string_compose ("Iterator: control: %1\n", seq._type_map.to_symbol(i->first)));
                double x, y;
                bool ret;
-               if (_force_discrete) {
+               if (_force_discrete || i->second->list()->interpolation() == ControlList::Discrete) {
                        ret = i->second->list()->rt_safe_earliest_event_discrete_unlocked (t, x, y, true);
                } else {
                        ret = i->second->list()->rt_safe_earliest_event_unlocked(t, x, y, true);
@@ -290,12 +293,10 @@ Sequence<Time>::const_iterator::operator++()
                // Increment current controller iterator
                if (_force_discrete || _control_iter->list->interpolation() == ControlList::Discrete) {
                        ret = _control_iter->list->rt_safe_earliest_event_discrete_unlocked (
-                               _control_iter->x, x, y, false
-                                                                                            );
+                               _control_iter->x, x, y, false);
                } else {
                        ret = _control_iter->list->rt_safe_earliest_event_linear_unlocked (
-                               _control_iter->x + time_between_interpolated_controller_outputs, x, y, false
-                                                                                          );
+                               _control_iter->x + time_between_interpolated_controller_outputs, x, y, false);
                }
                assert(!ret || x > _control_iter->x);
                if (ret) {