Handle edits while playing precisely.

This avoids stuck notes if active notes are edited, but without stopping all
active notes in the region on any edit as before.

This implementation injects note ons in places that aren't actually note
starts.  Depending on how percussive the instrument is, this may not be
desired.  In the future, an option for this would be an improvement, but there
are other places where "start notes in the middle" is a reasonable option.  I
think that should be handled universally if we're to do it at all, so not
considering it a part of this fix for now.

11 files changed, 378 insertions, 116 deletions

diff --git a/libs/ardour/ardour/midi_model.h b/libs/ardour/ardour/midi_model.h
index 4214431280..b2e018ca3b 100644
--- a/libs/ardour/ardour/midi_model.h
+++ b/libs/ardour/ardour/midi_model.h
@@ -126,7 +126,6 @@ public:
 
 		static Variant::Type value_type (Property prop);
 
-	private:
 		struct NoteChange {
 			NoteDiffCommand::Property property;
 			NotePtr note;
@@ -135,12 +134,17 @@ public:
 			Variant new_value;
 		};
 
-		typedef std::list<NoteChange> ChangeList;
-		ChangeList _changes;
-
+		typedef std::list<NoteChange>                                    ChangeList;
 		typedef std::list< boost::shared_ptr< Evoral::Note<TimeType> > > NoteList;
-		NoteList _added_notes;
-		NoteList _removed_notes;
+
+		const ChangeList& changes()       const { return _changes; }
+		const NoteList&   added_notes()   const { return _added_notes; }
+		const NoteList&   removed_notes() const { return _removed_notes; }
+
+	private:
+		ChangeList _changes;
+		NoteList   _added_notes;
+		NoteList   _removed_notes;
 
 		std::set<NotePtr> side_effect_removals;
 
@@ -285,6 +289,8 @@ public:
 	void insert_silence_at_start (TimeType);
 	void transpose (TimeType, TimeType, int);
 
+	std::set<WeakNotePtr>& active_notes() { return _active_notes; }
+
 protected:
 	int resolve_overlaps_unlocked (const NotePtr, void* arg = 0);
 
@@ -302,7 +308,6 @@ private:
 
 public:
 	WriteLock edit_lock();
-	WriteLock write_lock();
 
 private:
 	friend class DeltaCommand;
@@ -319,6 +324,8 @@ private:
 	// We cannot use a boost::shared_ptr here to avoid a retain cycle
 	boost::weak_ptr<MidiSource> _midi_source;
 	InsertMergePolicy _insert_merge_policy;
+
+	std::set<WeakNotePtr> _active_notes;
 };
 
 } /* namespace ARDOUR */
diff --git a/libs/ardour/ardour/midi_playlist.h b/libs/ardour/ardour/midi_playlist.h
index f49593bd85..614a5e1c1f 100644
--- a/libs/ardour/ardour/midi_playlist.h
+++ b/libs/ardour/ardour/midi_playlist.h
@@ -23,8 +23,15 @@
 #include <vector>
 #include <list>
 
+#include <boost/utility.hpp>
+
 #include "ardour/ardour.h"
+#include "ardour/midi_model.h"
+#include "ardour/midi_state_tracker.h"
+#include "ardour/note_fixer.h"
 #include "ardour/playlist.h"
