From d325e7302c8f5b34459597c9e5c0a5bfbbbb2fb7 Mon Sep 17 00:00:00 2001
From: Paul Davis <paul@linuxaudiosystems.com>
Date: Wed, 7 Dec 2016 11:16:10 +0000
Subject: [PATCH] another wide ranging set of changes to get this MIDI handling
 a step further.

Notably ... reading from MidiRingBuffer into MidiBuffer includes the event size,
rather than relying on the MIDI bytes to determine size. This isn't required
for MIDI, but is a more portable design for other event types.
---
 libs/ardour/ardour/event_ring_buffer.h | 25 ++++++++++++++--
 libs/ardour/ardour/lua_api.h           |  3 +-
 libs/ardour/ardour/midi_buffer.h       |  6 ++--
 libs/ardour/lua_api.cc                 | 11 ++++---
 libs/ardour/midi_buffer.cc             | 31 ++++++++++++++------
 libs/ardour/midi_model.cc              | 16 +++--------
 libs/ardour/midi_ring_buffer.cc        |  4 +--
 libs/evoral/evoral/Event.hpp           | 40 ++++++++++----------------
 libs/evoral/evoral/Note.hpp            | 13 ++++++---
 libs/evoral/src/Note.cpp               | 22 +++++++-------
 libs/evoral/src/Sequence.cpp           | 17 ++++++++---
 11 files changed, 109 insertions(+), 79 deletions(-)

diff --git a/libs/ardour/ardour/event_ring_buffer.h b/libs/ardour/ardour/event_ring_buffer.h
index c7344c5ca7..4a3180b1a5 100644
--- a/libs/ardour/ardour/event_ring_buffer.h
+++ b/libs/ardour/ardour/event_ring_buffer.h
@@ -25,6 +25,7 @@
 
 #include "pbd/ringbufferNPT.h"
 
+#include "evoral/Event.hpp"
 #include "evoral/EventSink.hpp"
 #include "evoral/types.hpp"
 
@@ -61,8 +62,10 @@ public:
 	 */
 	inline bool peek (uint8_t*, size_t size);
 
-	inline uint32_t write(Time  time, Evoral::EventType  type, uint32_t  size, const uint8_t* buf);
-	inline bool     read (Time* time, Evoral::EventType* type, uint32_t* size,       uint8_t* buf);
+	inline uint32_t write (Time  time, Evoral::EventType  type, uint32_t  size, const uint8_t* buf);
+	inline uint32_t write (Evoral::Event<Time> const &);
+
+	inline bool     read (Time* time, Evoral::EventType* type, uint32_t* size, uint8_t* buf);
 };
 
 template<typename Time>
@@ -128,6 +131,24 @@ EventRingBuffer<Time>::write(Time time, Evoral::EventType type, uint32_t size, c
 	}
 }
 
+template<typename Time>
+inline uint32_t
+EventRingBuffer<Time>::write(Evoral::Event<Time> const & ev)
+{
+	if (!buf || write_space() < (sizeof(Time) + sizeof(Evoral::EventType) + sizeof(uint32_t) + size)) {
+		return 0;
+	} else {
+		const Time tm = ev->time();
+		const Evoral::EventType ty = ev->event_type ();
+		const uint32_t sz = ev->size();
+		PBD::RingBufferNPT<uint8_t>::write ((uint8_t*)&tm, sizeof(Time));
+		PBD::RingBufferNPT<uint8_t>::write ((uint8_t*)&ty, sizeof(Evoral::EventType));
+		PBD::RingBufferNPT<uint8_t>::write ((uint8_t*)&sz, sizeof(uint32_t));
+		PBD::RingBufferNPT<uint8_t>::write (ev->buffer(), sz);
+		return size;
+	}
+}
+
 } // namespace ARDOUR
 
 #endif // __ardour_event_ring_buffer_h__
