From: David Robillard <d@drobilla.net>
Date: Thu, 19 Feb 2015 20:55:16 +0000 (-0500)
Subject: Don't allocate then discard notes on note off.
X-Git-Tag: 4.0-rc1~416
X-Git-Url: https://main.carlh.net/gitweb/?p=ardour.git;a=commitdiff_plain;h=6fb4bd578ef30fc65c06b98c32a3278d9c443696

Don't allocate then discard notes on note off.

Silly to make a junk Note just to pass to append_note_off_unlocked, which just
uses the fields that are on the MIDIEvent anyway then throws it away.

Also explicitly dispatch to append_note_off_unlocked in the caller for note ons
with velocity 0 rather than make append_note_on_unlocked deal with it.
---

diff --git a/libs/evoral/evoral/Sequence.hpp b/libs/evoral/evoral/Sequence.hpp
index 4e6420fbb1..639a85730c 100644
--- a/libs/evoral/evoral/Sequence.hpp
+++ b/libs/evoral/evoral/Sequence.hpp
@@ -331,11 +331,11 @@ private:
 	bool overlaps_unlocked (const NotePtr& ev, const NotePtr& ignore_this_note) const;
 	bool contains_unlocked (const NotePtr& ev) const;
 
-	void append_note_on_unlocked (NotePtr, Evoral::event_id_t);
-	void append_note_off_unlocked(NotePtr);
+	void append_note_on_unlocked(const MIDIEvent<Time>& event, Evoral::event_id_t);
+	void append_note_off_unlocked(const MIDIEvent<Time>& event);
 	void append_control_unlocked(const Parameter& param, Time time, double value, Evoral::event_id_t);
 	void append_sysex_unlocked(const MIDIEvent<Time>& ev, Evoral::event_id_t);
-	void append_patch_change_unlocked (const PatchChange<Time>&, Evoral::event_id_t);
+	void append_patch_change_unlocked(const PatchChange<Time>&, Evoral::event_id_t);
 
 	void get_notes_by_pitch (Notes&, NoteOperator, uint8_t val, int chan_mask = 0) const;
 	void get_notes_by_velocity (Notes&, NoteOperator, uint8_t val, int chan_mask = 0) const;
diff --git a/libs/evoral/src/Sequence.cpp b/libs/evoral/src/Sequence.cpp
index 1b5abe8ce9..1d52a12e4f 100644
--- a/libs/evoral/src/Sequence.cpp
+++ b/libs/evoral/src/Sequence.cpp
@@ -886,15 +886,13 @@ Sequence<Time>::append(const Event<Time>& event, event_id_t evid)
 		return;
 	}
 
-	if (ev.is_note_on()) {
-		NotePtr note(new Note<Time>(ev.channel(), ev.time(), Time(), ev.note(), ev.velocity()));
-		append_note_on_unlocked (note, evid);
-	} else if (ev.is_note_off()) {
-		NotePtr note(new Note<Time>(ev.channel(), ev.time(), Time(), ev.note(), ev.velocity()));
+	if (ev.is_note_on() && ev.velocity() > 0) {
+		append_note_on_unlocked (ev, evid);
+	} else if (ev.is_note_off() || (ev.is_note_on() && ev.velocity() == 0)) {
 		/* XXX note: event ID is discarded because we merge the on+off events into
 		   a single note object
 		*/
-		append_note_off_unlocked (note);
+		append_note_off_unlocked (ev);
 	} else if (ev.is_sysex()) {
 		append_sysex_unlocked(ev, evid);
 	} else if (ev.is_cc() && (ev.cc_number() == MIDI_CTL_MSB_BANK || ev.cc_number() == MIDI_CTL_LSB_BANK)) {
@@ -938,31 +936,28 @@ Sequence<Time>::append(const Event<Time>& event, event_id_t evid)
 
 template<typename Time>
 void
-Sequence<Time>::append_note_on_unlocked (NotePtr note, event_id_t evid)
+Sequence<Time>::append_note_on_unlocked (const MIDIEvent<Time>& ev, event_id_t evid)
 {
 	DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1 c=%2 note %3 on @ %4 v=%5\n", this,
-	                                              (int) note->channel(), (int) note->note(),
-	                                              note->time(), (int) note->velocity()));
+	                                              (int)ev.channel(), (int)ev.note(),
+	                                              ev.time(), (int)ev.velocity()));
 	assert(_writing);
 