+#include "evoral/Beats.hpp"
+#include "evoral/Note.hpp"
 #include "evoral/Parameter.hpp"
 
 namespace Evoral {
@@ -34,10 +41,10 @@ template<typename Time> class EventSink;
 namespace ARDOUR
 {
 
-class Session;
+class BeatsFramesConverter;
 class MidiRegion;
+class Session;
 class Source;
-class MidiStateTracker;
 
 template<typename T> class MidiRingBuffer;
 
@@ -80,6 +87,15 @@ public:
 
 	std::set<Evoral::Parameter> contained_automation();
 
+	/** Handle a region edit during read.
+	 *
+	 * This must be called before the command is applied to the model.  Events
+	 * are injected into the playlist output to compensate for edits to active
+	 * notes and maintain coherent output and tracker state.
+	 */
+	void region_edited(boost::shared_ptr<Region>         region,
+	                   const MidiModel::NoteDiffCommand* cmd);
+
 	/** Clear all note trackers. */
 	void reset_note_trackers ();
 
@@ -91,18 +107,24 @@ public:
 	void resolve_note_trackers (Evoral::EventSink<framepos_t>& dst, framepos_t time);
 
 protected:
-
 	void remove_dependents (boost::shared_ptr<Region> region);
 
 private:
-	void dump () const;
+	typedef Evoral::Note<Evoral::Beats> Note;
+	typedef Evoral::Event<framepos_t>   Event;
 
-	bool region_changed (const PBD::PropertyChange&, boost::shared_ptr<Region>);
+	struct RegionTracker : public boost::noncopyable {
+		MidiStateTracker tracker;  ///< Active note tracker
+		NoteFixer        fixer;    ///< Edit compensation
+	};
 
-	NoteMode _note_mode;
+	typedef std::map< Region*, boost::shared_ptr<RegionTracker> > NoteTrackers;
+
+	void dump () const;
 
-	typedef std::map<Region*,MidiStateTracker*> NoteTrackers;
 	NoteTrackers _note_trackers;
+	NoteMode     _note_mode;
+	framepos_t   _read_end;
 };
 
 } /* namespace ARDOUR */
