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.

5 files changed, 151 insertions, 22 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__ */
+
+