diff --git a/libs/ardour/ardour/lua_api.h b/libs/ardour/ardour/lua_api.h
index 017246cf1d..9851bbc91a 100644
--- a/libs/ardour/ardour/lua_api.h
+++ b/libs/ardour/ardour/lua_api.h
@@ -245,8 +245,7 @@ namespace ARDOUR { namespace LuaAPI {
 
 	};
 
-	boost::shared_ptr<Evoral::Note<Evoral::Beats> >
-		new_noteptr (uint8_t, Evoral::Beats, Evoral::Beats, uint8_t, uint8_t);
+	Evoral::NotePointer<Evoral::Beats> new_noteptr (Evoral::EventPool&, uint8_t, Evoral::Beats, Evoral::Beats, uint8_t, uint8_t);
 
 } } /* namespace */
 
diff --git a/libs/ardour/ardour/midi_buffer.h b/libs/ardour/ardour/midi_buffer.h
index 37a87bd21c..f9a4e02aba 100644
--- a/libs/ardour/ardour/midi_buffer.h
+++ b/libs/ardour/ardour/midi_buffer.h
@@ -51,7 +51,7 @@ public:
 	bool     push_back(const Evoral::Event<TimeType>& event);
 	bool     push_back(TimeType time, size_t size, const uint8_t* data);
 
-	uint8_t* reserve (size_t object_size);
+	uint8_t* reserve (TimeType time, size_t object_size);
 
 	void resize(size_t);
 	size_t size() const { return _size; }
