From 8f8a4878f30795b7ac5398c8daa6b01b5c6d4468 Mon Sep 17 00:00:00 2001
From: David Robillard <d@drobilla.net>
Date: Wed, 19 Nov 2014 20:47:18 -0500
Subject: [PATCH] Clean up some MIDI code.

---
 libs/ardour/ardour/midi_playlist.h      |  7 ++-
 libs/ardour/ardour/midi_port.h          |  1 -
 libs/ardour/ardour/midi_source.h        | 33 +++++------
 libs/ardour/ardour/midi_state_tracker.h | 19 +++++-
 libs/ardour/midi_source.cc              | 79 +++++++++----------------
 libs/ardour/midi_state_tracker.cc       | 25 +-------
 6 files changed, 68 insertions(+), 96 deletions(-)

diff --git a/libs/ardour/ardour/midi_playlist.h b/libs/ardour/ardour/midi_playlist.h
index b118214c9e..2603de45f7 100644
--- a/libs/ardour/ardour/midi_playlist.h
+++ b/libs/ardour/ardour/midi_playlist.h
@@ -25,15 +25,20 @@
 
 #include "ardour/ardour.h"
 #include "ardour/playlist.h"
-#include "ardour/midi_state_tracker.h"
 #include "evoral/Parameter.hpp"
 