diff --git a/libs/ardour/ardour/midi_region.h b/libs/ardour/ardour/midi_region.h
index ece23b65f0..f7e6c97ea0 100644
--- a/libs/ardour/ardour/midi_region.h
+++ b/libs/ardour/ardour/midi_region.h
@@ -32,11 +32,6 @@ class XMLNode;
 
 namespace ARDOUR {
 	namespace Properties {
-		/* this is pseudo-property: nothing has this as an actual
-		   property, but it allows us to signal changes to the
-		   MidiModel used by the MidiRegion
-		*/
-		LIBARDOUR_API extern PBD::PropertyDescriptor<void*> midi_data;
 		LIBARDOUR_API extern PBD::PropertyDescriptor<Evoral::Beats> start_beats;
 		LIBARDOUR_API extern PBD::PropertyDescriptor<Evoral::Beats> length_beats;
 	}
@@ -141,7 +136,6 @@ class LIBARDOUR_API MidiRegion : public Region
 
 	void model_changed ();
 	void model_automation_state_changed (Evoral::Parameter const &);
-	void model_contents_changed ();
 
 	void set_start_beats_from_start_frames ();
 	void update_after_tempo_map_change ();
diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h
index 8a0c13681d..156f3dbfa0 100644
--- a/libs/ardour/ardour/midi_source.h
+++ b/libs/ardour/ardour/midi_source.h
@@ -154,8 +154,12 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	virtual void load_model(const Glib::Threads::Mutex::Lock& lock, bool force_reload=false) = 0;
 	virtual void destroy_model(const Glib::Threads::Mutex::Lock& lock) = 0;
 
-	/** Reset cached information (like iterators) when things have changed. */
-	void invalidate(const Glib::Threads::Mutex::Lock& lock);
+	/** Reset cached information (like iterators) when things have changed.
+	 * @param lock Source lock, which must be held by caller.
+	 * @param notes If non-NULL, currently active notes are added to this set.
+	 */
+	void invalidate(const Glib::Threads::Mutex::Lock&                       lock,
+	                std::set<Evoral::Sequence<Evoral::Beats>::WeakNotePtr>* notes=NULL);
 
 	void set_note_mode(const Glib::Threads::Mutex::Lock& lock, NoteMode mode);
 
diff --git a/libs/ardour/ardour/note_fixer.h b/libs/ardour/ardour/note_fixer.h
new file mode 100644
index 0000000000..09f45cdec7
--- /dev/null
+++ b/libs/ardour/ardour/note_fixer.h
@@ -0,0 +1,102 @@
+/*
+    Copyright (C) 2015 Paul Davis
+    Author: David Robillard
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+#ifndef __ardour_note_fixer_h__
+#define __ardour_note_fixer_h__
+
+#include <list>
+
+#include <boost/utility.hpp>
+
+#include "ardour/midi_model.h"
+#include "ardour/types.h"
+#include "evoral/Beats.hpp"
+#include "evoral/Note.hpp"
+
+namespace Evoral { template<typename Time> class EventSink; }
+
+namespace ARDOUR {
+
+class BeatsFramesConverter;
+class MidiStateTracker;
+class TempoMap;
+
+/** A tracker and compensator for note edit operations.
+ *
+ * This monitors edit operations sent to a model that affect active notes
+ * during a read, and maintains a queue of synthetic events that should be sent
+ * at the start of the next read to maintain coherent MIDI state.
+ */
+class NoteFixer : public boost::noncopyable
+{
+public:
+	typedef Evoral::Note<Evoral::Beats> Note;
+
+	~NoteFixer();
+
+	/** Clear all internal state. */
+	void clear();
+
+	/** Handle a region edit during read.
+	 *
+	 * This must be called before the command is applied to the model.  Events
+	 * are enqueued to compensate for edits which should be later sent with
+	 * emit() at the start of the next read.
+	 *
+	 * @param cmd Command to compensate for.
+	 * @param origin Timeline position of edited source.
+	 * @param pos Current read position (last read end).
+	 */
+	void prepare(TempoMap&                          tempo_map,
+	             const MidiModel::NoteDiffCommand*  cmd,
+	             framepos_t                         origin,
+	             framepos_t                         pos,
+	             std::set< boost::weak_ptr<Note> >& active_notes);
+
+	/** Emit any pending edit compensation events.
+	 *
+	 * @param dst Destination for events.
+	 * @param pos Timestamp to be used for every event, should be the start of
+	 * the read block immediately following any calls to prepare().
+	 * @param tracker Tracker to update with emitted events.
+	 */
+	void emit(Evoral::EventSink<framepos_t>& dst,
+	          framepos_t                     pos,
+	          MidiStateTracker&              tracker);
+
+private:
+	typedef Evoral::Event<framepos_t> Event;
+	typedef std::list<Event*>         Events;
+
+	/** Copy a beats event to a frames event with the given time stamp. */
+	Event* copy_event(framepos_t time, const Evoral::Event<Evoral::Beats>& ev);
+
+	/** Return true iff `note` is active at `pos`. */
+	bool note_is_active(const BeatsFramesConverter& converter,
+	                    boost::shared_ptr<Note>     note,
+	                    framepos_t                  pos);
+
+	Events _events;
+};
+
+} /* namespace ARDOUR */
+
+#endif	/* __ardour_note_fixer_h__ */
+
+
diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc
index 7e88569adc..de5000669c 100644
--- a/libs/ardour/midi_model.cc
+++ b/libs/ardour/midi_model.cc
@@ -1612,25 +1612,19 @@ MidiModel::find_sysex (gint sysex_id)
 MidiModel::WriteLock
 MidiModel::edit_lock()
 {
-	boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
-	assert (ms);
-
-	Glib::Threads::Mutex::Lock* source_lock = new Glib::Threads::Mutex::Lock (ms->mutex());
-	ms->invalidate(*source_lock); // Release cached iterator's read lock on model
-	return WriteLock(new WriteLockImpl(source_lock, _lock, _control_lock));
-}
+	boost::shared_ptr<MidiSource> ms          = _midi_source.lock();
+	Glib::Threads::Mutex::Lock*   source_lock = 0;
 
-/** Lock just the model, the source lock must already be held.
- * This should only be called from libardour/evoral places
- */
-MidiModel::WriteLock
-MidiModel::write_lock()
-{
-	boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
-	assert (ms);
+	if (ms) {
+		/* Take source lock and invalidate iterator to release its lock on model.
+		   Add currently active notes to _active_notes so we can restore them
+		   if playback resumes at the same point after the edit. */
+		source_lock = new Glib::Threads::Mutex::Lock(ms->mutex());
+		ms->invalidate(*source_lock,
+		               ms->session().transport_rolling() ? &_active_notes : NULL);
+	}
 
-	assert (!ms->mutex().trylock ());
-	return WriteLock(new WriteLockImpl(0, _lock, _control_lock));
+	return WriteLock(new WriteLockImpl(source_lock, _lock, _control_lock));
 }
 
 int