-	if (note->note() > 127) {
-		error << string_compose (_("illegal note number (%1) used in Note on event - event will be ignored"), (int)  note->note()) << endmsg;
+	if (ev.note() > 127) {
+		error << string_compose (_("invalid note on number (%1) ignored"), (int) ev.note()) << endmsg;
 		return;
-	}
-	if (note->channel() >= 16) {
-		error << string_compose (_("illegal channel number (%1) used in Note on event - event will be ignored"), (int) note->channel()) << endmsg;
+	} else if (ev.channel() >= 16) {
+		error << string_compose (_("invalid note on channel (%1) ignored"), (int) ev.channel()) << endmsg;
 		return;
-	}
-
-	if (note->id() < 0) {
-		note->set_id (evid);
-	}
-
-	if (note->velocity() == 0) {
-		append_note_off_unlocked (note);
+	} else if (ev.velocity() == 0) {
+		// Note on with velocity 0 handled as note off by caller
+		error << string_compose (_("invalid note on velocity (%1) ignored"), (int) ev.velocity()) << endmsg;
 		return;
 	}
 
+	NotePtr note(new Note<Time>(ev.channel(), ev.time(), Time(), ev.note(), ev.velocity()));
+	note->set_id (evid);
+
 	add_note_unlocked (note);
 
 	DEBUG_TRACE (DEBUG::Sequence, string_compose ("Appending active note on %1 channel %2\n",
@@ -973,19 +968,18 @@ Sequence<Time>::append_note_on_unlocked (NotePtr note, event_id_t evid)
 
 template<typename Time>
 void
-Sequence<Time>::append_note_off_unlocked (NotePtr note)
+Sequence<Time>::append_note_off_unlocked (const MIDIEvent<Time>& ev)
 {
 	DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1 c=%2 note %3 OFF @ %4 v=%5\n",
-	                                              this, (int)note->channel(),
-	                                              (int)note->note(), note->time(), (int)note->velocity()));
+	                                              this, (int)ev.channel(),
+	                                              (int)ev.note(), ev.time(), (int)ev.velocity()));
 	assert(_writing);
 
-	if (note->note() > 127) {
-		error << string_compose (_("illegal note number (%1) used in Note off event - event will be ignored"), (int) note->note()) << endmsg;
+	if (ev.note() > 127) {
+		error << string_compose (_("invalid note off number (%1) ignored"), (int) ev.note()) << endmsg;
 		return;
-	}
-	if (note->channel() >= 16) {
-		error << string_compose (_("illegal channel number (%1) used in Note off event - event will be ignored"), (int) note->channel()) << endmsg;
+	} else if (ev.channel() >= 16) {
+		error << string_compose (_("invalid note off channel (%1) ignored"), (int) ev.channel()) << endmsg;
 		return;
 	}
 
@@ -1001,19 +995,19 @@ Sequence<Time>::append_note_off_unlocked (NotePtr note)
 
 	/* XXX use _overlap_pitch_resolution to determine FIFO/LIFO ... */
 
-	for (typename WriteNotes::iterator n = _write_notes[note->channel()].begin(); n != _write_notes[note->channel()].end(); ) {
+	for (typename WriteNotes::iterator n = _write_notes[ev.channel()].begin(); n != _write_notes[ev.channel()].end(); ) {
 
 		typename WriteNotes::iterator tmp = n;
 		++tmp;
 
 		NotePtr nn = *n;
-		if (note->note() == nn->note() && nn->channel() == note->channel()) {
-			assert(note->time() >= nn->time());
+		if (ev.note() == nn->note() && nn->channel() == ev.channel()) {
+			assert(ev.time() >= nn->time());
 
-			nn->set_length (note->time() - nn->time());
-			nn->set_off_velocity (note->velocity());
+			nn->set_length (ev.time() - nn->time());
+			nn->set_off_velocity (ev.velocity());
 
-			_write_notes[note->channel()].erase(n);
+			_write_notes[ev.channel()].erase(n);
 			DEBUG_TRACE (DEBUG::Sequence, string_compose ("resolved note @ %2 length: %1\n", nn->length(), nn->time()));
 			resolved = true;
 			break;
@@ -1023,8 +1017,8 @@ Sequence<Time>::append_note_off_unlocked (NotePtr note)
 	}
 
 	if (!resolved) {
-		cerr << this << " spurious note off chan " << (int)note->channel()
-		     << ", note " << (int)note->note() << " @ " << note->time() << endl;
+		cerr << this << " spurious note off chan " << (int)ev.channel()
+		     << ", note " << (int)ev.note() << " @ " << ev.time() << endl;
 	}
 }