major fixes for MIDI patch change and note undo/redo. Patch change handling was completely broken because of the use of absolute floating point comparisons for time comparison, and serialization/deserialization of patch change property changes was borked because of int/char conversions by stringstream. Note undo/redo would fail for note removal if a note had been moved and/or had its note number changed as the next operation after it was added, because time-based lookup would fail. Similar small changes made for sysex messages, which just needed the musical_time comparisons and nothing else

4 files changed, 43 insertions, 10 deletions

diff --git a/libs/evoral/evoral/ControlSet.hpp b/libs/evoral/evoral/ControlSet.hpp
index 431885112b..716d199fec 100644
--- a/libs/evoral/evoral/ControlSet.hpp
+++ b/libs/evoral/evoral/ControlSet.hpp
@@ -65,7 +65,7 @@ public:
 
 	void what_has_data(std::set<Parameter>&) const;
 
-	Glib::Threads::Mutex& control_lock() const { return _control_lock; }
+        Glib::Threads::Mutex& control_lock() const { return _control_lock; }
 
 protected:
 	virtual void control_list_marked_dirty () {}
diff --git a/libs/evoral/evoral/PatchChange.hpp b/libs/evoral/evoral/PatchChange.hpp
index 82e2023941..b1a42c6f2e 100644
--- a/libs/evoral/evoral/PatchChange.hpp
+++ b/libs/evoral/evoral/PatchChange.hpp
@@ -119,7 +119,7 @@ public:
 	uint8_t channel () const { return _program_change.buffer()[0] & 0xf; }
 
 	inline bool operator< (const PatchChange<Time>& o) const {
-		if (time() != o.time()) {
+		if (!musical_time_equal (time(), o.time())) {
 			return time() < o.time();
 		}
 
@@ -131,7 +131,7 @@ public:
 	}
 
 	inline bool operator== (const PatchChange<Time>& o) const {
-		return (time() == o.time() && program() == o.program() && bank() == o.bank());
+		return (musical_time_equal (time(), o.time()) && program() == o.program() && bank() == o.bank());
 	}
 
 	/** The PatchChange is made up of messages() MIDI messages; this method returns them by index.
@@ -165,4 +165,10 @@ private:
 
 }
 
+template<typename Time>
+std::ostream& operator<< (std::ostream& o, const Evoral::PatchChange<Time>& p) {
+	o << "Patch Change " << p.id() << " @ " << p.time() << " bank " << (int) p.bank() << " program " << (int) p.program();
+	return o;
+}
+
 #endif
diff --git a/libs/evoral/evoral/Sequence.hpp b/libs/evoral/evoral/Sequence.hpp
index 815d02f980..26bef20232 100644
--- a/libs/evoral/evoral/Sequence.hpp
+++ b/libs/evoral/evoral/Sequence.hpp
@@ -69,8 +69,7 @@ protected:
 	struct WriteLockImpl {
 		WriteLockImpl(Glib::Threads::RWLock& s, Glib::Threads::Mutex& c)
 			: sequence_lock(new Glib::Threads::RWLock::WriterLock(s))
-			, control_lock(new Glib::Threads::Mutex::Lock(c))
-		{ }
+			, control_lock(new Glib::Threads::Mutex::Lock(c)) { }
 		~WriteLockImpl() {
 			delete sequence_lock;
 			delete control_lock;
@@ -88,7 +87,7 @@ public:
 	typedef boost::shared_ptr<WriteLockImpl>            WriteLock;
 
 	virtual ReadLock  read_lock() const { return ReadLock(new Glib::Threads::RWLock::ReaderLock(_lock)); }
-	virtual WriteLock write_lock()      { return WriteLock(new WriteLockImpl(_lock, _control_lock)); }
+        virtual WriteLock write_lock()      { return WriteLock(new WriteLockImpl(_lock, _control_lock)); }
 
 	void clear();
 
@@ -126,7 +125,7 @@ public:
 	struct EarlierNoteComparator {
 		inline bool operator()(const boost::shared_ptr< const Note<Time> > a,
 		                       const boost::shared_ptr< const Note<Time> > b) const {
-			return a->time() < b->time();
+			return musical_time_less_than (a->time(), b->time());
 		}
 	};
 
@@ -134,6 +133,7 @@ public:
 		typedef const Note<Time>* value_type;
 		inline bool operator()(const boost::shared_ptr< const Note<Time> > a,
 		                       const boost::shared_ptr< const Note<Time> > b) const {
+			return musical_time_greater_than (a->time(), b->time());
 			return a->time() > b->time();
 		}
 	};
@@ -142,7 +142,7 @@ public:
 		typedef const Note<Time>* value_type;
 		inline bool operator()(const boost::shared_ptr< const Note<Time> > a,
 		                       const boost::shared_ptr< const Note<Time> > b) const {
-			return a->end_time() > b->end_time();
+			return musical_time_less_than (a->end_time(), b->end_time());
 		}
 	};
 
@@ -186,7 +186,7 @@ public:
 
 	struct EarlierSysExComparator {
 		inline bool operator() (constSysExPtr a, constSysExPtr b) const {
-			return a->time() < b->time();
+			return musical_time_less_than (a->time(), b->time());
 		}
 	};
 
@@ -199,7 +199,7 @@ public:
 
 	struct EarlierPatchChangeComparator {
 		inline bool operator() (constPatchChangePtr a, constPatchChangePtr b) const {
-			return a->time() < b->time();
+			return musical_time_less_than (a->time(), b->time());
 		}
 	};
 
diff --git a/libs/evoral/evoral/types.hpp b/libs/evoral/evoral/types.hpp
index 7bdbdc7a2e..000b79bb94 100644
--- a/libs/evoral/evoral/types.hpp
+++ b/libs/evoral/evoral/types.hpp
@@ -43,6 +43,33 @@ static inline bool musical_time_equal (MusicalTime a, MusicalTime b) {
 	return fabs (a - b) <= (1.0/1920.0);
 }
 
+static inline bool musical_time_less_than (MusicalTime a, MusicalTime b) {
+	/* acceptable tolerance is 1 tick. Nice if there was no magic number here */
+	if (fabs (a - b) <= (1.0/1920.0)) {
+		return false; /* effectively identical */
+	} else {
+		return a < b;
+	}
+}
+
+static inline bool musical_time_greater_than (MusicalTime a, MusicalTime b) {
+	/* acceptable tolerance is 1 tick. Nice if there was no magic number here */
+	if (fabs (a - b) <= (1.0/1920.0)) {
+		return false; /* effectively identical */
+	} else {
+		return a > b;
+	}
+}
+
+static inline bool musical_time_greater_or_equal_to (MusicalTime a, MusicalTime b) {
+	/* acceptable tolerance is 1 tick. Nice if there was no magic number here */
+	if (fabs (a - b) <= (1.0/1920.0)) {
+		return true; /* effectively identical, note the "or_equal_to" */
+	} else {
+		return a >= b;
+	}
+}
+
 /** Type of an event (opaque, mapped by application) */
 typedef uint32_t EventType;