From: David Robillard Date: Thu, 22 Oct 2009 14:46:47 +0000 (+0000) Subject: Fix MidiBuffer::merge_in_place and add aggressive correctness checking. X-Git-Tag: 3.0-alpha5~2944 X-Git-Url: https://main.carlh.net/gitweb/?a=commitdiff_plain;h=33da74c8e353ac56194956cae8e2b7d74ec1a1b0;p=ardour.git Fix MidiBuffer::merge_in_place and add aggressive correctness checking. git-svn-id: svn://localhost/ardour2/branches/3.0@5854 d708f5d6-7413-0410-9779-e7cbd77b26cf --- diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h index 5298ab2f4f..0192d94e2d 100644 --- a/libs/ardour/ardour/midi_source.h +++ b/libs/ardour/ardour/midi_source.h @@ -132,6 +132,7 @@ class MidiSource : virtual public Source bool _writing; mutable Evoral::Sequence::const_iterator _model_iter; + mutable bool _model_iterator_valid; mutable double _length_beats; mutable sframes_t _last_read_end; diff --git a/libs/ardour/ardour/region.h b/libs/ardour/ardour/region.h index 0f1f10eff8..c1c7ddd5e0 100644 --- a/libs/ardour/ardour/region.h +++ b/libs/ardour/ardour/region.h @@ -107,8 +107,7 @@ class Region const DataType& data_type() const { return _type; } - /** - * Thats how the region parameters play together: + /** How the region parameters play together: * 
 	 * |------------------------------------------------------------------- track
 	 *                    |..........[------------------].....| region
diff --git a/libs/ardour/midi_buffer.cc b/libs/ardour/midi_buffer.cc
index d14a9b95e9..cbf7603c96 100644
--- a/libs/ardour/midi_buffer.cc
+++ b/libs/ardour/midi_buffer.cc
@@ -261,12 +261,32 @@ MidiBuffer::merge_in_place(const MidiBuffer &other)
 		return false;
 	}
 
+#ifndef NDEBUG
+	size_t   test_orig_us_size   = _size;
+	size_t   test_orig_them_size = other._size;
+	TimeType test_time           = 0;
+	size_t   test_us_count       = 0;
+	size_t   test_them_count     = 0;
+	for (iterator i = begin(); i != end(); ++i) {
+		assert(Evoral::midi_event_is_valid((*i).buffer(), (*i).size()));
+		assert((*i).time() >= test_time);
+		test_time = (*i).time();
+		++test_us_count;
+	}
+	test_time = 0;
+	for (const_iterator i = other.begin(); i != other.end(); ++i) {
+		assert(Evoral::midi_event_is_valid((*i).buffer(), (*i).size()));
+		assert((*i).time() >= test_time);
+		test_time = (*i).time();
+		++test_them_count;
+	}
+#endif
+
 	const_iterator them = other.begin();
 	iterator us = begin();
 
 	while (them != other.end()) {
 
-		Evoral::MIDIEvent ev_other (*them);
 		size_t sz = 0;
 		ssize_t src = -1;
 
@@ -274,14 +294,19 @@ MidiBuffer::merge_in_place(const MidiBuffer &other)
 		   the event referenced by "us"
 		*/
 
-		while (them != other.end() && ev_other.time() < (*us).time()) {
+		while (them != other.end() && (*them).time() <= (*us).time()) {
 			if (src == -1) {
 				src = them.offset;
 			}
-			sz += sizeof (TimeType) + ev_other.size();
+			sz += sizeof (TimeType) + (*them).size();
 			++them;
 		}
 
+		if (us != end())
+			cerr << "us @ " << (*us).time() << endl;
+		if (them != other.end())
+			cerr << "them @ " << (*them).time() << endl;
+
 		if (sz) {
 			assert(src >= 0);
 			/* move existing */
@@ -299,7 +324,7 @@ MidiBuffer::merge_in_place(const MidiBuffer &other)
 			   point for the next event(s) from "other"
 			*/
 
-			while (us != end() && (*us).time() < ev_other.time()) {
+			while (us != end() && (*us).time() < (*them).time()) {
 				++us;
 			}
 		}
@@ -307,10 +332,25 @@ MidiBuffer::merge_in_place(const MidiBuffer &other)
 		if (!(us != end())) {
 			/* just append the rest of other */
 			memcpy (_data + us.offset, other._data + them.offset, other._size - them.offset);
+			_size += other._size - them.offset;
 			break;
 		}
 	}
 
+#ifndef NDEBUG
+	assert(_size == test_orig_us_size + test_orig_them_size);
+	size_t test_final_count = 0;
+	test_time = 0;
+	for (iterator i = begin(); i != end(); ++i) {
+		cerr << "CHECK " << test_final_count << " / " << test_us_count + test_them_count << endl;
+		assert(Evoral::midi_event_is_valid((*i).buffer(), (*i).size()));
+		assert((*i).time() >= test_time);
+		test_time = (*i).time();
+		++test_final_count;
+	}
+	assert(test_final_count = test_us_count + test_them_count);
+#endif
+
 	return true;
 }
 
diff --git a/libs/ardour/midi_playlist.cc b/libs/ardour/midi_playlist.cc
index 991e709651..49b5ff2c4f 100644
--- a/libs/ardour/midi_playlist.cc
+++ b/libs/ardour/midi_playlist.cc
@@ -137,7 +137,7 @@ MidiPlaylist::read (MidiRingBuffer& dst, nframes_t start, nframes_t d
 	_read_data_count = 0;
 
 	// relevent regions overlapping start <--> end
-	vector > regs;
+	vector< boost::shared_ptr > regs;
 
 	for (RegionList::iterator i = regions.begin(); i != regions.end(); ++i) {
 		if ((*i)->coverage (start, end) != OverlapNone) {
@@ -164,7 +164,7 @@ MidiPlaylist::read (MidiRingBuffer& dst, nframes_t start, nframes_t d
 
 			NoteTrackers::iterator t = _note_trackers.find ((*i).get());
 			MidiStateTracker* tracker;
-			
+
 			if (t == _note_trackers.end()) {
 				pair newpair;
 				newpair.first = (*i).get();
@@ -173,7 +173,7 @@ MidiPlaylist::read (MidiRingBuffer& dst, nframes_t start, nframes_t d
 			} else {
 				tracker = t->second;
 			}
-				
+
 			mr->read_at (dst, start, dur, chan_n, _note_mode, tracker);
 			_read_data_count += mr->read_data_count();
 		}
diff --git a/libs/ardour/midi_region.cc b/libs/ardour/midi_region.cc
index afa1c0f8cc..9b7a4b8748 100644
--- a/libs/ardour/midi_region.cc
+++ b/libs/ardour/midi_region.cc
@@ -142,8 +142,9 @@ MidiRegion::master_read_at (MidiRingBuffer& out, sframes_t position,
 }
 
 nframes_t
-MidiRegion::_read_at (const SourceList& /*srcs*/, MidiRingBuffer& dst, sframes_t position, nframes_t dur, uint32_t chan_n, 
-		      NoteMode mode, MidiStateTracker* tracker) const
+MidiRegion::_read_at (const SourceList& /*srcs*/,
+		MidiRingBuffer& dst, sframes_t position, nframes_t dur, uint32_t chan_n,
+		NoteMode mode, MidiStateTracker* tracker) const
 {
 	nframes_t internal_offset = 0;
 	nframes_t src_offset      = 0;