+namespace Evoral {
+template<typename Time> class EventSink;
+}
+
 namespace ARDOUR
 {
 
 class Session;
 class MidiRegion;
 class Source;
+class MidiStateTracker;
+
 template<typename T> class MidiRingBuffer;
 
 class LIBARDOUR_API MidiPlaylist : public ARDOUR::Playlist
diff --git a/libs/ardour/ardour/midi_port.h b/libs/ardour/ardour/midi_port.h
index cc9fee1b15..debe29877a 100644
--- a/libs/ardour/ardour/midi_port.h
+++ b/libs/ardour/ardour/midi_port.h
@@ -25,7 +25,6 @@
 
 #include "ardour/port.h"
 #include "ardour/midi_buffer.h"
-#include "ardour/midi_state_tracker.h"
 
 namespace ARDOUR {
 
diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h
index 411c76eeea..364b220ebb 100644
--- a/libs/ardour/ardour/midi_source.h
+++ b/libs/ardour/ardour/midi_source.h
@@ -53,32 +53,29 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	 * \param newsrc MidiSource to which data will be written. Should be a
 	 *        new, empty source. If it already has contents, the results are
 	 *        undefined. Source must be writable.
-	 *
 	 * \param begin time of earliest event that can be written.
 	 * \param end time of latest event that can be written.
-	 *
-	 * Returns zero on success, non-zero if the write failed for any
-	 * reason.
-	 *
+	 * \return zero on success, non-zero if the write failed for any reason.
 	 */
 	int write_to (boost::shared_ptr<MidiSource> newsrc,
-		      Evoral::MusicalTime begin = Evoral::MinMusicalTime,
-		      Evoral::MusicalTime end = Evoral::MaxMusicalTime);
+	              Evoral::MusicalTime           begin = Evoral::MinMusicalTime,
+	              Evoral::MusicalTime           end   = Evoral::MaxMusicalTime);
 
 	/** Read the data in a given time range from the MIDI source.
 	 * All time stamps in parameters are in audio frames (even if the source has tempo time).
-	 * \param dst Ring buffer where read events are written
-	 * \param source_start Start position of the SOURCE in this read context
-	 * \param start Start of range to be read
-	 * \param cnt Length of range to be read (in audio frames)
-	 * \param tracker an optional pointer to MidiStateTracker object, for note on/off tracking
+	 * \param dst Ring buffer where read events are written.
+	 * \param source_start Start position of the SOURCE in this read context.
+	 * \param start Start of range to be read.
+	 * \param cnt Length of range to be read (in audio frames).
+	 * \param tracker an optional pointer to MidiStateTracker object, for note on/off tracking.
+	 * \param filtered Parameters whose MIDI messages will not be returned.
 	 */
-	virtual framecnt_t midi_read (Evoral::EventSink<framepos_t>& dst,
-	                              framepos_t                     source_start,
-	                              framepos_t                     start,
-	                              framecnt_t                     cnt,
-	                              MidiStateTracker*              tracker,
-	                              std::set<Evoral::Parameter> const &) const;
+	virtual framecnt_t midi_read (Evoral::EventSink<framepos_t>&     dst,
+	                              framepos_t                         source_start,
+	                              framepos_t                         start,
+	                              framecnt_t                         cnt,
+	                              MidiStateTracker*                  tracker,
+	                              const std::set<Evoral::Parameter>& filtered) const;
 
 	/** Write data from a MidiRingBuffer to this source.
 	 *  @param source Source to read from.
diff --git a/libs/ardour/ardour/midi_state_tracker.h b/libs/ardour/ardour/midi_state_tracker.h
index 046e77f38c..a77ffdada3 100644
--- a/libs/ardour/ardour/midi_state_tracker.h
+++ b/libs/ardour/ardour/midi_state_tracker.h
@@ -52,9 +52,24 @@ public:
 		return _active_notes[(channel*128)+note] > 0;
 	}
 
-private:
-	void track_note_onoffs(const Evoral::MIDIEvent<MidiBuffer::TimeType>& event);
+	template<typename Time>
+	void track (const Evoral::Event<Time>& ev) {
+		const uint8_t type = ev.buffer()[0] & 0xF0;
+		const uint8_t chan = ev.buffer()[0] & 0x0F;
+		switch (type) {
+		case MIDI_CTL_ALL_NOTES_OFF:
+			reset();
+			break;
+		case MIDI_CMD_NOTE_ON:
+			add(ev.buffer()[1], chan);
+			break;
+		case MIDI_CMD_NOTE_OFF:
+			remove(ev.buffer()[1], chan);
+			break;
+		}
+	}
 
+private:
 	uint8_t  _active_notes[128*16];
 	uint16_t _on;
 };
diff --git a/libs/ardour/midi_source.cc b/libs/ardour/midi_source.cc
index 52c15f94cd..494bb513fe 100644
--- a/libs/ardour/midi_source.cc
+++ b/libs/ardour/midi_source.cc
@@ -74,7 +74,6 @@ MidiSource::MidiSource (Session& s, const XMLNode& node)
 	}
 }
 
-
 MidiSource::~MidiSource ()
 {
 }
@@ -107,7 +106,6 @@ int
 MidiSource::set_state (const XMLNode& node, int /*version*/)
 {
 	const XMLProperty* prop;
-
 	if ((prop = node.property ("captured-for")) != 0) {
 		_captured_for = prop->value();
 	}
@@ -115,39 +113,31 @@ MidiSource::set_state (const XMLNode& node, int /*version*/)
 	XMLNodeList children = node.children ();
 	for (XMLNodeConstIterator i = children.begin(); i != children.end(); ++i) {
 		if ((*i)->name() == X_("InterpolationStyle")) {
-			XMLProperty* prop;
-
 			if ((prop = (*i)->property (X_("parameter"))) == 0) {
 				error << _("Missing parameter property on InterpolationStyle") << endmsg;
 				return -1;
 			}
-
 			Evoral::Parameter p = EventTypeMap::instance().new_parameter (prop->value());
 
 			if ((prop = (*i)->property (X_("style"))) == 0) {
 				error << _("Missing style property on InterpolationStyle") << endmsg;
 				return -1;
 			}
-
-			Evoral::ControlList::InterpolationStyle s = static_cast<Evoral::ControlList::InterpolationStyle> (string_2_enum (prop->value(), s));
+			Evoral::ControlList::InterpolationStyle s = static_cast<Evoral::ControlList::InterpolationStyle>(
+				string_2_enum (prop->value(), s));
 			set_interpolation_of (p, s);
 
 		} else if ((*i)->name() == X_("AutomationState")) {
-
-			XMLProperty* prop;
-
 			if ((prop = (*i)->property (X_("parameter"))) == 0) {
 				error << _("Missing parameter property on AutomationState") << endmsg;
 				return -1;
 			}
-
 			Evoral::Parameter p = EventTypeMap::instance().new_parameter (prop->value());
 
 			if ((prop = (*i)->property (X_("state"))) == 0) {
 				error << _("Missing state property on AutomationState") << endmsg;
 				return -1;
 			}
-
 			AutoState s = static_cast<AutoState> (string_2_enum (prop->value(), s));
 			set_automation_state_of (p, s);
 		}
@@ -179,19 +169,21 @@ MidiSource::update_length (framecnt_t)
 	// You're not the boss of me!
 }
 
-/** @param filtered A set of parameters whose MIDI messages will not be returned */
 framecnt_t
-MidiSource::midi_read (Evoral::EventSink<framepos_t>& dst, framepos_t source_start,
-                       framepos_t start, framecnt_t cnt,
-                       MidiStateTracker* tracker,
-                       std::set<Evoral::Parameter> const & filtered) const
+MidiSource::midi_read (Evoral::EventSink<framepos_t>&     dst,
+                       framepos_t                         source_start,
+                       framepos_t                         start,
+                       framecnt_t                         cnt,
+                       MidiStateTracker*                  tracker,
+                       const std::set<Evoral::Parameter>& filtered) const
 {
 	Glib::Threads::Mutex::Lock lm (_lock);
 
 	BeatsFramesConverter converter(_session.tempo_map(), source_start);
 
-	DEBUG_TRACE (DEBUG::MidiSourceIO, string_compose ("MidiSource::midi-read() %5 sstart %1 start %2 cnt %3 tracker %4\n",
-							  source_start, start, cnt, tracker, name()));
+	DEBUG_TRACE (DEBUG::MidiSourceIO,
+	             string_compose ("MidiSource::midi_read() %5 sstart %1 start %2 cnt %3 tracker %4\n",
+	                             source_start, start, cnt, tracker, name()));
 
 	if (_model) {
 		// Read events up to end
@@ -201,26 +193,20 @@ MidiSource::midi_read (Evoral::EventSink<framepos_t>& dst, framepos_t source_sta
 		     ++i) {
 			const framecnt_t time_frames = converter.to(i->time());
 			if (time_frames < start + cnt) {
-				/* convert event times to session frames by adding on the source start position in session frames */
+				// Offset by source start to convert event time to session time
 				dst.write (time_frames + source_start, i->event_type(), i->size(), i->buffer());
 
-                                DEBUG_TRACE (DEBUG::MidiSourceIO, string_compose ("%1: add event @ %2 type %3 size = %4\n",
-                                                                                  _name, time_frames + source_start, i->event_type(), i->size()));
+				DEBUG_TRACE (DEBUG::MidiSourceIO,
+				             string_compose ("%1: add event @ %2 type %3 size %4\n",
+				                             _name, time_frames + source_start, i->event_type(), i->size()));
 
 				if (tracker) {
-					Evoral::MIDIEvent<Evoral::MusicalTime>& ev (*(reinterpret_cast<Evoral::MIDIEvent<Evoral::MusicalTime>*> 
-										      (const_cast<Evoral::Event<Evoral::MusicalTime>*> (&(*i)))));
-					if (ev.is_note_on()) {
-						DEBUG_TRACE (DEBUG::MidiSourceIO, string_compose ("\t%1 track note on %2 @ %3 velocity %4\n", _name, (int) ev.note(), time_frames, (int) ev.velocity()));
-						tracker->add (ev.note(), ev.channel());
-					} else if (ev.is_note_off()) {
-						DEBUG_TRACE (DEBUG::MidiSourceIO, string_compose ("\t%1 track note off %2 @ %3\n", _name, (int) ev.note(), time_frames));
-						tracker->remove (ev.note(), ev.channel());
-					}
+					tracker->track (*i);
 				}
 			} else {
-                                DEBUG_TRACE (DEBUG::MidiSourceIO, string_compose ("%1: reached end with event @ %2 vs. %3\n",
-                                                                                  _name, time_frames, start+cnt));
+				DEBUG_TRACE (DEBUG::MidiSourceIO,
+				             string_compose ("%1: reached end with event @ %2 vs. %3\n",
+				                             _name, time_frames, start+cnt));
 				break;
 			}
 		}
@@ -289,7 +275,8 @@ MidiSource::mark_streaming_write_started ()
 }
 
 void
-MidiSource::mark_midi_streaming_write_completed (Evoral::Sequence<Evoral::MusicalTime>::StuckNoteOption option, Evoral::MusicalTime end)
+MidiSource::mark_midi_streaming_write_completed (Evoral::Sequence<Evoral::MusicalTime>::StuckNoteOption option,
+                                                 Evoral::MusicalTime                                    end)
 {
 	if (_model) {
 		_model->end_write (option, end);
@@ -307,7 +294,7 @@ MidiSource::mark_streaming_write_completed ()
 int
 MidiSource::write_to (boost::shared_ptr<MidiSource> newsrc, Evoral::MusicalTime begin, Evoral::MusicalTime end)
 {
-	newsrc->set_timeline_position(_timeline_position);
+	newsrc->set_timeline_position (_timeline_position);
 	newsrc->copy_interpolation_from (this);
 	newsrc->copy_automation_state_from (this);
 
@@ -331,7 +318,7 @@ MidiSource::write_to (boost::shared_ptr<MidiSource> newsrc, Evoral::MusicalTime
 	} else {
 		newsrc->set_model (_model);
 	}
-	
+
 	/* this file is not removable (but since it is MIDI, it is mutable) */
 
 	boost::dynamic_pointer_cast<FileSource> (newsrc)->prevent_deletion ();
@@ -347,25 +334,18 @@ MidiSource::session_saved()
 	*/
 
 	if (_model && _model->edited()) {
-		
-		// if the model is edited, write its contents into
-		// the current source file (overwiting previous contents.
-
-		/* temporarily drop our reference to the model so that
-		   as the model pushes its current state to us, we don't
-		   try to update it.
-		*/
+		/* The model is edited, write its contents into the current source
+		   file (overwiting previous contents). */
 
+		/* Temporarily drop our reference to the model so that as the model
+		   pushes its current state to us, we don't try to update it. */
 		boost::shared_ptr<MidiModel> mm = _model;
 		_model.reset ();
 
-		/* flush model contents to disk
-		 */
-
+		/* Flush model contents to disk. */
 		mm->sync_to_source ();
 
-		/* reacquire model */
-
+		/* Reacquire model. */
 		_model = mm;
 
 	} else {
@@ -395,7 +375,6 @@ MidiSource::set_model (boost::shared_ptr<MidiModel> m)
 	ModelChanged (); /* EMIT SIGNAL */
 }
 
-/** @return Interpolation style that should be used for control parameter \a p */
 Evoral::ControlList::InterpolationStyle
 MidiSource::interpolation_of (Evoral::Parameter p) const
 {
diff --git a/libs/ardour/midi_state_tracker.cc b/libs/ardour/midi_state_tracker.cc
index 466fc20b63..5e99c35294 100644
--- a/libs/ardour/midi_state_tracker.cc
+++ b/libs/ardour/midi_state_tracker.cc
@@ -46,16 +46,6 @@ MidiStateTracker::reset ()
 	_on = 0;
 }
 
-void
-MidiStateTracker::track_note_onoffs (const Evoral::MIDIEvent<MidiBuffer::TimeType>& event)
-{
-	if (event.is_note_on()) {
-		add (event.note(), event.channel());
-	} else if (event.is_note_off()){
-		remove (event.note(), event.channel());
-	}
-}
-
 void
 MidiStateTracker::add (uint8_t note, uint8_t chn)
 {
@@ -96,21 +86,8 @@ MidiStateTracker::remove (uint8_t note, uint8_t chn)
 void
 MidiStateTracker::track (const MidiBuffer::iterator &from, const MidiBuffer::iterator &to)
 {
-	// DEBUG_TRACE (PBD::DEBUG::MidiTrackers, string_compose ("%1 track notes\n", this));
-
 	for (MidiBuffer::iterator i = from; i != to; ++i) {
-		const Evoral::MIDIEvent<MidiBuffer::TimeType> ev(*i, false);
-
-		/* catch AllNotesOff message and turn off all notes
-		 */
-		
-		if (ev.type() == MIDI_CTL_ALL_NOTES_OFF) {
-			cerr << "State tracker sees ALL_NOTES_OFF, silenceing " << sizeof (_active_notes) << endl;
-			memset (_active_notes, 0, sizeof (_active_notes));
-			_on = 0;
-		} else {
-			track_note_onoffs (ev);
-		}
+		track(*i);
 	}
 }