@@ -68,10 +68,10 @@ public:
 	{
 	public:
 		iterator_base<BufferType, EventType>(BufferType& b, framecnt_t o)
-			: buffer(&b), offset(o) {}
+		: buffer(&b), offset(o) { }
 
 		iterator_base<BufferType, EventType>(const iterator_base<BufferType,EventType>& o)
-			: buffer (o.buffer), offset(o.offset) {}
+		: buffer (o.buffer), offset(o.offset) { }
 
 		inline iterator_base<BufferType,EventType> operator= (const iterator_base<BufferType,EventType>& o) {
 			if (&o != this) {
diff --git a/libs/ardour/lua_api.cc b/libs/ardour/lua_api.cc
index 1751104d39..fbd75bc2b2 100644
--- a/libs/ardour/lua_api.cc
+++ b/libs/ardour/lua_api.cc
@@ -655,9 +655,12 @@ LuaAPI::Vamp::process (const std::vector<float*>& d, ::Vamp::RealTime rt)
 	return _plugin->process (bufs, rt);
 }
 
-boost::shared_ptr<Evoral::Note<Evoral::Beats> >
-LuaAPI::new_noteptr (uint8_t chan, Evoral::Beats beat_time, Evoral::Beats length, uint8_t note, uint8_t velocity)
+Evoral::NotePointer<Evoral::Beats>
+LuaAPI::new_noteptr (Evoral::EventPool& pool, uint8_t chan, Evoral::Beats beat_time, Evoral::Beats length, uint8_t note, uint8_t velocity)
 {
-	return boost::shared_ptr<Evoral::Note<Evoral::Beats> > (
-		new Evoral::Note<Evoral::Beats>(chan, beat_time, length, note, velocity));
+	/* creates a new Note, then a NotePointer that references the note,
+	   then returns the NotePointer by reference, causing a (stack) copy or
+	   two of the NotePointer flyweight.
+	*/
+	return Evoral::NotePointer<Evoral::Beats> (pool, chan, beat_time, length, note, velocity);
 }
diff --git a/libs/ardour/midi_buffer.cc b/libs/ardour/midi_buffer.cc
index 0f4b30a7b1..2c2a330b49 100644
--- a/libs/ardour/midi_buffer.cc
+++ b/libs/ardour/midi_buffer.cc
@@ -88,7 +88,6 @@ MidiBuffer::copy(MidiBuffer const * const copy)
 	memcpy(_data, copy->_data, _size);
 }
 
-
 /** Read events from @a src starting at time @a offset into the START of this buffer, for
  * time duration @a nframes.  Relative time, where 0 = start of buffer.
  *
@@ -175,7 +174,7 @@ MidiBuffer::push_back (const Evoral::Event<TimeType>& ev)
 #ifndef NDEBUG
 	if (DEBUG_ENABLED(DEBUG::MidiIO)) {
 		DEBUG_STR_DECL(a);
-		DEBUG_STR_APPEND(a, string_compose ("midibuffer %1 push event @ %2 sz %3 ", this, ev.time(), ev.size()));
+		DEBUG_STR_APPEND(a, string_compose ("midibuffer %1 cursize %4 push event @ %2 sz %3 ", this, ev.time(), ev.size(), _size));
 		for (size_t i=0; i < ev.size(); ++i) {
 			DEBUG_STR_APPEND(a,hex);
 			DEBUG_STR_APPEND(a,"0x");
@@ -259,7 +258,7 @@ MidiBuffer::insert_event(const Evoral::Event<TimeType>& ev)
 #ifndef NDEBUG
 	if (DEBUG_ENABLED(DEBUG::MidiIO)) {
 		DEBUG_STR_DECL(a);
-		DEBUG_STR_APPEND(a, string_compose ("midibuffer %1 insert event @ %2 sz %3 ", this, ev.time(), ev.size()));
+		DEBUG_STR_APPEND(a, string_compose ("midibuffer %1 cursize %4 insert event @ %2 sz %3 ", this, ev.time(), ev.size(), _size));
 		for (size_t i=0; i < ev.size(); ++i) {
 			DEBUG_STR_APPEND(a,hex);
 			DEBUG_STR_APPEND(a,"0x");
@@ -322,14 +321,28 @@ MidiBuffer::write (TimeType time, Evoral::EventType type, uint32_t size, const u
  * location, or the buffer will be corrupted and very nasty things will happen.
  */
 uint8_t*
-MidiBuffer::reserve (size_t object_size)
+MidiBuffer::reserve(TimeType time, size_t size)
 {
-	uint8_t * const write_loc = _data + _size;
-	_size += object_size;
-	_silent = false;
-	return write_loc;
-}
+	const size_t ev_size = Evoral::Event<TimeType>::memory_size (size);
 
+	if (_size + ev_size >= _capacity) {
+		return 0;
+	}
+
+	/* on-stack temporary Event that we can write into the buffer */
+	uint8_t buf[ev_size];
+	Evoral::Event<TimeType>* ev = ::new (buf) Evoral::Event<TimeType> (Evoral::MIDI_EVENT, time, size, 0);
+
+	/* copy just the Event structure itself (no data, that comes later) */
+	memcpy (_data + _size, ev, sizeof (Evoral::Event<TimeType>));
+
+	// record new size as if data has already been written
+	_size += ev_size;
+	_silent = false;
+
+	/* return address where data can be written */
+	return _data + sizeof (Evoral::Event<TimeType>);
+}
 
 void
 MidiBuffer::silence (framecnt_t /*nframes*/, framecnt_t /*offset*/)
diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc
index c64a3af87d..af99c0f3b1 100644
--- a/libs/ardour/midi_model.cc
+++ b/libs/ardour/midi_model.cc
@@ -244,15 +244,9 @@ MidiModel::NoteDiffCommand::operator() ()
 	{
 		MidiModel::WriteLock lock(_model->edit_lock());
 
-		for (NoteList::iterator i = _added_notes.begin(); i != _added_notes.end(); *i) {
+		for (NoteList::iterator i = _added_notes.begin(); i != _added_notes.end(); ++i) {
 
-			/* locally scoped pointer to note */
-
-			NotePtr np (*i);
-
-			cerr << "local NP linked? " << np.is_linked() << " iterator version " << (*i).is_linked() << endl;
-
-			if (!_model->add_note_unlocked (np)) {
+			if (!_model->add_note_unlocked (*i)) {
 				/* failed to add it, so don't leave it in the removed list, to
 				   avoid apparent errors on undo.
 				*/
@@ -261,8 +255,7 @@ MidiModel::NoteDiffCommand::operator() ()
 		}
 
 		for (NoteList::iterator i = _removed_notes.begin(); i != _removed_notes.end(); ++i) {
-			NotePtr np (*i);
-			_model->remove_note_unlocked (np);
+			_model->remove_note_unlocked (*i);
 		}
 
 		/* notes we modify in a way that requires remove-then-add to maintain ordering */
@@ -556,7 +549,7 @@ MidiModel::NoteDiffCommand::unmarshal_note (XMLNode *xml_note)
 		velocity = 127;
 	}
 
-	NotePtr note_ptr(new Evoral::Note<TimeType>(channel, time, length, note, velocity));
+	NotePtr note_ptr (_model->event_pool(), channel, time, length, note, velocity);
 	note_ptr->set_id (id);
 
 	return note_ptr;
@@ -1650,7 +1643,6 @@ MidiModel::resolve_overlaps_unlocked (const NotePtr note, void* arg)
 	TimeType ea  = note->end_time();
 
 	const Pitches& p (pitches (note->channel()));
-	NotePtr search_note (new Note<TimeType>(0, TimeType(), TimeType(), note->note()));
 	Notes to_be_deleted;
 	bool set_note_length = false;
 	bool set_note_time = false;
diff --git a/libs/ardour/midi_ring_buffer.cc b/libs/ardour/midi_ring_buffer.cc
index 8f2642758b..303a831a26 100644
--- a/libs/ardour/midi_ring_buffer.cc
+++ b/libs/ardour/midi_ring_buffer.cc
@@ -88,7 +88,7 @@ MidiRingBuffer<T>::read (MidiBuffer& dst, framepos_t start, framepos_t end, fram
 
 		/* lets see if we are going to be able to write this event into dst.
 		 */
-		uint8_t* write_loc = dst.reserve (ev_size);
+		uint8_t* write_loc = dst.reserve (ev_time, ev_size);
 		if (write_loc == 0) {
 			if (stop_on_overflow_in_dst) {
 				DEBUG_TRACE (DEBUG::MidiRingBuffer, string_compose ("MidiRingBuffer: overflow in destination MIDI buffer, stopped after %1 events\n", count));
@@ -105,7 +105,7 @@ MidiRingBuffer<T>::read (MidiBuffer& dst, framepos_t start, framepos_t end, fram
 #ifndef NDEBUG
 		if (DEBUG_ENABLED (DEBUG::MidiRingBuffer)) {
 			DEBUG_STR_DECL(a);
-			DEBUG_STR_APPEND(a, string_compose ("wrote MidiEvent to Buffer (time=%1, start=%2 offset=%3) ", ev_time, start, offset));
+			DEBUG_STR_APPEND(a, string_compose ("wrote %4 bytes of MidiEvent to Buffer (time=%1, start=%2 offset=%3) ", ev_time, start, offset, ev_size));
 			for (size_t i=0; i < ev_size; ++i) {
 				DEBUG_STR_APPEND(a,hex);
 				DEBUG_STR_APPEND(a,"0x");
diff --git a/libs/evoral/evoral/Event.hpp b/libs/evoral/evoral/Event.hpp
index ddc4969db7..b94d9d4815 100644
--- a/libs/evoral/evoral/Event.hpp
+++ b/libs/evoral/evoral/Event.hpp
@@ -61,8 +61,8 @@ class LIBEVORAL_API Event /* INHERITANCE ILLEGAL HERE */
 {
 public:
 	Event (EventType ty, Time tm, size_t sz, uint8_t const * data, event_id_t id = -1)
-		: _type (ty)
-		, _time (tm)
+		: _time (tm)
+		, _type (ty)
 		, _size (sz)
 		, _id (id)
 	{
@@ -76,8 +76,8 @@ public:
 	}
 
 	Event (Event const & other)
-		: _type (other.event_type())
-		, _time (other.time())
+		: _time (other.time())
+		, _type (other.event_type())
 		, _size (other.size())
 		, _id (next_event_id())
 	{
@@ -244,12 +244,13 @@ public:
 		return other_first;
 	}
 
+	static uint32_t memory_size (uint32_t size) { return sizeof (Event<Time>) + size; }
 
 	inline EventType      event_type()    const { return _type; }
 	inline Time           time()          const { return _time; }
 	inline uint32_t       size()          const { return _size; }
 	inline void           set_size (uint32_t s) { _size = s; /* CAREFUL !!! */ }
-	inline uint32_t       object_size()   const { return size() + sizeof (*this); }
+	inline uint32_t       object_size()   const { return memory_size (size()); }
 	inline const uint8_t* buffer()        const { return _buf; }
 	inline uint8_t*       buffer()              { return _buf; }
 
@@ -323,8 +324,8 @@ public:
 
 
   public:
-	EventType  _type;   /*< Type of event (application relative, NOT MIDI 'type') */ \
 	Time       _time;   /*< Time stamp of event */ \
+	EventType  _type;   /*< Type of event (application relative, NOT MIDI 'type') */ \
 	uint32_t   _size;   /*< Size of buffer in bytes */ \
 	event_id_t _id;     /*< Unique event ID */ \
 	uint8_t    _buf[0]; /*< Event data. Must be at end, to use C-style variable-sized structure hack */
@@ -400,17 +401,18 @@ class LIBEVORAL_API ManagedEvent /* INHERITANCE ILLEGAL HERE */
 	~ManagedEvent ();
 
 	int refcnt() const { return _refcnt.load(); }
-	EventPool* pool() const { return _pool; }
+	EventPool& pool() const { return _pool; }
 
 	void operator delete (void* ptr) {
-		ManagedEvent* ev = reinterpret_cast<ManagedEvent*> (ptr);
-		if (ev && ev->_pool) {
-			ev->_pool->release (ptr);
+		ManagedEvent* mev = reinterpret_cast<ManagedEvent*> (ptr);
+		if (mev) {
+			mev->_pool.release (ptr);
 		}
 	}
 
 	ManagedEvent& operator= (ManagedEvent<Time> const & other) {
 		if (this != &other) {
+			/* DOES NOT COPY POOL */
 			_event = other._event;
 		}
 		return *this;
@@ -489,7 +491,7 @@ class LIBEVORAL_API ManagedEvent /* INHERITANCE ILLEGAL HERE */
 		}
 	}
   private:
-	EventPool*  _pool;
+	EventPool&  _pool;
 	Event<Time> _event;
 	/* C++ standard: "Nonstatic data members of a (non-union) class with
 	 * the same access control (Clause 11) are allocated so that later
@@ -499,25 +501,13 @@ class LIBEVORAL_API ManagedEvent /* INHERITANCE ILLEGAL HERE */
 	 */
 
 	ManagedEvent (EventPool& p, EventType ty, Time tm, size_t sz, uint8_t* data, event_id_t id = -1)
-		: _pool (&p)
+		: _pool (p)
 		, _event (ty, tm, sz, data, id)
 	{
 	}
 
 	ManagedEvent (EventPool& p, Event<Time> const & other)
-		: _pool (&p)
-		, _event (other)
-	{
-	}
-
-	ManagedEvent (EventType ty, Time tm, size_t sz, uint8_t* data, event_id_t id = -1)
-		: _pool (0)
-		, _event (ty, tm, sz, data, id)
-	{
-	}
-
-	ManagedEvent (Event<Time> const & other)
-		: _pool (0)
+		: _pool (p)
 		, _event (other)
 	{
 	}
diff --git a/libs/evoral/evoral/Note.hpp b/libs/evoral/evoral/Note.hpp
index 725968866f..20931ab47b 100644
--- a/libs/evoral/evoral/Note.hpp
+++ b/libs/evoral/evoral/Note.hpp
@@ -44,7 +44,7 @@ public:
 	using MultiEvent<Time,2>::set_event;
 
 	Note (EventPointer<Time> const & on_event, EventPointer<Time> const & off_event);
-	Note (uint8_t chan, Time time, Time length, uint8_t note, uint8_t velocity = 64);
+	Note (EventPool&, uint8_t chan, Time time, Time length, uint8_t note, uint8_t velocity = 64);
 	Note (Note<Time> const & other);
 
 	~Note();
@@ -77,6 +77,8 @@ public:
 			(*_events[1] == *other.off_event());
 	}
 
+	EventPool& pool() const { return _events[0]->pool(); }
+
 private:
 	inline int clamp(int val, int low, int high) {
 		return std::min (std::max (val, low), high);
@@ -129,7 +131,10 @@ template<typename Time>
 	o << "Note #" << n.id() << ": pitch = " << (int) n.note()
 	  << " @ " << n.time() << " .. " << n.end_time()
 	  << " velocity " << (int) n.velocity()
-	  << " chn " << (int) n.channel();
+	  << " chn " << (int) n.channel()
+	  << " on @ " << n.on_event()
+	  << " off @ " << n.off_event()
+		;
 	return o;
 }
 
@@ -140,8 +145,8 @@ class NotePointer : public MultiEventPointer<Note<Time> >
 	NotePointer () {}
 	NotePointer (Note<Time>* n) : MultiEventPointer<Note<Time> > (n) {}
 	NotePointer (NotePointer const & other);
-	NotePointer (uint8_t chan, Time time, Time length, uint8_t note, uint8_t velocity)
-		: MultiEventPointer<Note<Time> > (new Note<Time> (chan, time, length, note, velocity))
+	NotePointer (EventPool& pool, uint8_t chan, Time time, Time length, uint8_t note, uint8_t velocity)
+		: MultiEventPointer<Note<Time> > (new Note<Time> (pool, chan, time, length, note, velocity))
 	{
 	}
 
diff --git a/libs/evoral/src/Note.cpp b/libs/evoral/src/Note.cpp
index 25661f5508..74753a0829 100644
--- a/libs/evoral/src/Note.cpp
+++ b/libs/evoral/src/Note.cpp
@@ -33,12 +33,15 @@ namespace Evoral {
 template<typename Time>
 Note<Time>::Note (EventPointer<Time> const & on, EventPointer<Time> const & off)
 {
+	/* Note "owns" the two events pointed to by @param on and @param off,
+	   but they are refcnt'ed anyway.
+	*/
 	set_event (0, on);
 	set_event (1, off);
 }
 
 template<typename Time>
-Note<Time>::Note (uint8_t chan, Time time, Time length, uint8_t note, uint8_t velocity)
+Note<Time>::Note (EventPool& pool, uint8_t chan, Time time, Time length, uint8_t note, uint8_t velocity)
 {
 	/* this uses the ManagedEvent::default_event_pool to create the Events.
 	 * This is suboptimal if the events/notes are going to end up in a
@@ -53,26 +56,26 @@ Note<Time>::Note (uint8_t chan, Time time, Time length, uint8_t note, uint8_t ve
 
 	std::cerr << "NEW NOTE\n";
 
-	set_event (0, EventPointer<Time>::create (Evoral::MIDI_EVENT, time, 3, data));
+	set_event (0, EventPointer<Time>::create (pool, Evoral::MIDI_EVENT, time, 3, data));
 
 	data[0] = (MIDI_CMD_NOTE_OFF|chan);
 	data[1] = note;
 	data[2] = velocity;
 
-	set_event (1, EventPointer<Time>::create (Evoral::MIDI_EVENT, time + length, 3, data));
+	set_event (1, EventPointer<Time>::create (pool, Evoral::MIDI_EVENT, time + length, 3, data));
 	std::cerr << "NEW NOTE DONE\n";
 }
 
 template<typename Time>
 Note<Time>::Note (Note<Time> const & other)
 {
-	EventPointer<Time> on (other.on_event());
-	EventPointer<Time> off (other.off_event());
+	EventPointer<Time> const & on (other.on_event());
+	EventPointer<Time> const & off (other.off_event());
 
 	std::cerr << "NOTE COPY\n";
 
-	set_event (0, EventPointer<Time>::create (Evoral::MIDI_EVENT, on->time(), on->size(), on->buffer()));
-	set_event (1, EventPointer<Time>::create (Evoral::MIDI_EVENT, off->time(), off->size(), off->buffer()));
+	set_event (0, EventPointer<Time>::create (other.pool(), Evoral::MIDI_EVENT, on->time(), on->size(), on->buffer()));
+	set_event (1, EventPointer<Time>::create (other.pool(), Evoral::MIDI_EVENT, off->time(), off->size(), off->buffer()));
 	std::cerr << "NOTE COPY DONE\n";
 }
 
@@ -103,11 +106,6 @@ template<typename Time>
 NotePointer<Time>::NotePointer (NotePointer<Time> const & other)
 	: MultiEventPointer<Note<Time> > (other.get())
 {
-	std::cerr << "copy-constructed note pointer @ " << this << " points to " << this->get()
-	          << " UC now " << this->get()->use_count()
-	          << " linked ? " << this->is_linked ()
-	          << std::endl;
-	PBD::stacktrace (std::cerr, 20);
 }
 
 template class Note<Evoral::Beats>;
diff --git a/libs/evoral/src/Sequence.cpp b/libs/evoral/src/Sequence.cpp
index 9aeb7309eb..053afff5e1 100644
--- a/libs/evoral/src/Sequence.cpp
+++ b/libs/evoral/src/Sequence.cpp
@@ -306,13 +306,22 @@ Sequence<Time>::add_note_unlocked (NotePtr & note, void* arg)
 		note->set_id (Evoral::next_event_id());
 	}
 
-	if (note->note() < _lowest_note)
+	if (note->note() < _lowest_note) {
 		_lowest_note = note->note();
-	if (note->note() > _highest_note)
+	}
+	if (note->note() > _highest_note) {
 		_highest_note = note->note();
+	}
 
+	ordered_insert (_events, note->on_event (), TimeComparator<EventPtr,Time>());
+	ordered_insert (_events, note->off_event (), TimeComparator<EventPtr,Time>());
 	ordered_insert (_notes, note, TimeComparator<NotePtr,Time>());
-	ordered_insert (_pitches[note->channel()], note, LowerNoteValueComparator<NotePtr>());
+
+	/* already inserted @param note into one intrusive list (_notes); need to copy
+	   so that we can do add it to a second list (_pitches).
+	*/
+
+	ordered_insert (_pitches[note->channel()], *(new NotePtr (note)), LowerNoteValueComparator<NotePtr>());
 
 	_edited = true;
 
@@ -733,7 +742,7 @@ Sequence<Time>::set_notes (typename Sequence<Time>::Notes const & n)
 		event_id_t id = next_event_id ();
 
 		EventPtr on (EventPtr::create (*_event_pool, MIDI_EVENT, (*i)->on_event()->time(), (*i)->on_event()->size(), (*i)->on_event()->buffer(), id));
-		EventPtr off (EventPtr::create (*_event_pool, MIDI_EVENT, (*i)->on_event()->time(), (*i)->off_event()->size(), (*i)->off_event()->buffer(), id));
+		EventPtr off (EventPtr::create (*_event_pool, MIDI_EVENT, (*i)->off_event()->time(), (*i)->off_event()->size(), (*i)->off_event()->buffer(), id));
 
 		NotePtr& np (*(new NotePtr (new Note<Time> (on, off))));