diff --git a/libs/ardour/midi_playlist.cc b/libs/ardour/midi_playlist.cc
index 60753d0baa..398234e2ba 100644
--- a/libs/ardour/midi_playlist.cc
+++ b/libs/ardour/midi_playlist.cc
@@ -17,21 +17,22 @@
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 */
 
-#include <cassert>
-
 #include <algorithm>
+#include <cassert>
+#include <cstdlib>
 #include <iostream>
 #include <utility>
 
-#include <stdlib.h>
-
 #include "evoral/EventList.hpp"
 
+#include "ardour/beats_frames_converter.h"
 #include "ardour/debug.h"
 #include "ardour/midi_model.h"
 #include "ardour/midi_playlist.h"
 #include "ardour/midi_region.h"
+#include "ardour/midi_source.h"
 #include "ardour/midi_state_tracker.h"
+#include "ardour/session.h"
 #include "ardour/types.h"
 
 #include "i18n.h"
@@ -43,6 +44,7 @@ using namespace std;
 MidiPlaylist::MidiPlaylist (Session& session, const XMLNode& node, bool hidden)
 	: Playlist (session, node, DataType::MIDI, hidden)
 	, _note_mode(Sustained)
+	, _read_end(0)
 {
 #ifndef NDEBUG
 	const XMLProperty* prop = node.property("type");
@@ -61,12 +63,14 @@ MidiPlaylist::MidiPlaylist (Session& session, const XMLNode& node, bool hidden)
 MidiPlaylist::MidiPlaylist (Session& session, string name, bool hidden)
 	: Playlist (session, name, DataType::MIDI, hidden)
 	, _note_mode(Sustained)
+	, _read_end(0)
 {
 }
 
 MidiPlaylist::MidiPlaylist (boost::shared_ptr<const MidiPlaylist> other, string name, bool hidden)
 	: Playlist (other, name, hidden)
 	, _note_mode(other->_note_mode)
+	, _read_end(0)
 {
 }
 
@@ -77,6 +81,7 @@ MidiPlaylist::MidiPlaylist (boost::shared_ptr<const MidiPlaylist> other,
                             bool                                  hidden)
 	: Playlist (other, start, dur, name, hidden)
 	, _note_mode(other->_note_mode)
+	, _read_end(0)
 {
 }
 
@@ -103,13 +108,18 @@ struct EventsSortByTimeAndType {
 framecnt_t
 MidiPlaylist::read (Evoral::EventSink<framepos_t>& dst, framepos_t start, framecnt_t dur, unsigned chan_n)
 {
+	typedef pair<MidiStateTracker*,framepos_t> TrackerInfo;
+
 	Playlist::RegionReadLock rl (this);
 
 	DEBUG_TRACE (DEBUG::MidiPlaylistIO,
 	             string_compose ("---- MidiPlaylist::read %1 .. %2 (%3 trackers) ----\n",
 	                             start, start + dur, _note_trackers.size()));
 
-	typedef pair<MidiStateTracker*,framepos_t> TrackerInfo;
+	/* First, emit any queued edit fixup events at start. */
+	for (NoteTrackers::iterator t = _note_trackers.begin(); t != _note_trackers.end(); ++t) {
+		t->second->fixer.emit(dst, _read_end, t->second->tracker);
+	}
 
 	/* Find relevant regions that overlap [start..end] */
 	const framepos_t                         end = start + dur - 1;
@@ -158,11 +168,11 @@ MidiPlaylist::read (Evoral::EventSink<framepos_t>& dst, framepos_t start, framec
 		}
 
 		/* Get the existing note tracker for this region, or create a new one. */
-		NoteTrackers::iterator t           = _note_trackers.find (mr.get());
-		MidiStateTracker*      tracker     = NULL;
-		bool                   new_tracker = false;
+		NoteTrackers::iterator           t           = _note_trackers.find (mr.get());
+		bool                             new_tracker = false;
+		boost::shared_ptr<RegionTracker> tracker;
 		if (t == _note_trackers.end()) {
-			tracker = new MidiStateTracker;
+			tracker     = boost::shared_ptr<RegionTracker>(new RegionTracker);
 			new_tracker = true;
 			DEBUG_TRACE (DEBUG::MidiPlaylistIO,
 			             string_compose ("\tPre-read %1 (%2 .. %3): new tracker\n",
@@ -171,13 +181,13 @@ MidiPlaylist::read (Evoral::EventSink<framepos_t>& dst, framepos_t start, framec
 			tracker = t->second;
 			DEBUG_TRACE (DEBUG::MidiPlaylistIO,
 			             string_compose ("\tPre-read %1 (%2 .. %3): %4 active notes\n",
-			                             mr->name(), mr->position(), mr->last_frame(), tracker->on()));
+			                             mr->name(), mr->position(), mr->last_frame(), tracker->tracker.on()));
 		}
 
-		/** Read from region into target. */
-		mr->read_at (tgt, start, dur, chan_n, _note_mode, tracker);
+		/* Read from region into target. */
+		mr->read_at (tgt, start, dur, chan_n, _note_mode, &tracker->tracker);
 		DEBUG_TRACE (DEBUG::MidiPlaylistIO,
-		             string_compose ("\tPost-read: %1 active notes\n", tracker->on()));
+		             string_compose ("\tPost-read: %1 active notes\n", tracker->tracker.on()));
 
 		if (find (ended.begin(), ended.end(), *i) != ended.end()) {
 			/* Region ended within the read range, so resolve any active notes
@@ -187,8 +197,7 @@ MidiPlaylist::read (Evoral::EventSink<framepos_t>& dst, framepos_t start, framec
 			             string_compose ("\t%1 ended, resolve notes and delete (%2) tracker\n",
 			                             mr->name(), ((new_tracker) ? "new" : "old")));
 
-			tracker->resolve_notes (tgt, (*i)->last_frame());
-			delete tracker;
+			tracker->tracker.resolve_notes (tgt, (*i)->last_frame());
 			if (!new_tracker) {
 				_note_trackers.erase (t);
 			}
@@ -216,17 +225,40 @@ MidiPlaylist::read (Evoral::EventSink<framepos_t>& dst, framepos_t start, framec
 	}
 
 	DEBUG_TRACE (DEBUG::MidiPlaylistIO, "---- End MidiPlaylist::read ----\n");
+	_read_end = start + dur;
 	return dur;
 }
 
 void
+MidiPlaylist::region_edited(boost::shared_ptr<Region>         region,
+                            const MidiModel::NoteDiffCommand* cmd)
+{
+	typedef MidiModel::NoteDiffCommand Command;
+
+	boost::shared_ptr<MidiRegion> mr = boost::dynamic_pointer_cast<MidiRegion>(region);
+	if (!mr || !_session.transport_rolling()) {
+		return;
+	}
+
+	/* Take write lock to prevent concurrency with read(). */
+	Playlist::RegionWriteLock lock(this);
+
+	NoteTrackers::iterator t = _note_trackers.find(mr.get());
+	if (t == _note_trackers.end()) {
+		return; /* Region is not currently active, nothing to do. */
+	}
+
+	/* Queue any necessary edit compensation events. */
+	t->second->fixer.prepare(
+		_session.tempo_map(), cmd, mr->position() - mr->start(),
+		_read_end, mr->midi_source()->model()->active_notes());
+}
+
+void
 MidiPlaylist::reset_note_trackers ()
 {
 	Playlist::RegionWriteLock rl (this, false);
 
-	for (NoteTrackers::iterator n = _note_trackers.begin(); n != _note_trackers.end(); ++n) {
-		delete n->second;
-	}
 	DEBUG_TRACE (DEBUG::MidiTrackers, string_compose ("%1 reset all note trackers\n", name()));
 	_note_trackers.clear ();
 }
@@ -237,8 +269,7 @@ MidiPlaylist::resolve_note_trackers (Evoral::EventSink<framepos_t>& dst, framepo
 	Playlist::RegionWriteLock rl (this, false);
 
 	for (NoteTrackers::iterator n = _note_trackers.begin(); n != _note_trackers.end(); ++n) {
-		n->second->resolve_notes(dst, time);
-		delete n->second;
+		n->second->tracker.resolve_notes(dst, time);
 	}
 	DEBUG_TRACE (DEBUG::MidiTrackers, string_compose ("%1 resolve all note trackers\n", name()));
 	_note_trackers.clear ();
@@ -248,14 +279,7 @@ void
 MidiPlaylist::remove_dependents (boost::shared_ptr<Region> region)
 {
 	/* MIDI regions have no dependents (crossfades) but we might be tracking notes */
-	NoteTrackers::iterator t = _note_trackers.find (region.get());
-
-	/* GACK! THREAD SAFETY! */
-
-	if (t != _note_trackers.end()) {
-		delete t->second;
-		_note_trackers.erase (t);
-	}
+	_note_trackers.erase(region.get());
 }
 
 int
@@ -355,24 +379,3 @@ MidiPlaylist::contained_automation()
 
 	return ret;
 }
-
-
-bool
-MidiPlaylist::region_changed (const PBD::PropertyChange& what_changed, boost::shared_ptr<Region> region)
-{
-	if (in_flush || in_set_state) {
-		return false;
-	}
-
-	PBD::PropertyChange our_interests;
-	our_interests.add (Properties::midi_data);
-
-	bool parent_wants_notify = Playlist::region_changed (what_changed, region);
-
-	if (parent_wants_notify || what_changed.contains (our_interests)) {
-		notify_contents_changed ();
-	}
-
-	return true;
-}
-
diff --git a/libs/ardour/midi_region.cc b/libs/ardour/midi_region.cc
index c02caff470..5c197761ac 100644
--- a/libs/ardour/midi_region.cc
+++ b/libs/ardour/midi_region.cc
@@ -53,7 +53,6 @@ using namespace PBD;
 
 namespace ARDOUR {
 	namespace Properties {
-		PBD::PropertyDescriptor<void*>         midi_data;
 		PBD::PropertyDescriptor<Evoral::Beats> start_beats;
 		PBD::PropertyDescriptor<Evoral::Beats> length_beats;
 	}
@@ -62,8 +61,6 @@ namespace ARDOUR {
 void
 MidiRegion::make_property_quarks ()
 {
-	Properties::midi_data.property_id = g_quark_from_static_string (X_("midi-data"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for midi-data = %1\n", Properties::midi_data.property_id));
 	Properties::start_beats.property_id = g_quark_from_static_string (X_("start-beats"));
 	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for start-beats = %1\n", Properties::start_beats.property_id));
 	Properties::length_beats.property_id = g_quark_from_static_string (X_("length-beats"));
@@ -245,11 +242,16 @@ MidiRegion::master_read_at (MidiRingBuffer<framepos_t>& out, framepos_t position
 }
 
 framecnt_t
-MidiRegion::_read_at (const SourceList& /*srcs*/, Evoral::EventSink<framepos_t>& dst, framepos_t position, framecnt_t dur, uint32_t chan_n,
-		      NoteMode mode, MidiStateTracker* tracker) const
+MidiRegion::_read_at (const SourceList&              /*srcs*/,
+                      Evoral::EventSink<framepos_t>& dst,
+                      framepos_t                     position,
+                      framecnt_t                     dur,
+                      uint32_t                       chan_n,
+                      NoteMode                       mode,
+                      MidiStateTracker*              tracker) const
 {
 	frameoffset_t internal_offset = 0;
-	framecnt_t to_read         = 0;
+	framecnt_t    to_read         = 0;
 
 	/* precondition: caller has verified that we cover the desired section */
 
@@ -408,24 +410,6 @@ MidiRegion::model_changed ()
 	midi_source()->AutomationStateChanged.connect_same_thread (
 		_model_connection, boost::bind (&MidiRegion::model_automation_state_changed, this, _1)
 		);
-
-	model()->ContentsChanged.connect_same_thread (
-	        _model_contents_connection, boost::bind (&MidiRegion::model_contents_changed, this));
-}
-
-void
-MidiRegion::model_contents_changed ()
-{
-	{
-		/* Invalidate source iterator to force reading new contents even if the
-		   calls to read() progress linearly.  Try-lock only to avoid deadlock
-		   when called while writing with the source already locked. */
-		Glib::Threads::Mutex::Lock lm (midi_source(0)->mutex(), Glib::Threads::TRY_LOCK);
-		if (lm.locked()) {
-			midi_source(0)->invalidate (lm);
-		}
-	}
-	send_change (PropertyChange (Properties::midi_data));
 }
 
 void
@@ -448,6 +432,7 @@ MidiRegion::model_automation_state_changed (Evoral::Parameter const & p)
 	*/
 	Glib::Threads::Mutex::Lock lm (midi_source(0)->mutex(), Glib::Threads::TRY_LOCK);
 	if (lm.locked()) {
+		/* TODO: This is too aggressive, we need more fine-grained invalidation. */
 		midi_source(0)->invalidate (lm);
 	}
 }
diff --git a/libs/ardour/midi_source.cc b/libs/ardour/midi_source.cc
index 8d3b130594..47cfb1b0a0 100644
--- a/libs/ardour/midi_source.cc
+++ b/libs/ardour/midi_source.cc
@@ -177,10 +177,10 @@ MidiSource::update_length (framecnt_t)
 }
 
 void
-MidiSource::invalidate (const Lock& lock)
+MidiSource::invalidate (const Lock& lock, std::set<Evoral::Sequence<Evoral::Beats>::WeakNotePtr>* notes)
 {
 	_model_iter_valid = false;
-	_model_iter.invalidate();
+	_model_iter.invalidate(notes);
 }
 
 framecnt_t
@@ -201,10 +201,15 @@ MidiSource::midi_read (const Lock&                        lm,
 	if (_model) {
 		// Find appropriate model iterator
 		Evoral::Sequence<Evoral::Beats>::const_iterator& i = _model_iter;
-		if (_last_read_end == 0 || start != _last_read_end || !_model_iter_valid) {
+		const bool linear_read = _last_read_end != 0 && start == _last_read_end;
+		if (!linear_read || !_model_iter_valid) {
 			// Cached iterator is invalid, search for the first event past start
-			i                 = _model->begin(converter.from(start), false, filtered);
+			i = _model->begin(converter.from(start), false, filtered,
+			                  linear_read ? &_model->active_notes() : NULL);
 			_model_iter_valid = true;
+			if (!linear_read) {
+				_model->active_notes().clear();
+			}
 		}
 
 		_last_read_end = start + cnt;
diff --git a/libs/ardour/note_fixer.cc b/libs/ardour/note_fixer.cc
new file mode 100644
index 0000000000..749d873779
--- /dev/null
+++ b/libs/ardour/note_fixer.cc
@@ -0,0 +1,145 @@
+/*
+    Copyright (C) 2015 Paul Davis
+    Author: David Robillard
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+#include "evoral/EventList.hpp"
+
+#include "ardour/beats_frames_converter.h"
+#include "ardour/midi_state_tracker.h"
+#include "ardour/note_fixer.h"
+#include "ardour/tempo.h"
+
+namespace ARDOUR {
+
+NoteFixer::~NoteFixer()
+{
+	clear();
+}
+
+void
+NoteFixer::clear()
+{
+	for (Events::iterator i = _events.begin(); i != _events.end(); ++i) {
+		delete *i;
+	}
+}
+
+void
+NoteFixer::prepare(TempoMap&                          tempo_map,
+                   const MidiModel::NoteDiffCommand*  cmd,
+                   const framepos_t                   origin,
+                   const framepos_t                   pos,
+                   std::set< boost::weak_ptr<Note> >& active_notes)
+{
+	typedef MidiModel::NoteDiffCommand Command;
+
+	BeatsFramesConverter converter(tempo_map, origin);
+
+	for (Command::NoteList::const_iterator i = cmd->removed_notes().begin();
+	     i != cmd->removed_notes().end(); ++i) {
+		if (note_is_active(converter, *i, pos)) {
+			/* Deleted note spans the end of the latest read, so we will never
+			   read its off event.  Emit a note off to prevent a stuck note. */
+			_events.push_back(copy_event(pos, (*i)->off_event()));
+			active_notes.erase(*i);
+		}
+	}
+
+	for (Command::NoteList::const_iterator i = cmd->added_notes().begin();
+	     i != cmd->added_notes().end(); ++i) {
+		if (note_is_active(converter, *i, pos)) {
+			/* Added note spans the end of the latest read, so we missed its on
+			   event.  Emit note on immediately to make the state consistent. */
+			_events.push_back(copy_event(pos, (*i)->on_event()));
+			active_notes.insert(*i);
+		}
+	}
+
+	for (Command::ChangeList::const_iterator i = cmd->changes().begin();
+	     i != cmd->changes().end(); ++i) {
+		if (!note_is_active(converter, i->note, pos)) {
+			/* Note is not currently active, no compensation needed. */
+			continue;
+		}
+
+		/* Changed note spans the end of the latest read. */
+		if (i->property == Command::NoteNumber) {
+			/* Note number has changed, end the old note. */
+			_events.push_back(copy_event(pos, i->note->off_event()));
+
+			/* Start a new note on the new note number.  The same note object
+			   is active, so we leave active_notes alone. */
+			Event* on = copy_event(pos, i->note->on_event());
+			on->buffer()[1] = (uint8_t)i->new_value.get_int();
+			_events.push_back(on);
+		} else if (i->property == Command::StartTime &&
+		           converter.to(i->new_value.get_beats()) >= pos) {
+			/* Start time has moved from before to after the end of the
+			   latest read, end the old note. */
+			_events.push_back(copy_event(pos, i->note->off_event()));
+			active_notes.erase(i->note);
+		} else if (i->property == Command::Length &&
+		           converter.to(i->note->time() + i->new_value.get_beats()) < pos) {
+			/* Length has shortened to before the end of the latest read,
+			   end the note. */
+			_events.push_back(copy_event(pos, i->note->off_event()));
+			active_notes.erase(i->note);
+		} else if (i->property == Command::Channel) {
+			/* Channel has changed, end the old note. */
+			_events.push_back(copy_event(pos, i->note->off_event()));
+
+			/* Start a new note on the new channel.  See number change above. */
+			Event* on = copy_event(pos, i->note->on_event());
+			on->buffer()[0] &= 0xF0;
+			on->buffer()[0] |= (uint8_t)i->new_value.get_int();
+			_events.push_back(on);
+		}
+	}
+}
+
+void
+NoteFixer::emit(Evoral::EventSink<framepos_t>& dst,
+                framepos_t                     pos,
+                MidiStateTracker&              tracker)
+{
+	for (Events::iterator i = _events.begin(); i != _events.end(); ++i) {
+		dst.write(pos, (*i)->event_type(), (*i)->size(), (*i)->buffer());
+		tracker.track(**i);
+		delete *i;
+	}
+	_events.clear();
+}
+
+NoteFixer::Event*
+NoteFixer::copy_event(framepos_t time, const Evoral::Event<Evoral::Beats>& ev)
+{
+	return new Event(ev.event_type(), time, ev.size(), ev.buffer());
+}
+
+bool
+NoteFixer::note_is_active(const BeatsFramesConverter& converter,
+                          boost::shared_ptr<Note>     note,
+                          framepos_t                  pos)
+{
+	const framepos_t start_time = converter.to(note->time());
+	const framepos_t end_time   = converter.to(note->end_time());
+
+	return (start_time < pos && end_time >= pos);
+}
+
+} // namespace ARDOUR
diff --git a/libs/ardour/wscript b/libs/ardour/wscript
index 9b2e9c4b2c..293915cd74 100644
--- a/libs/ardour/wscript
+++ b/libs/ardour/wscript
@@ -132,6 +132,7 @@ libardour_sources = [
         'mididm.cc',
         'mtdm.cc',
         'mute_master.cc',
+        'note_fixer.cc',
         'onset_detector.cc',
         'operations.cc',
         'pan_controllable.cc',