Better MidiModel command framework, ready to go for all your canvas editing needs.

Rewrote MidiEvent to be a well-behaved self-contained object that optionally owns it's buffer, has proper copying semantics, etc.
Fixed crazy bugs triggered by adding lots of events with varying times to a region.
Speed up initial session display significantly (don't redraw each MIDI region tons of times, though still happens more than once and can use fixing...).


git-svn-id: svn://localhost/ardour2/trunk@2213 d708f5d6-7413-0410-9779-e7cbd77b26cf

12 files changed, 238 insertions, 126 deletions

diff --git a/libs/ardour/ardour/midi_event.h b/libs/ardour/ardour/midi_event.h
index 8aca2d14da..a0816d5105 100644
--- a/libs/ardour/ardour/midi_event.h
+++ b/libs/ardour/ardour/midi_event.h
@@ -30,17 +30,75 @@ namespace ARDOUR {
  * tempo time, used in MidiModel) depending on context.
  */
 struct MidiEvent {
-	MidiEvent(double t=0, size_t s=0, Byte* b=NULL)
-		: time(t), size(s), buffer(b)
-	{}
+	MidiEvent(bool owns_buffer=false, double t=0, size_t s=0, Byte* b=NULL)
+		: _owns_buffer(owns_buffer)
+		, _time(t)
+		, _size(s)
+		, _buffer(b)
+	{
+		if (owns_buffer) {
+			_buffer = (Byte*)malloc(_size);
+			if (b)
+				memcpy(_buffer, b, _size);
+			else
+				memset(_buffer, 0, _size);
+		}
+	}
+	
+	MidiEvent(const MidiEvent& copy, bool owns_buffer)
+		: _owns_buffer(owns_buffer)
+		, _time(copy._time)
+		, _size(copy._size)
+		, _buffer(copy._buffer)
+	{
+		if (owns_buffer) {
+			_buffer = (Byte*)malloc(_size);
+			if (copy._buffer)
+				memcpy(_buffer, copy._buffer, _size);
+			else
+				memset(_buffer, 0, _size);
+		}
+	}
+	
+	~MidiEvent() {
+		if (_owns_buffer)
+			free(_buffer);
+	}
 
-	inline uint8_t type()     const { return (buffer[0] & 0xF0); }
-	inline uint8_t note()     const { return (buffer[1]); }
-	inline uint8_t velocity() const { return (buffer[2]); }
+	inline const MidiEvent& operator=(const MidiEvent& copy) {
+		_time = copy._time;
+		if (!_owns_buffer) {
+			_buffer = copy._buffer;
+		} else if (copy._buffer) {
+			if (!_buffer || _size < copy._size)
+				_buffer = (Byte*)realloc(_buffer, copy._size);
+			memcpy(_buffer, copy._buffer, copy._size);
+		} else {
+			free(_buffer);
+			_buffer = NULL;
+		}
+		_size = copy._size;
+		return *this;
+	}
 
-	double time;   /**< Sample index (or beat time) at which event is valid */
-	size_t size;   /**< Number of bytes of data in \a buffer */
-	Byte*  buffer; /**< Raw MIDI data */
+	inline bool        owns_buffer() const { return _owns_buffer; }
+	inline double      time()        const { return _time; }
+	inline double&     time()              { return _time; }
+	inline size_t      size()        const { return _size; }
+	inline size_t&     size()              { return _size; }
+	inline uint8_t     type()        const { return (_buffer[0] & 0xF0); }
+	inline uint8_t     note()        const { return (_buffer[1]); }
+	inline uint8_t     velocity()    const { return (_buffer[2]); }
+	inline const Byte* buffer()      const { return _buffer; }
+	inline Byte*       buffer()            { return _buffer; }
+
+	void set_buffer(Byte* buf) { assert(!_owns_buffer); _buffer = buf; }
+
+private:
+	bool   _owns_buffer; /**< Whether buffer is locally allocated */
+	double _time;         /**< Sample index (or beat time) at which event is valid */
+	size_t _size;         /**< Number of bytes of data in \a buffer */
+	Byte*  _buffer;       /**< Raw MIDI data */
 };
 
 
diff --git a/libs/ardour/ardour/midi_model.h b/libs/ardour/ardour/midi_model.h
index a2e2275157..994d15d5b8 100644
--- a/libs/ardour/ardour/midi_model.h
+++ b/libs/ardour/ardour/midi_model.h
@@ -43,17 +43,19 @@ public:
 	struct Note {
 		Note(double time=0, double dur=0, uint8_t note=0, uint8_t vel=0x40);
 		Note(const Note& copy);
+		
+		const MidiModel::Note& operator=(const MidiModel::Note& copy);
 
 		inline bool operator==(const Note& other)
 			{ return time() == other.time() && note() == other.note(); }
 
-		inline double  time()     const { return _on_event.time; }
-		inline double  end_time() const { return _off_event.time; }
+		inline double  time()     const { return _on_event.time(); }
+		inline double  end_time() const { return _off_event.time(); }
 		inline uint8_t note()     const { return _on_event.note(); }
 		inline uint8_t velocity() const { return _on_event.velocity(); }
-		inline double  duration() const { return _off_event.time - _on_event.time; }
+		inline double  duration() const { return _off_event.time() - _on_event.time(); }
 
-		inline void set_duration(double d) { _off_event.time = _on_event.time + d; }
+		inline void set_duration(double d) { _off_event.time() = _on_event.time() + d; }
 
 		inline MidiEvent& on_event()  { return _on_event; }
 		inline MidiEvent& off_event() { return _off_event; }
@@ -62,10 +64,9 @@ public:
 		inline const MidiEvent& off_event() const { return _off_event; }
 
 	private:
+		// Event buffers are self-contained
 		MidiEvent _on_event;
 		MidiEvent _off_event;
-		Byte      _on_event_buffer[3];
-		Byte      _off_event_buffer[3];
 	};
 
 	MidiModel(Session& s, size_t size=0);
@@ -106,23 +107,18 @@ public:
 
 	inline       Notes& notes()       { return _notes; }
 	inline const Notes& notes() const { return _notes; }
-
-	void     begin_command();
-	Command* current_command() { return _command; }
-	void     finish_command();
-
-	// Commands
-	void add_note(const Note& note);
-	void remove_note(const Note& note);
-
-	sigc::signal<void> ContentsChanged;
 	
-private:
-	class MidiEditCommand : public Command
+	/** Add/Remove notes.
+	 * Technically all operations can be implemented as one of these.
+	 */
+	class DeltaCommand : public Command
 	{
 	public:
-		MidiEditCommand (MidiModel& m) : _model(m) {}
-		//MidiEditCommand (MidiModel&, const XMLNode& node);
+		DeltaCommand (MidiModel& m, const std::string& name)
+			: Command(name), _model(m), _name(name) {}
+		//DeltaCommand (MidiModel&, const XMLNode& node);
+
+		const std::string& name() const { return _name; }
 		
 		void operator()();
 		void undo();
@@ -130,15 +126,28 @@ private:
 		/*int set_state (const XMLNode&);
 		XMLNode& get_state ();*/
 
-		void add_note(const Note& note);
-		void remove_note(const Note& note);
+		void add(const Note& note);
+		void remove(const Note& note);
 
 	private:
 		MidiModel&      _model;
+		std::string     _name;
 		std::list<Note> _added_notes;
 		std::list<Note> _removed_notes;
 	};
 
+	MidiModel::DeltaCommand* new_delta_command(const std::string name="midi edit");
+	void                     apply_command(Command* cmd);
+
+	sigc::signal<void> ContentsChanged;
+	
+private:
+	friend class DeltaCommand;
+	void add_note(const Note& note);
+	void remove_note(const Note& note);
+
+	bool is_sorted() const;
+
 	void append_note_on(double time, uint8_t note, uint8_t velocity);
 	void append_note_off(double time, uint8_t note);
 
@@ -150,8 +159,6 @@ private:
 	typedef std::vector<size_t> WriteNotes;
 	WriteNotes _write_notes;
 	bool       _writing;
-
-	MidiEditCommand* _command; ///< In-progress command
 };
 
 } /* namespace ARDOUR */
