Use double MIDI timestamps (towards tempo based time, and more-than-sample-accurate LV2 MIDI plugin application).

Eliminate double iteration over MIDIRingBuffer read to translate timestamps.


git-svn-id: svn://localhost/ardour2/trunk@1981 d708f5d6-7413-0410-9779-e7cbd77b26cf

11 files changed, 102 insertions, 94 deletions

diff --git a/gtk2_ardour/midi_region_view.cc b/gtk2_ardour/midi_region_view.cc
index 918505b123..1750ece8a4 100644
--- a/gtk2_ardour/midi_region_view.cc
+++ b/gtk2_ardour/midi_region_view.cc
@@ -150,12 +150,11 @@ MidiRegionView::end_write()
 void
 MidiRegionView::add_event (const MidiEvent& ev)
 {
-	printf("Event, time = %u, size = %zu, data = ",
-	  ev.time, ev.size);
-	  for (size_t i=0; i < ev.size; ++i) {
-	  printf("%X ", ev.buffer[i]);
-	  }
-	  printf("\n\n");
+	printf("Event, time = %f, size = %zu, data = ", ev.time, ev.size);
+	for (size_t i=0; i < ev.size; ++i) {
+		printf("%X ", ev.buffer[i]);
+	}
+	printf("\n\n");
 
 	double y1 = trackview.height / 2.0;
 	if ((ev.buffer[0] & 0xF0) == MIDI_CMD_NOTE_ON) {
@@ -165,7 +164,7 @@ MidiRegionView::add_event (const MidiEvent& ev)
 		ArdourCanvas::SimpleRect * ev_rect = new Gnome::Canvas::SimpleRect(
 				*(ArdourCanvas::Group*)get_canvas_group());
 		ev_rect->property_x1() = trackview.editor.frame_to_pixel (
-				ev.time);
+				(nframes_t)ev.time);
 		ev_rect->property_y1() = y1;
 		ev_rect->property_x2() = trackview.editor.frame_to_pixel (
 				_region->length());
@@ -182,7 +181,7 @@ MidiRegionView::add_event (const MidiEvent& ev)
 	} else if ((ev.buffer[0] & 0xF0) == MIDI_CMD_NOTE_OFF) {
 		const Byte& note = ev.buffer[1];
 		if (_active_notes && _active_notes[note]) {
-			_active_notes[note]->property_x2() = trackview.editor.frame_to_pixel(ev.time);
+			_active_notes[note]->property_x2() = trackview.editor.frame_to_pixel((nframes_t)ev.time);
 			_active_notes[note]->property_outline_what() = (guint32) 0xF; // all edges
 			_active_notes[note] = NULL;
 		}
diff --git a/libs/ardour/ardour/buffer.h b/libs/ardour/ardour/buffer.h
index 61e7a843f1..fe9516cb84 100644
--- a/libs/ardour/ardour/buffer.h
+++ b/libs/ardour/ardour/buffer.h
@@ -190,7 +190,8 @@ public:
 	void read_from(const Buffer& src, nframes_t nframes, nframes_t offset);
 
 	bool  push_back(const ARDOUR::MidiEvent& event);
-	Byte* reserve(nframes_t time, size_t size);
+	bool  push_back(const jack_midi_event_t& event);
+	Byte* reserve(double time, size_t size);
 	
 	const MidiEvent& operator[](size_t i) const { assert(i < _size); return _events[i]; }
 	MidiEvent& operator[](size_t i) { assert(i < _size); return _events[i]; }
diff --git a/libs/ardour/ardour/midi_model.h b/libs/ardour/ardour/midi_model.h
index 509504a472..c68ea668a5 100644
--- a/libs/ardour/ardour/midi_model.h
+++ b/libs/ardour/ardour/midi_model.h
@@ -41,7 +41,7 @@ public:
 	void append(const MidiBuffer& data);
 	
 	/** Resizes vector if necessary (NOT realtime safe) */
-	void append(const MidiEvent& ev);
+	void append(double time, size_t size, Byte* in_buffer);
 	
 	inline const MidiEvent& event_at(unsigned i) const { return _events[i]; }
 
diff --git a/libs/ardour/ardour/midi_ring_buffer.h b/libs/ardour/ardour/midi_ring_buffer.h
index 657ec0852a..0ceedd7d74 100644
--- a/libs/ardour/ardour/midi_ring_buffer.h
+++ b/libs/ardour/ardour/midi_ring_buffer.h
@@ -26,6 +26,9 @@
 namespace ARDOUR {
 
 
+/* FIXME: this is probably too much inlined code */
+
+
 /** A RingBuffer.
  * Read/Write realtime safe.
  * Single-reader Single-writer thread safe.
@@ -226,17 +229,17 @@ public:
 		: MidiRingBufferBase<Byte>(size)
 	{}
 
-	size_t write(nframes_t time, size_t size, const Byte* buf);
-	bool   read(nframes_t time, size_t* size, Byte* buf);
+	size_t write(double time, size_t size, const Byte* buf);
+	bool   read(double* time, size_t* size, Byte* buf);
 
 	size_t read(MidiBuffer& dst, nframes_t start, nframes_t end);
 };
 
 
 inline bool
-MidiRingBuffer::read(nframes_t time, size_t* size, Byte* buf)
+MidiRingBuffer::read(double* time, size_t* size, Byte* buf)
 {
-	bool success = MidiRingBufferBase<Byte>::full_read(sizeof(nframes_t), (Byte*)time);
+	bool success = MidiRingBufferBase<Byte>::full_read(sizeof(double), (Byte*)time);
 	if (success)
 		success = MidiRingBufferBase<Byte>::full_read(sizeof(size_t), (Byte*)size);
 	if (success)
@@ -247,14 +250,14 @@ MidiRingBuffer::read(nframes_t time, size_t* size, Byte* buf)
 
 
 inline size_t
-MidiRingBuffer::write(nframes_t time, size_t size, const Byte* buf)
+MidiRingBuffer::write(double time, size_t size, const Byte* buf)
 {
 	assert(size > 0);
 
-	if (write_space() < (sizeof(nframes_t) + sizeof(size_t) + size)) {
+	if (write_space() < (sizeof(double) + sizeof(size_t) + size)) {
 		return 0;
 	} else {
-		MidiRingBufferBase<Byte>::write(sizeof(nframes_t), (Byte*)&time);
+		MidiRingBufferBase<Byte>::write(sizeof(double), (Byte*)&time);
 		MidiRingBufferBase<Byte>::write(sizeof(size_t), (Byte*)&size);
 		MidiRingBufferBase<Byte>::write(size, buf);
 		return size;
@@ -262,6 +265,11 @@ MidiRingBuffer::write(nframes_t time, size_t size, const Byte* buf)
 }
 
 
+/** Read a block of MIDI events from buffer.
+ *
+ * Timestamps of events returned are relative to start (ie event with stamp 0
+ * occurred at start).
+ */
 inline size_t
 MidiRingBuffer::read(MidiBuffer& dst, nframes_t start, nframes_t end)
 {
@@ -272,14 +280,14 @@ MidiRingBuffer::read(MidiBuffer& dst, nframes_t start, nframes_t end)
 
 	size_t count = 0;
 
-	while (read_space() > sizeof(nframes_t) + sizeof(size_t)) {
+	while (read_space() > sizeof(double) + sizeof(size_t)) {
 	
-		full_peek(sizeof(nframes_t), (Byte*)&ev.time);
+		full_peek(sizeof(double), (Byte*)&ev.time);
 	
 		if (ev.time > end)
 			break;
 
-		bool success = MidiRingBufferBase<Byte>::full_read(sizeof(nframes_t), (Byte*)&ev.time);
+		bool success = MidiRingBufferBase<Byte>::full_read(sizeof(double), (Byte*)&ev.time);
 		if (success)
 			success = MidiRingBufferBase<Byte>::full_read(sizeof(size_t), (Byte*)&ev.size);
 		
@@ -300,13 +308,14 @@ MidiRingBuffer::read(MidiBuffer& dst, nframes_t start, nframes_t end)
 			//	priv_read_ptr);
 			//
 		} else {
-			printf("MRB - SKIPPING EVENT (with time %u)\n", ev.time);
+			printf("MRB - SKIPPING EVENT (with time %f)\n", ev.time);
 			break;
 		}
 
 		++count;
 
 		assert(ev.time <= end);
+		ev.time -= start;
 	}
 	
 	//printf("(R) read space: %zu\n", read_space());
diff --git a/libs/ardour/ardour/smf_source.h b/libs/ardour/ardour/smf_source.h
index b4a0ada555..086c77537d 100644
--- a/libs/ardour/ardour/smf_source.h
+++ b/libs/ardour/ardour/smf_source.h
@@ -110,7 +110,7 @@ class SMFSource : public MidiSource {
 	void     write_chunk(char id[4], uint32_t length, void* data);
 	size_t   write_var_len(uint32_t val);
 	uint32_t read_var_len() const;
-	int      read_event(MidiEvent& ev) const;
+	int      read_event(jack_midi_event_t& ev) const;
 
 	static const uint16_t _ppqn = 19200;
 
@@ -121,7 +121,7 @@ class SMFSource : public MidiSource {
 	bool           _allow_remove_if_empty;
 	uint64_t       _timeline_position;
 	FILE*          _fd;
-	nframes_t _last_ev_time; // last frame time written, relative to source start
+	double         _last_ev_time; // last frame time written, relative to source start
 	uint32_t       _track_size;
 	uint32_t       _header_size; // size of SMF header, including MTrk chunk header
 
diff --git a/libs/ardour/ardour/types.h b/libs/ardour/ardour/types.h
index 3dc4ae2693..d5f10410db 100644
--- a/libs/ardour/ardour/types.h
+++ b/libs/ardour/ardour/types.h
@@ -61,8 +61,19 @@ namespace ARDOUR {
 
 	typedef unsigned char Byte;
 
-	struct MidiEvent : public jack_midi_event_t {
-		MidiEvent() { time = 0; size = 0; buffer = NULL; }
+	/** Identical to jack_midi_event_t, but with double timestamp
+	 *
+	 * time is either a frame time (from/to Jack) or a beat time (internal
+	 * tempo time, used in MidiModel) depending on context.
+	 */
+	struct MidiEvent {
+		MidiEvent(double t=0, size_t s=0, Byte* b=NULL)
+		: time(t), size(s), buffer(b)
+		{}
+		
+		double time;   /**< Sample index (or beat time) at which event is valid */
+		size_t size;   /**< Number of bytes of data in \a buffer */
+		Byte*  buffer; /**< Raw MIDI data */
 	};
 
 	enum IOChange {
diff --git a/libs/ardour/buffer.cc b/libs/ardour/buffer.cc
index 0328122940..2ab70cc96b 100644
--- a/libs/ardour/buffer.cc
+++ b/libs/ardour/buffer.cc
@@ -153,6 +153,35 @@ MidiBuffer::push_back(const MidiEvent& ev)
 }
 
 
+/** Push an event into the buffer.
+ *
+ * Note that the raw MIDI pointed to by ev will be COPIED and unmodified.
+ * That is, the caller still owns it, if it needs freeing it's Not My Problem(TM).
+ * Realtime safe.
+ * @return false if operation failed (not enough room)
+ */
+bool
+MidiBuffer::push_back(const jack_midi_event_t& ev)
+{
+	if (_size == _capacity)
+		return false;
+
+	Byte* const write_loc = _data + (_size * MAX_EVENT_SIZE);
+
+	memcpy(write_loc, ev.buffer, ev.size);
+	_events[_size].time = (double)ev.time;
+	_events[_size].size = ev.size;
+	_events[_size].buffer = write_loc;
+	++_size;
+
+	//cerr << "MidiBuffer: pushed, size = " << _size << endl;
+
+	_silent = false;
+
+	return true;
+}
+
+
 /** Reserve space for a new event in the buffer.
  *
  * This call is for copying MIDI directly into the buffer, the data location
@@ -161,7 +190,7 @@ MidiBuffer::push_back(const MidiEvent& ev)
  * location, or the buffer will be corrupted and very nasty things will happen.
  */
 Byte*
-MidiBuffer::reserve(nframes_t time, size_t size)
+MidiBuffer::reserve(double time, size_t size)
 {
 	assert(size < MAX_EVENT_SIZE);
 
diff --git a/libs/ardour/midi_diskstream.cc b/libs/ardour/midi_diskstream.cc
index 8c033f0d50..1fddeb7eda 100644
--- a/libs/ardour/midi_diskstream.cc
+++ b/libs/ardour/midi_diskstream.cc
@@ -1471,44 +1471,10 @@ MidiDiskstream::get_playback(MidiBuffer& dst, nframes_t start, nframes_t end)
 	
 	// I think this happens with reverse varispeed?  maybe?
 	if (end <= start) {
+		cerr << "MDS: Reverse?  Skipping" << endl;
 		return;
 	}
 
-/*
-	cerr << "MIDI Diskstream pretending to read" << endl;
-
-	MidiEvent ev;
-	Byte data[4];
-
-	const char note = rand()%30 + 30;
-	
-	ev.buffer = data;
-	ev.time = 0;
-	ev.size = 3;
-
-	data[0] = 0x90;
-	data[1] = note;
-	data[2] = 120;
-
-	dst.push_back(ev);
-	
-	ev.buffer = data;
-	ev.time = (end - start) / 2;
-	ev.size = 3;
-
-	data[0] = 0x80;
-	data[1] = note;
-	data[2] = 64;
-*/
+	// Translates stamps to be relative to start
 	_playback_buf->read(dst, start, end);
-
-	// Translate time stamps to be relative to the start of this cycle
-	for (size_t i=0; i < dst.size(); ++i) {
-		assert(dst[i].time >= start);
-		assert(dst[i].time <= end);
-		//cerr << "Translating event stamp " << dst[i].time << " to ";
-		dst[i].time -= start;
-		//cerr << dst[i].time << endl;
-
-	}
 }
diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc
index f12d91ba8a..dc58afa8ea 100644
--- a/libs/ardour/midi_model.cc
+++ b/libs/ardour/midi_model.cc
@@ -38,7 +38,7 @@ MidiModel::~MidiModel()
 }
 
 
-/** Append contents of \a buf to model.  NOT (even remotely) realtime safe.
+/** Append contents of \a buf to model.  NOT realtime safe.
  *
  * Timestamps of events in \a buf are expected to be relative to
  * the start of this model (t=0) and MUST be monotonically increasing
@@ -64,25 +64,21 @@ MidiModel::append(const MidiBuffer& buf)
 }
 
 
-/** Append \a in_event to model.  NOT (even remotely) realtime safe.
+/** Append \a in_event to model.  NOT realtime safe.
  *
  * Timestamps of events in \a buf are expected to be relative to
  * the start of this model (t=0) and MUST be monotonically increasing
  * and MUST be >= the latest event currently in the model.
- *
- * Events in buf are deep copied.
  */
 void
-MidiModel::append(const MidiEvent& in_event)
+MidiModel::append(double time, size_t size, Byte* in_buffer)
 {
-	assert(_events.empty() || in_event.time >= _events.back().time);
+	assert(_events.empty() || time >= _events.back().time);
 
-	_events.push_back(in_event);
-	MidiEvent& my_event = _events.back();
-	assert(my_event.time == in_event.time);
-	assert(my_event.size == in_event.size);
+	cerr << "Model event: time = " << time << endl;
 
-	my_event.buffer = new Byte[my_event.size];
-	memcpy(my_event.buffer, in_event.buffer, my_event.size);
+	Byte* my_buffer = new Byte[size];
+	memcpy(my_buffer, in_buffer, size);
+	_events.push_back(MidiEvent(time, size, my_buffer));
 }
 
diff --git a/libs/ardour/midi_port.cc b/libs/ardour/midi_port.cc
index 1ffd7e93bb..ba8d1aa841 100644
--- a/libs/ardour/midi_port.cc
+++ b/libs/ardour/midi_port.cc
@@ -66,19 +66,13 @@ MidiPort::cycle_start (nframes_t nframes)
 
 	assert(event_count < _buffer.capacity());
 
-	MidiEvent ev;
+	jack_midi_event_t ev;
 
-	// FIXME: too slow, event struct is copied twice (here and MidiBuffer::push_back)
 	for (nframes_t i=0; i < event_count; ++i) {
 
-		// This will fail to compile if we change MidiEvent to our own class
-		jack_midi_event_get(static_cast<jack_midi_event_t*>(&ev), jack_buffer, i);
+		jack_midi_event_get(&ev, jack_buffer, i);
 
 		_buffer.push_back(ev);
-		// Convert note ons with velocity 0 to proper note offs
-		// FIXME: Jack MIDI should guarantee this - does it?
-		//if (ev->buffer[0] == MIDI_CMD_NOTE_ON && ev->buffer[2] == 0)
-		//	ev->buffer[0] = MIDI_CMD_NOTE_OFF;
 	}
 
 	assert(_buffer.size() == event_count);
@@ -106,9 +100,11 @@ MidiPort::cycle_end()
 
 	jack_midi_clear_buffer(jack_buffer);
 	for (nframes_t i=0; i < event_count; ++i) {
-		const jack_midi_event_t& ev = _buffer[i];
+		const MidiEvent& ev = _buffer[i];
+		// event times should be frames, relative to cycle start
+		assert(ev.time >= 0);
 		assert(ev.time < _nframes_this_cycle);
-		jack_midi_event_write(jack_buffer, ev.time, ev.buffer, ev.size);
+		jack_midi_event_write(jack_buffer, (jack_nframes_t)ev.time, ev.buffer, ev.size);
 	}
 	
 	_nframes_this_cycle = 0;
diff --git a/libs/ardour/smf_source.cc b/libs/ardour/smf_source.cc
index b046ad6fca..d1192f5186 100644
--- a/libs/ardour/smf_source.cc
+++ b/libs/ardour/smf_source.cc
@@ -250,7 +250,7 @@ SMFSource::find_first_event_after(nframes_t start)
  * skipped (eg a meta event), or -1 on EOF (or end of track).
  */
 int
-SMFSource::read_event(MidiEvent& ev) const
+SMFSource::read_event(jack_midi_event_t& ev) const
 {
 	// - 4 is for the EOT event, which we don't actually want to read
 	//if (feof(_fd) || ftell(_fd) >= _header_size + _track_size - 4) {
@@ -307,13 +307,12 @@ SMFSource::read_unlocked (MidiRingBuffer& dst, nframes_t start, nframes_t cnt, n
 
 	// FIXME: ugh
 	unsigned char ev_buf[MidiBuffer::max_event_size()];
-	MidiEvent ev;
+	jack_midi_event_t ev; // time in SMF ticks
 	ev.time = 0;
 	ev.size = MidiBuffer::max_event_size();
 	ev.buffer = ev_buf;
 
-	// FIXME: it would be an impressive feat to actually make this any slower :)
-	
+	// FIXME: don't seek to start every read
 	fseek(_fd, _header_size, 0);
 	
 	// FIXME: assumes tempo never changes after start
@@ -787,8 +786,11 @@ SMFSource::load_model(bool lock)
 
 	fseek(_fd, _header_size, 0);
 
-	nframes_t time = 0;
-	MidiEvent ev;
+	uint64_t time = 0; /* in SMF ticks */
+	jack_midi_event_t ev;
+	ev.time = 0;
+	ev.size = 0;
+	ev.buffer = NULL;
 	
 	// FIXME: assumes tempo never changes after start
 	const double frames_per_beat = _session.tempo_map().tempo_at(_timeline_position).frames_per_beat(
@@ -797,13 +799,12 @@ SMFSource::load_model(bool lock)
 	int ret;
 	while ((ret = read_event(ev)) >= 0) {
 		time += ev.time;
-		ev.time = time;
-
-		ev.time = (nframes_t)(ev.time * frames_per_beat / (double)_ppqn);
+		
+		const double ev_time = (double)(time * frames_per_beat / (double)_ppqn); // in frames
 
 		if (ret > 0) { // didn't skip (meta) event
 			//cerr << "ADDING EVENT TO MODEL: " << ev.time << endl;
-			_model->append(ev);
+			_model->append(ev_time, ev.size, ev.buffer);
 		}
 	}
 }