Fix event type and parameter type confusion

I'm not sure if this is really the best way to do event types (should it
just be a completely static enum in evoral, or completely dynamic and
provided by the type map, or a mix like currently?), but previously the
event type was frequently set to either total garbage, or parameter
types, which are a different thing.

This fixes all those cases, and makes Evoral::EventType an enum so the
compiler will warn about implicit conversions from int.

10 files changed, 66 insertions, 47 deletions

diff --git a/libs/evoral/evoral/Event.hpp b/libs/evoral/evoral/Event.hpp
index 526513b3fe..696fb995ab 100644
--- a/libs/evoral/evoral/Event.hpp
+++ b/libs/evoral/evoral/Event.hpp
@@ -48,7 +48,7 @@ template<typename Time>
 class LIBEVORAL_API Event {
 public:
 #ifdef EVORAL_EVENT_ALLOC
-	Event(EventType type=0, Time time=Time(), uint32_t size=0, uint8_t* buf=NULL, bool alloc=false);
+	Event(EventType type=NO_EVENT, Time time=Time(), uint32_t size=0, uint8_t* buf=NULL, bool alloc=false);
 
 	Event(EventType type, Time time, uint32_t size, const uint8_t* buf);
 
@@ -108,7 +108,7 @@ public:
 	}
 
 	inline void clear() {
-		_type          = 0;
+		_type          = NO_EVENT;
 		_original_time = Time();
 		_nominal_time  = Time();
 		_size          = 0;
diff --git a/libs/evoral/evoral/Parameter.hpp b/libs/evoral/evoral/Parameter.hpp
index 1412699b4d..2614a03d86 100644
--- a/libs/evoral/evoral/Parameter.hpp
+++ b/libs/evoral/evoral/Parameter.hpp
@@ -25,6 +25,7 @@
 #include <boost/shared_ptr.hpp>
 
 #include "evoral/visibility.h"
+#include "evoral/types.hpp"
 
 namespace Evoral {
 
@@ -40,13 +41,13 @@ namespace Evoral {
 class LIBEVORAL_API Parameter
 {
 public:
-	inline Parameter(uint32_t type, uint8_t channel=0, uint32_t id=0)
+	inline Parameter(ParameterType type, uint8_t channel=0, uint32_t id=0)
 		: _type(type), _id(id), _channel(channel)
 	{}
 
-	inline uint32_t type()    const { return _type; }
-	inline uint8_t  channel() const { return _channel; }
-	inline uint32_t id()      const { return _id; }
+	inline ParameterType type()    const { return _type; }
+	inline uint8_t       channel() const { return _channel; }
+	inline uint32_t      id()      const { return _id; }
 
 	/** Equivalence operator
 	 * It is obvious from the definition that this operator
@@ -80,9 +81,9 @@ public:
 	inline operator bool() const { return (_type != 0); }
 
 private:
-	uint32_t _type;
-	uint32_t _id;
-	uint8_t  _channel;
+	ParameterType _type;
+	uint32_t      _id;
+	uint8_t       _channel;
 };
 
 } // namespace Evoral
diff --git a/libs/evoral/evoral/PatchChange.hpp b/libs/evoral/evoral/PatchChange.hpp
index a775a72cc3..90d3dba732 100644
--- a/libs/evoral/evoral/PatchChange.hpp
+++ b/libs/evoral/evoral/PatchChange.hpp
@@ -39,9 +39,9 @@ public:
 	 *  @param b Bank number (counted from 0, 14-bit).
 	 */
 	PatchChange (Time t, uint8_t c, uint8_t p, int b)
-		: _bank_change_msb (0, t, 3, 0, true)
-		, _bank_change_lsb (0, t, 3, 0, true)
-		, _program_change (0, t, 2, 0, true)
+		: _bank_change_msb (MIDI_EVENT, t, 3, 0, true)
+		, _bank_change_lsb (MIDI_EVENT, t, 3, 0, true)
+		, _program_change (MIDI_EVENT, t, 2, 0, true)
 	{
 		_bank_change_msb.buffer()[0] = MIDI_CMD_CONTROL | c;
 		_bank_change_msb.buffer()[1] = MIDI_CTL_MSB_BANK;
diff --git a/libs/evoral/evoral/TypeMap.hpp b/libs/evoral/evoral/TypeMap.hpp
index d09439daa6..0ebc81370b 100644
--- a/libs/evoral/evoral/TypeMap.hpp
+++ b/libs/evoral/evoral/TypeMap.hpp
@@ -24,6 +24,7 @@
 #include <string>
 
 #include "evoral/visibility.h"
+#include "evoral/types.hpp"
 
 namespace Evoral {
 
@@ -47,9 +48,8 @@ public:
 	 */
 	virtual uint8_t parameter_midi_type(const Parameter& param) const = 0;
 
-	/** The type ID for a MIDI event with the given status byte
-	 */
-	virtual uint32_t midi_event_type(uint8_t status) const = 0;
+	/** The parameter type for the given MIDI event. */
+	virtual ParameterType midi_parameter_type(const uint8_t* buf, uint32_t len) const = 0;
 
 	/** Return the description of a parameter. */
 	virtual ParameterDescriptor descriptor(const Parameter& param) const = 0;
diff --git a/libs/evoral/evoral/types.hpp b/libs/evoral/evoral/types.hpp
index a5d4a8ca1e..72b95f8460 100644
--- a/libs/evoral/evoral/types.hpp
+++ b/libs/evoral/evoral/types.hpp
@@ -1,5 +1,5 @@
 /* This file is part of Evoral.
- * Copyright (C) 2008 David Robillard <http://drobilla.net>
+ * Copyright (C) 2008-2016 David Robillard <http://drobilla.net>
  * Copyright (C) 2000-2008 Paul Davis
  *
  * Evoral is free software; you can redistribute it and/or modify it under the
@@ -37,8 +37,23 @@ namespace Evoral {
 */
 typedef int32_t event_id_t;
 
-/** Type of an event (opaque, mapped by application) */
-typedef uint32_t EventType;
+/** Type of an event (opaque, mapped by application, e.g. MIDI).
+ *
+ * Event types are really an arbitrary integer provided by the type map, and it
+ * is safe to use values not in this enum, but this enum exists so the compiler
+ * can catch mistakes like setting the event type to a MIDI status byte.  Event
+ * types come from the type map and describe a format/protocol like MIDI, and
+ * must not be confused with the payload (such as a note on or CC change).
+ * There is a static value for MIDI as this type is handled specially by
+ * various parts of Evoral.
+ */
+enum EventType {
+	NO_EVENT,
+	MIDI_EVENT
+};
+
+/** Type of a parameter (opaque, mapped by application, e.g. gain) */
+typedef uint32_t ParameterType;
 
 class Beats;
 
diff --git a/libs/evoral/src/Note.cpp b/libs/evoral/src/Note.cpp
index 3b56a33241..52a4d6335b 100644
--- a/libs/evoral/src/Note.cpp
+++ b/libs/evoral/src/Note.cpp
@@ -30,9 +30,8 @@ namespace Evoral {
 
 template<typename Time>
 Note<Time>::Note(uint8_t chan, Time t, Time l, uint8_t n, uint8_t v)
-	// FIXME: types?
-	: _on_event (0xDE, t, 3, NULL, true)
-	, _off_event (0xAD, t + l, 3, NULL, true)
+	: _on_event (MIDI_EVENT, t, 3, NULL, true)
+	, _off_event (MIDI_EVENT, t + l, 3, NULL, true)
 {
 	assert(chan < 16);
 
diff --git a/libs/evoral/src/Sequence.cpp b/libs/evoral/src/Sequence.cpp
index 3098d0339d..14e3b65f37 100644
--- a/libs/evoral/src/Sequence.cpp
+++ b/libs/evoral/src/Sequence.cpp
@@ -63,7 +63,7 @@ template<typename Time>
 Sequence<Time>::const_iterator::const_iterator()
 	: _seq(NULL)
 	, _event(boost::shared_ptr< Event<Time> >(new Event<Time>()))
-	, _active_patch_change_message (0)
+	, _active_patch_change_message (NO_EVENT)
 	, _type(NIL)
 	, _is_end(true)
 	, _control_iter(_control_iters.end())
@@ -191,7 +191,7 @@ Sequence<Time>::const_iterator::const_iterator(const Sequence<Time>&
 
 	// Allocate a new event for storing the current event in MIDI format
 	_event = boost::shared_ptr< Event<Time> >(
-		new Event<Time>(0, Time(), 4, NULL, true));
+		new Event<Time>(NO_EVENT, Time(), 4, NULL, true));
 
 	// Set event from chosen sub-iterator
 	set_event();
@@ -515,9 +515,8 @@ Sequence<Time>::Sequence(const Sequence<Time>& other)
 	assert(! _end_iter._lock);
 }
 
-/** Write the controller event pointed to by \a iter to \a ev.
- * The buffer of \a ev will be allocated or resized as necessary.
- * The event_type of \a ev should be set to the expected output type.
+/** Write the controller event pointed to by `iter` to `ev`.
+ * The buffer of `ev` will be allocated or resized as necessary.
  * \return true on success
  */
 template<typename Time>
@@ -527,15 +526,14 @@ Sequence<Time>::control_to_midi_event(
 	const ControlIterator&            iter) const
 {
 	assert(iter.list.get());
-	const uint32_t event_type = iter.list->parameter().type();
 
 	// initialize the event pointer with a new event, if necessary
 	if (!ev) {
-		ev = boost::shared_ptr< Event<Time> >(new Event<Time>(event_type, Time(), 3, NULL, true));
+		ev = boost::shared_ptr< Event<Time> >(new Event<Time>(NO_EVENT, Time(), 3, NULL, true));
 	}
 
-	uint8_t midi_type = _type_map.parameter_midi_type(iter.list->parameter());
-	ev->set_event_type(_type_map.midi_event_type(midi_type));
+	const uint8_t midi_type = _type_map.parameter_midi_type(iter.list->parameter());
+	ev->set_event_type(MIDI_EVENT);
 	ev->set_id(-1);
 	switch (midi_type) {
 	case MIDI_CMD_CONTROL:
@@ -933,23 +931,28 @@ Sequence<Time>::append(const Event<Time>& ev, event_id_t evid)
 			_bank[ev.channel()] |= ev.cc_value();
 		}
 	} else if (ev.is_cc()) {
+		const ParameterType ptype = _type_map.midi_parameter_type(ev.buffer(), ev.size());
 		append_control_unlocked(
-			Parameter(ev.event_type(), ev.channel(), ev.cc_number()),
+			Parameter(ptype, ev.channel(), ev.cc_number()),
 			ev.time(), ev.cc_value(), evid);
 	} else if (ev.is_pgm_change()) {
 		/* write a patch change with this program change and any previously set-up bank number */
-		append_patch_change_unlocked (PatchChange<Time> (ev.time(), ev.channel(), ev.pgm_number(), _bank[ev.channel()]), evid);
+		append_patch_change_unlocked (
+			PatchChange<Time> (ev.time(), ev.channel(),
+			                   ev.pgm_number(), _bank[ev.channel()]), evid);
 	} else if (ev.is_pitch_bender()) {
+		const ParameterType ptype = _type_map.midi_parameter_type(ev.buffer(), ev.size());
 		append_control_unlocked(
-			Parameter(ev.event_type(), ev.channel()),
+			Parameter(ptype, ev.channel()),
 			ev.time(), double ((0x7F & ev.pitch_bender_msb()) << 7
 			                   | (0x7F & ev.pitch_bender_lsb())),
 			evid);
 	} else if (ev.is_poly_pressure()) {
 		append_control_unlocked (Parameter (ev.event_type(), ev.channel(), ev.poly_note()), ev.time(), ev.poly_pressure(), evid);
 	} else if (ev.is_channel_pressure()) {
+		const ParameterType ptype = _type_map.midi_parameter_type(ev.buffer(), ev.size());
 		append_control_unlocked(
-			Parameter(ev.event_type(), ev.channel()),
+			Parameter(ptype, ev.channel()),
 			ev.time(), ev.channel_pressure(), evid);
 	} else if (!_type_map.type_is_midi(ev.event_type())) {
 		printf("WARNING: Sequence: Unknown event type %X: ", ev.event_type());
@@ -1215,7 +1218,7 @@ template<typename Time>
 typename Sequence<Time>::SysExes::const_iterator
 Sequence<Time>::sysex_lower_bound (Time t) const
 {
-	SysExPtr search (new Event<Time> (0, t));
+	SysExPtr search (new Event<Time> (NO_EVENT, t));
 	typename Sequence<Time>::SysExes::const_iterator i = _sysexes.lower_bound (search);
 	assert (i == _sysexes.end() || (*i)->time() >= t);
 	return i;
@@ -1250,7 +1253,7 @@ template<typename Time>
 typename Sequence<Time>::SysExes::iterator
 Sequence<Time>::sysex_lower_bound (Time t)
 {
-	SysExPtr search (new Event<Time> (0, t));
+	SysExPtr search (new Event<Time> (NO_EVENT, t));
 	typename Sequence<Time>::SysExes::iterator i = _sysexes.lower_bound (search);
 	assert (i == _sysexes.end() || (*i)->time() >= t);
 	return i;
diff --git a/libs/evoral/test/SMFTest.cpp b/libs/evoral/test/SMFTest.cpp
index 0907cdc2f5..108052c98f 100644
--- a/libs/evoral/test/SMFTest.cpp
+++ b/libs/evoral/test/SMFTest.cpp
@@ -59,7 +59,7 @@ SMFTest::takeFiveTest ()
 		if (ret > 0) { // didn't skip (meta) event
 			//cerr << "read smf event type " << hex << int(buf[0]) << endl;
 			ev.set_time(Evoral::Beats::ticks_at_rate(time, smf.ppqn()));
-			ev.set_event_type(type_map->midi_event_type(buf[0]));
+			ev.set_event_type(Evoral::MIDI_EVENT);
 			seq->append(ev, next_event_id ());
 		}
 	}
diff --git a/libs/evoral/test/SequenceTest.cpp b/libs/evoral/test/SequenceTest.cpp
index a3dfabe139..16e93b37d2 100644
--- a/libs/evoral/test/SequenceTest.cpp
+++ b/libs/evoral/test/SequenceTest.cpp
@@ -25,7 +25,7 @@ SequenceTest::preserveEventOrderingTest ()
 	for (Notes::const_iterator i = test_notes.begin(); i != test_notes.end(); ++i) {
 		uint8_t buffer[3];
 		Event<Time>* event = new Event<Time>(
-				DummyTypeMap::CONTROL, (*i)->on_event().time(), 3, buffer, true
+			(Evoral::EventType)DummyTypeMap::CONTROL, (*i)->on_event().time(), 3, buffer, true
 		);
 
 		event->buffer()[0] = MIDI_CMD_CONTROL;
@@ -77,12 +77,12 @@ SequenceTest::iteratorSeekTest ()
 	bool on = true;
 	for (Sequence<Time>::const_iterator i = seq->begin(Evoral::Beats(600)); i != seq->end(); ++i) {
 		if (on) {
-			CPPUNIT_ASSERT((*i)->is_note_on());
+			CPPUNIT_ASSERT(i->is_note_on());
 			CPPUNIT_ASSERT_EQUAL(i->time(), Time((num_notes + 6) * 100));
 			++num_notes;
 			on = false;
 		} else {
-			CPPUNIT_ASSERT((*i)->is_note_off());
+			CPPUNIT_ASSERT(i->is_note_off());
 			on = true;
 		}
 	}
diff --git a/libs/evoral/test/SequenceTest.hpp b/libs/evoral/test/SequenceTest.hpp
index de51f714a1..48dff308b6 100644
--- a/libs/evoral/test/SequenceTest.hpp
+++ b/libs/evoral/test/SequenceTest.hpp
@@ -31,13 +31,14 @@ public:
 		};
 	}
 
-	uint32_t midi_event_type(uint8_t status) const {
-		status &= 0xf0;
-		switch (status) {
-		case MIDI_CMD_CONTROL:          return CONTROL;
-		case MIDI_CMD_COMMON_SYSEX:     return SYSEX;
-		default:                        return 0;
-		};
+	virtual ParameterType midi_parameter_type(const uint8_t* buf, uint32_t len) const {
+		switch (buf[0] & 0xF0) {
+		case MIDI_CMD_CONTROL:      return CONTROL;
+		case MIDI_CMD_COMMON_SYSEX: return SYSEX;
+		case MIDI_CMD_NOTE_ON:      return NOTE;
+		case MIDI_CMD_NOTE_OFF:     return NOTE;
+		default: return 0;
+		}
 	}
 
 	ParameterDescriptor descriptor(const Parameter& param) const {