diff --git a/libs/ardour/ardour/midi_ring_buffer.h b/libs/ardour/ardour/midi_ring_buffer.h
index 6c57c78c3c..21beebacd2 100644
--- a/libs/ardour/ardour/midi_ring_buffer.h
+++ b/libs/ardour/ardour/midi_ring_buffer.h
@@ -288,24 +288,24 @@ MidiRingBuffer::read(MidiBuffer& dst, nframes_t start, nframes_t end, nframes_t
 
 	while (read_space() > sizeof(double) + sizeof(size_t)) {
 	
-		full_peek(sizeof(double), (Byte*)&ev.time);
+		full_peek(sizeof(double), (Byte*)&ev.time());
 	
-		if (ev.time > end)
+		if (ev.time() > end)
 			break;
 
-		bool success = MidiRingBufferBase<Byte>::full_read(sizeof(double), (Byte*)&ev.time);
+		bool success = MidiRingBufferBase<Byte>::full_read(sizeof(double), (Byte*)&ev.time());
 		if (success)
-			success = MidiRingBufferBase<Byte>::full_read(sizeof(size_t), (Byte*)&ev.size);
+			success = MidiRingBufferBase<Byte>::full_read(sizeof(size_t), (Byte*)&ev.size());
 		
 		if (!success) {
 			std::cerr << "MRB: READ ERROR (time/size)" << std::endl;
 			continue;
 		}
 
-		if (ev.time >= start) {
-			ev.time -= start;
-			Byte* write_loc = dst.reserve(ev.time, ev.size);
-			success = MidiRingBufferBase<Byte>::full_read(ev.size, write_loc);
+		if (ev.time() >= start) {
+			ev.time() -= start;
+			Byte* write_loc = dst.reserve(ev.time(), ev.size());
+			success = MidiRingBufferBase<Byte>::full_read(ev.size(), write_loc);
 		
 			if (success) {
 				++count;
@@ -315,7 +315,7 @@ MidiRingBuffer::read(MidiBuffer& dst, nframes_t start, nframes_t end, nframes_t
 			}
 			
 		} else {
-			printf("MRB - SKIPPING EVENT (with time %f)\n", ev.time);
+			printf("MRB - SKIPPING EVENT (with time %f)\n", ev.time());
 		}
 	}
 	
diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h
index 5ba2e39526..05d87620f2 100644
--- a/libs/ardour/ardour/session.h
+++ b/libs/ardour/ardour/session.h
@@ -774,7 +774,7 @@ class Session : public PBD::StatefulDestructible
 	string next_undo() const { return _history.next_undo(); }
 	string next_redo() const { return _history.next_redo(); }
 
-	void begin_reversible_command (string cmd_name);
+	void begin_reversible_command (const string& cmd_name);
 	void commit_reversible_command (Command* cmd = 0);
 
 	void add_command (Command *const cmd) {
diff --git a/libs/ardour/meter.cc b/libs/ardour/meter.cc
index 84dac2ded4..74a3e61f9a 100644
--- a/libs/ardour/meter.cc
+++ b/libs/ardour/meter.cc
@@ -49,8 +49,8 @@ PeakMeter::run (BufferSet& bufs, nframes_t start_frame, nframes_t end_frame, nfr
 		// expressed through peaks alone
 		for (MidiBuffer::iterator i = bufs.get_midi(n).begin(); i != bufs.get_midi(n).end(); ++i) {
 			const MidiEvent& ev = *i;
-			if ((ev.buffer[0] & 0xF0) == MIDI_CMD_NOTE_ON) {
-				const float this_vel = log(ev.buffer[2] / 127.0 * (M_E*M_E-M_E) + M_E) - 1.0;
+			if ((ev.buffer()[0] & 0xF0) == MIDI_CMD_NOTE_ON) {
+				const float this_vel = log(ev.buffer()[2] / 127.0 * (M_E*M_E-M_E) + M_E) - 1.0;
 				//printf("V %d -> %f\n", (int)((Byte)ev.buffer[2]), this_vel);
 				if (this_vel > val)
 					val = this_vel;
diff --git a/libs/ardour/midi_buffer.cc b/libs/ardour/midi_buffer.cc
index 5b7c789ba0..07ae2abab0 100644
--- a/libs/ardour/midi_buffer.cc
+++ b/libs/ardour/midi_buffer.cc
@@ -90,11 +90,11 @@ MidiBuffer::read_from(const Buffer& src, nframes_t nframes, nframes_t offset)
 	// FIXME: slow
 	for (size_t i=0; i < src.size(); ++i) {
 		const MidiEvent& ev = msrc[i];
-		if (ev.time >= offset && ev.time < offset+nframes) {
-			//cerr << "MidiBuffer::read_from got event, " << ev.time << endl;
+		if (ev.time() >= offset && ev.time() < offset+nframes) {
+			//cerr << "MidiBuffer::read_from got event, " << ev.time() << endl;
 			push_back(ev);
 		} else {
-			//cerr << "MidiBuffer event out of range, " << ev.time << endl;
+			//cerr << "MidiBuffer event out of range, " << ev.time() << endl;
 		}
 	}
 
@@ -117,9 +117,9 @@ MidiBuffer::push_back(const MidiEvent& ev)
 
 	Byte* const write_loc = _data + (_size * MAX_EVENT_SIZE);
 
-	memcpy(write_loc, ev.buffer, ev.size);
+	memcpy(write_loc, ev.buffer(), ev.size());
 	_events[_size] = ev;
-	_events[_size].buffer = write_loc;
+	_events[_size].set_buffer(write_loc);
 	++_size;
 
 	//cerr << "MidiBuffer: pushed, size = " << _size << endl;
@@ -146,9 +146,9 @@ MidiBuffer::push_back(const jack_midi_event_t& ev)
 	Byte* const write_loc = _data + (_size * MAX_EVENT_SIZE);
 
 	memcpy(write_loc, ev.buffer, ev.size);
-	_events[_size].time = (double)ev.time;
-	_events[_size].size = ev.size;
-	_events[_size].buffer = write_loc;
+	_events[_size].time() = (double)ev.time;
+	_events[_size].size() = ev.size;
+	_events[_size].set_buffer(write_loc);
 	++_size;
 
 	//cerr << "MidiBuffer: pushed, size = " << _size << endl;
@@ -176,9 +176,9 @@ MidiBuffer::reserve(double time, size_t size)
 
 	Byte* const write_loc = _data + (_size * MAX_EVENT_SIZE);
 
-	_events[_size].time = time;
-	_events[_size].size = size;
-	_events[_size].buffer = write_loc;
+	_events[_size].time() = time;
+	_events[_size].size() = size;
+	_events[_size].set_buffer(write_loc);
 	++_size;
 
 	//cerr << "MidiBuffer: reserved, size = " << _size << endl;
@@ -238,7 +238,7 @@ MidiBuffer::merge(const MidiBuffer& a, const MidiBuffer& b)
 			const MidiEvent& a_ev = a[a_index];
 			const MidiEvent& b_ev = b[b_index];
 
-			if (a_ev.time <= b_ev.time) {
+			if (a_ev.time() <= b_ev.time()) {
 				push_back(a_ev);
 				++a_index;
 			} else {
diff --git a/libs/ardour/midi_diskstream.cc b/libs/ardour/midi_diskstream.cc
index 3fed0cf68c..7137772193 100644
--- a/libs/ardour/midi_diskstream.cc
+++ b/libs/ardour/midi_diskstream.cc
@@ -555,7 +555,7 @@ MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t
 
 		for (size_t i=0; i < to_write; ++i) {
 			const MidiEvent& ev = *port_iter;
-			_capture_buf->write(ev.time + transport_frame, ev.size, ev.buffer);
+			_capture_buf->write(ev.time() + transport_frame, ev.size(), ev.buffer());
 			++port_iter;
 		}
 	
diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc
index 7374916c14..27bb681c43 100644
--- a/libs/ardour/midi_model.cc
+++ b/libs/ardour/midi_model.cc
@@ -19,6 +19,7 @@
 */
 
 #include <iostream>
+#include <algorithm>
 #include <queue>
 #include <pbd/enumwriter.h>
 #include <ardour/midi_model.h>
@@ -32,21 +33,17 @@ using namespace ARDOUR;
 // Note
 
 MidiModel::Note::Note(double t, double d, uint8_t n, uint8_t v)
+	: _on_event(true, t, 3, NULL)
+	, _off_event(true, t + d, 3, NULL)
 {
-	_on_event.time = t;
-	_on_event.buffer = _on_event_buffer;
-	_on_event.size = 3;
-	_on_event.buffer[0] = MIDI_CMD_NOTE_ON;
-	_on_event.buffer[1] = n;
-	_on_event.buffer[2] = v;
+	_on_event.buffer()[0] = MIDI_CMD_NOTE_ON;
+	_on_event.buffer()[1] = n;
+	_on_event.buffer()[2] = v;
+	
+	_off_event.buffer()[0] = MIDI_CMD_NOTE_OFF;
+	_off_event.buffer()[1] = n;
+	_off_event.buffer()[2] = 0x40;
 	
-	_off_event.time = t + d;
-	_off_event.buffer = _off_event_buffer;
-	_off_event.size = 3;
-	_off_event.buffer[0] = MIDI_CMD_NOTE_OFF;
-	_off_event.buffer[1] = n;
-	_off_event.buffer[2] = 0x40;
-
 	assert(time() == t);
 	assert(duration() == d);
 	assert(note() == n);
@@ -55,16 +52,50 @@ MidiModel::Note::Note(double t, double d, uint8_t n, uint8_t v)
 
 
 MidiModel::Note::Note(const Note& copy)
-	: _on_event(copy._on_event)
-	, _off_event(copy._off_event)
+	: _on_event(copy._on_event, true)
+	, _off_event(copy._off_event, true)
 {
-	memcpy(_on_event_buffer, copy._on_event_buffer, 3);
-	memcpy(_off_event_buffer, copy._off_event_buffer, 3);
+	/*
+	assert(copy._on_event.size == 3);
 	_on_event.buffer = _on_event_buffer;
+	memcpy(_on_event_buffer, copy._on_event_buffer, 3);
+	
+	assert(copy._off_event.size == 3);
 	_off_event.buffer = _off_event_buffer;
+	memcpy(_off_event_buffer, copy._off_event_buffer, 3);
+	*/
+
+	assert(time() == copy.time());
+	assert(end_time() == copy.end_time());
+	assert(note() == copy.note());
+	assert(velocity() == copy.velocity());
+	assert(duration() == copy.duration());
 }
 
 
+const MidiModel::Note&
+MidiModel::Note::operator=(const Note& copy)
+{
+	_on_event = copy._on_event;
+	_off_event = copy._off_event;
+	/*_on_event.time = copy._on_event.time;
+	assert(copy._on_event.size == 3);
+	memcpy(_on_event_buffer, copy._on_event_buffer, 3);
+	
+	_off_event.time = copy._off_event.time;
+	assert(copy._off_event.size == 3);
+	memcpy(_off_event_buffer, copy._off_event_buffer, 3);
+	*/
+	
+	assert(time() == copy.time());
+	assert(end_time() == copy.end_time());
+	assert(note() == copy.note());
+	assert(velocity() == copy.velocity());
+	assert(duration() == copy.duration());
+
+	return *this;
+}
+
 // MidiModel
 
 MidiModel::MidiModel(Session& s, size_t size)
@@ -72,7 +103,6 @@ MidiModel::MidiModel(Session& s, size_t size)
 	, _notes(size)
 	, _note_mode(Sustained)
 	, _writing(false)
-	, _command(NULL)
 {
 }
 
@@ -105,8 +135,8 @@ MidiModel::read (MidiRingBuffer& dst, nframes_t start, nframes_t nframes, nframe
 			while ( ! active_notes.empty() ) {
 				const Note* const earliest_off = active_notes.top();
 				const MidiEvent& ev = earliest_off->off_event();
-				if (ev.time < start + nframes && ev.time <= n->time()) {
-					dst.write(ev.time + stamp_offset, ev.size, ev.buffer);
+				if (ev.time() < start + nframes && ev.time() <= n->time()) {
+					dst.write(ev.time() + stamp_offset, ev.size(), ev.buffer());
 					active_notes.pop();
 					++read_events;
 				} else {
@@ -117,7 +147,7 @@ MidiModel::read (MidiRingBuffer& dst, nframes_t start, nframes_t nframes, nframe
 			// Note on
 			if (n->time() >= start) {
 				const MidiEvent& ev = n->on_event();
-				dst.write(ev.time + stamp_offset, ev.size, ev.buffer);
+				dst.write(ev.time() + stamp_offset, ev.size(), ev.buffer());
 				active_notes.push(&(*n));
 				++read_events;
 			}
@@ -131,7 +161,7 @@ MidiModel::read (MidiRingBuffer& dst, nframes_t start, nframes_t nframes, nframe
 			if (n->time() >= start) {
 				if (n->time() < start + nframes) {
 					const MidiEvent& ev = n->on_event();
-					dst.write(ev.time + stamp_offset, ev.size, ev.buffer);
+					dst.write(ev.time() + stamp_offset, ev.size(), ev.buffer());
 					++read_events;
 				} else {
 					break;
@@ -209,12 +239,12 @@ MidiModel::append(const MidiBuffer& buf)
 	for (MidiBuffer::const_iterator i = buf.begin(); i != buf.end(); ++i) {
 		const MidiEvent& ev = *i;
 		
-		assert(_notes.empty() || ev.time >= _notes.back().time());
+		assert(_notes.empty() || ev.time() >= _notes.back().time());
 
 		if (ev.type() == MIDI_CMD_NOTE_ON)
-			append_note_on(ev.time, ev.note(), ev.velocity());
+			append_note_on(ev.time(), ev.note(), ev.velocity());
 		else if (ev.type() == MIDI_CMD_NOTE_OFF)
-			append_note_off(ev.time, ev.note());
+			append_note_off(ev.time(), ev.note());
 	}
 }
 
@@ -285,42 +315,60 @@ MidiModel::append_note_off(double time, uint8_t note_num)
 void
 MidiModel::add_note(const Note& note)
 {
-	// FIXME: take source lock
 	Notes::iterator i = upper_bound(_notes.begin(), _notes.end(), note, note_time_comparator);
 	_notes.insert(i, note);
-	if (_command)
-		_command->add_note(note);
 }
 
 
 void
 MidiModel::remove_note(const Note& note)
 {
-	// FIXME: take source lock
 	Notes::iterator n = find(_notes.begin(), _notes.end(), note);
 	if (n != _notes.end())
 		_notes.erase(n);
-	
-	if (_command)
-		_command->remove_note(note);
 }
 
+/** Slow!  for debugging only. */
+bool
+MidiModel::is_sorted() const
+{
+	bool t = 0;
+	for (Notes::const_iterator n = _notes.begin(); n != _notes.end(); ++n)
+		if (n->time() < t)
+			return false;
+		else
+			t = n->time();
+
+	return true;
+}
 
 
-void
-MidiModel::begin_command()
+/** Start a new command.
+ *
+ * This has no side-effects on the model or Session, the returned command
+ * can be held on to for as long as the caller wishes, or discarded without
+ * formality, until apply_command is called and ownership is taken.
+ */
+MidiModel::DeltaCommand*
+MidiModel::new_delta_command(const string name)
 {
-	assert(!_command);
-	_session.begin_reversible_command("midi edit");
-	_command = new MidiEditCommand(*this);
+	DeltaCommand* cmd =  new DeltaCommand(*this, name);
+	return cmd;
 }
 
 
+/** Apply a command.
+ *
+ * Ownership of cmd is taken, it must not be deleted by the caller.
+ * The command will constitute one item on the undo stack.
+ */
 void
-MidiModel::finish_command()
+MidiModel::apply_command(Command* cmd)
 {
-	_session.commit_reversible_command(_command);
-	_command = NULL;
+	_session.begin_reversible_command(cmd->name());
+	(*cmd)();
+	assert(is_sorted());
+	_session.commit_reversible_command(cmd);
 }
 
 
@@ -328,7 +376,7 @@ MidiModel::finish_command()
 
 
 void
-MidiModel::MidiEditCommand::add_note(const Note& note)
+MidiModel::DeltaCommand::add(const Note& note)
 {
 	//cerr << "MEC: apply" << endl;
 
@@ -338,7 +386,7 @@ MidiModel::MidiEditCommand::add_note(const Note& note)
 
 
 void
-MidiModel::MidiEditCommand::remove_note(const Note& note)
+MidiModel::DeltaCommand::remove(const Note& note)
 {
 	//cerr << "MEC: remove" << endl;
 
@@ -348,26 +396,26 @@ MidiModel::MidiEditCommand::remove_note(const Note& note)
 
 		
 void 
-MidiModel::MidiEditCommand::operator()()
+MidiModel::DeltaCommand::operator()()
 {
-	//cerr << "MEC: apply" << endl;
-	assert(!_model.current_command());
+	// This could be made much faster by using a priority_queue for added and
+	// removed notes (or sort here), and doing a single iteration over _model
 	
 	for (std::list<Note>::iterator i = _added_notes.begin(); i != _added_notes.end(); ++i)
 		_model.add_note(*i);
 	
 	for (std::list<Note>::iterator i = _removed_notes.begin(); i != _removed_notes.end(); ++i)
 		_model.remove_note(*i);
-
+	
 	_model.ContentsChanged(); /* EMIT SIGNAL */
 }
 
 
 void
-MidiModel::MidiEditCommand::undo()
+MidiModel::DeltaCommand::undo()
 {
-	//cerr << "MEC: undo" << endl;
-	assert(!_model.current_command());
+	// This could be made much faster by using a priority_queue for added and
+	// removed notes (or sort here), and doing a single iteration over _model
 
 	for (std::list<Note>::iterator i = _added_notes.begin(); i != _added_notes.end(); ++i)
 		_model.remove_note(*i);
diff --git a/libs/ardour/midi_port.cc b/libs/ardour/midi_port.cc
index 2dc90a09da..3f03cfe880 100644
--- a/libs/ardour/midi_port.cc
+++ b/libs/ardour/midi_port.cc
@@ -102,9 +102,9 @@ MidiPort::cycle_end()
 	for (MidiBuffer::iterator i = _buffer.begin(); i != _buffer.end(); ++i) {
 		const MidiEvent& ev = *i;
 		// event times should be frames, relative to cycle start
-		assert(ev.time >= 0);
-		assert(ev.time < _nframes_this_cycle);
-		jack_midi_event_write(jack_buffer, (jack_nframes_t)ev.time, ev.buffer, ev.size);
+		assert(ev.time() >= 0);
+		assert(ev.time() < _nframes_this_cycle);
+		jack_midi_event_write(jack_buffer, (jack_nframes_t)ev.time(), ev.buffer(), ev.size());
 	}
 	
 	_nframes_this_cycle = 0;
diff --git a/libs/ardour/midi_track.cc b/libs/ardour/midi_track.cc
index 55d195fab0..ae1c8f1a07 100644
--- a/libs/ardour/midi_track.cc
+++ b/libs/ardour/midi_track.cc
@@ -584,11 +584,10 @@ MidiTrack::write_controller_messages(MidiBuffer& output_buf, nframes_t start_fra
 	// Write immediate events (UI controls)
 	MidiBuffer& cc_buf = mix_buffers.get_midi(0);
 	cc_buf.silence(nframes, offset);
-	MidiEvent ev;
-	ev.size = 3; // CC = 3 bytes
-	Byte buf[ev.size];
+	
+	Byte buf[3]; // CC = 3 bytes
 	buf[0] = MIDI_CMD_CONTROL;
-	ev.buffer = buf;
+	MidiEvent ev(false, 0, 3, buf);
 
 	// Write controller automation
 	if (_session.transport_rolling()) {
@@ -611,9 +610,9 @@ MidiTrack::write_controller_messages(MidiBuffer& output_buf, nframes_t start_fra
 				assert(y >= 0.0);
 				assert(y <= 127.0);
 
-				ev.time = stamp;
-				ev.buffer[1] = (Byte)list->parameter().id();
-				ev.buffer[2] = (Byte)y;
+				ev.time() = stamp;
+				ev.buffer()[1] = (Byte)list->parameter().id();
+				ev.buffer()[2] = (Byte)y;
 
 				cc_buf.push_back(ev);
 
diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc
index 94aae01a53..f6debff123 100644
--- a/libs/ardour/session_state.cc
+++ b/libs/ardour/session_state.cc
@@ -2027,7 +2027,7 @@ Session::edit_group_by_name (string name)
 }
 
 void
-Session::begin_reversible_command (string name)
+Session::begin_reversible_command (const string& name)
 {
 	current_trans = new UndoTransaction;
 	current_trans->set_name (name);
diff --git a/libs/ardour/smf_source.cc b/libs/ardour/smf_source.cc
index d9a46ec782..4ae1cd34ab 100644
--- a/libs/ardour/smf_source.cc
+++ b/libs/ardour/smf_source.cc
@@ -382,10 +382,10 @@ SMFSource::write_unlocked (MidiRingBuffer& src, nframes_t cnt)
 	
 	for (MidiBuffer::iterator i = buf.begin(); i != buf.end(); ++i) {
 		MidiEvent& ev = *i;
-		assert(ev.time >= _timeline_position);
-		ev.time -= _timeline_position;
-		assert(ev.time >= _last_ev_time);
-		const uint32_t delta_time = (uint32_t)((ev.time - _last_ev_time) / frames_per_beat * _ppqn);
+		assert(ev.time() >= _timeline_position);
+		ev.time() -= _timeline_position;
+		assert(ev.time() >= _last_ev_time);
+		const uint32_t delta_time = (uint32_t)((ev.time() - _last_ev_time) / frames_per_beat * _ppqn);
 		
 		/*printf("SMF - writing event, delta = %u, size = %zu, data = ",
 			delta_time, ev.size);
@@ -395,12 +395,12 @@ SMFSource::write_unlocked (MidiRingBuffer& src, nframes_t cnt)
 		printf("\n");
 		*/
 		size_t stamp_size = write_var_len(delta_time);
-		fwrite(ev.buffer, 1, ev.size, _fd);
+		fwrite(ev.buffer(), 1, ev.size(), _fd);
 
-		_track_size += stamp_size + ev.size;
-		_write_data_count += ev.size;
+		_track_size += stamp_size + ev.size();
+		_write_data_count += ev.size();
 		
-		_last_ev_time = ev.time;
+		_last_ev_time = ev.time();
 	}
 
 	fflush(_fd);