Rewrote MidiRingBuffer to more efficiently pack data (flat pack stamps, sizes, and event data into a single buffer).

Eliminate a double-copy on MIDI playback (MidiRingBuffer -> MidiBuffer).
Various MIDI diskstream/source/SMF fixes (only write when appropriate, handle transport locates, etc).
Fix MIDI rec region size/offset problems.
Code cleanups.


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

14 files changed, 355 insertions, 250 deletions

diff --git a/libs/ardour/ardour/buffer.h b/libs/ardour/ardour/buffer.h
index b890afbb03..86a7aa9f95 100644
--- a/libs/ardour/ardour/buffer.h
+++ b/libs/ardour/ardour/buffer.h
@@ -197,18 +197,14 @@ public:
 	
 	void read_from(const Buffer& src, nframes_t nframes, nframes_t offset);
 
-	bool push_back(const MidiEvent& event);
+	bool  push_back(const MidiEvent& event);
+	Byte* reserve(nframes_t 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]; }
 
 	static size_t max_event_size() { return MAX_EVENT_SIZE; }
 
-	//void set_size(size_t size) { _size = size; }
-
-	//const RawMidi* data() const { return _data; }
-	//RawMidi*       data()       { return _data; }
-
 private:
 	// These are undefined (prevent copies)
 	MidiBuffer(const MidiBuffer& copy);            
@@ -221,8 +217,8 @@ private:
 	 * (_size * MAX_EVENT_SIZE)
 	 */
 
-	MidiEvent* _events;     ///< Event structs that point to offsets in _data
-	RawMidi*   _data;       ///< MIDI, straight up.  No time stamps.
+	MidiEvent* _events; ///< Event structs that point to offsets in _data
+	Byte*      _data;   ///< MIDI, straight up.  No time stamps.
 };
 
 } // namespace ARDOUR
diff --git a/libs/ardour/ardour/midi_diskstream.h b/libs/ardour/ardour/midi_diskstream.h
index bc126ce364..cf78d0d05b 100644
--- a/libs/ardour/ardour/midi_diskstream.h
+++ b/libs/ardour/ardour/midi_diskstream.h
@@ -145,19 +145,12 @@ class MidiDiskstream : public Diskstream
 	void engage_record_enable ();
 	void disengage_record_enable ();
 	
-	// FIXME: This is basically a single ChannelInfo.. abstractify that concept?
-	MidiRingBuffer*                    _playback_buf;
-	MidiRingBuffer*                    _capture_buf;
-	//RawMidi*                          _current_playback_buffer;
-	//RawMidi*                          _current_capture_buffer;
-	//RawMidi*                          _playback_wrap_buffer;
-	//RawMidi*                          _capture_wrap_buffer;
+	MidiRingBuffer*                   _playback_buf;
+	MidiRingBuffer*                   _capture_buf;
 	MidiPort*                         _source_port;
 	boost::shared_ptr<SMFSource>      _write_source;
 	RingBufferNPT<CaptureTransition>* _capture_transition_buf;
-	//RingBufferNPT<RawMidi>::rw_vector _playback_vector;
-	//RingBufferNPT<RawMidi>::rw_vector _capture_vector;
-	nframes_t _last_flush_frame;
+	nframes_t                         _last_flush_frame;
 };
 
 }; /* namespace ARDOUR */
diff --git a/libs/ardour/ardour/midi_ring_buffer.h b/libs/ardour/ardour/midi_ring_buffer.h
index 71f42d7182..86319ad4b2 100644
--- a/libs/ardour/ardour/midi_ring_buffer.h
+++ b/libs/ardour/ardour/midi_ring_buffer.h
@@ -21,61 +21,67 @@
 
 #include <algorithm>
 #include <ardour/types.h>
-#include <pbd/ringbufferNPT.h>
 #include <ardour/buffer.h>
 
 namespace ARDOUR {
 
-/** A MIDI RingBuffer
- * (necessary because MIDI events are variable sized so a generic RB won't do).
+
+/** A RingBuffer.
+ * Read/Write realtime safe.
+ * Single-reader Single-writer thread safe.
+ *
+ * This is Raul::RingBuffer, lifted for MIDIRingBuffer to inherit from as it works
+ * a bit differently than PBD::Ringbuffer.  This could/should be replaced with
+ * the PBD ringbuffer to decrease code size, but this code is tested and known to
+ * work, so here it sits for now...
  *
- * ALL publically accessible sizes refer to event COUNTS.  What actually goes
- * on in here is none of the callers business :)
+ * Ignore this class, use MidiRingBuffer.
  */
-class MidiRingBuffer {
+template <typename T>
+class MidiRingBufferBase {
 public:
-	MidiRingBuffer (size_t size)
+
+	/** @param size Size in bytes.
+	 */
+	MidiRingBufferBase(size_t size)
 		: _size(size)
-		, _max_event_size(MidiBuffer::max_event_size())
-		, _ev_buf(new MidiEvent[size])
-		, _raw_buf(new RawMidi[size * _max_event_size])
+		, _buf(new T[size])
 	{
-		reset ();
+		reset();
 		assert(read_space() == 0);
 		assert(write_space() == size - 1);
 	}
 	
-	virtual ~MidiRingBuffer() {
-		delete[] _ev_buf;
-		delete[] _raw_buf;
+	virtual ~MidiRingBufferBase() {
+		delete[] _buf;
 	}
 
-	void reset () {
-		/* !!! NOT THREAD SAFE !!! */
-		g_atomic_int_set (&_write_ptr, 0);
-		g_atomic_int_set (&_read_ptr, 0);
+	/** Reset(empty) the ringbuffer.
+	 * NOT thread safe.
+	 */
+	void reset() {
+		g_atomic_int_set(&_write_ptr, 0);
+		g_atomic_int_set(&_read_ptr, 0);
 	}
 
-	size_t write_space () {
-		size_t w, r;
+	size_t write_space() const {
 		
-		w = g_atomic_int_get (&_write_ptr);
-		r = g_atomic_int_get (&_read_ptr);
+		const size_t w = g_atomic_int_get(&_write_ptr);
+		const size_t r = g_atomic_int_get(&_read_ptr);
 		
 		if (w > r) {
 			return ((r - w + _size) % _size) - 1;
-		} else if (w < r) {
+		} else if(w < r) {
 			return (r - w) - 1;
 		} else {
 			return _size - 1;
 		}
 	}
 	
-	size_t read_space () {
-		size_t w, r;
+	size_t read_space() const {
 		
-		w = g_atomic_int_get (&_write_ptr);
-		r = g_atomic_int_get (&_read_ptr);
+		const size_t w = g_atomic_int_get(&_write_ptr);
+		const size_t r = g_atomic_int_get(&_read_ptr);
 		
 		if (w > r) {
 			return w - r;
@@ -86,139 +92,229 @@ public:
 
 	size_t capacity() const { return _size; }
 
-	/** Read one event and appends it to @a out. */
-	//size_t read(MidiBuffer& out);
+	size_t peek(size_t size, T* dst);
+	bool   full_peek(size_t size, T* dst);
+
+	size_t read(size_t size, T* dst);
+	bool   full_read(size_t size, T* dst);
+	
+	void   write(size_t size, const T* src);
+
+protected:
+	mutable gint _write_ptr;
+	mutable gint _read_ptr;
+	
+	size_t _size; ///< Size (capacity) in bytes
+	T*  _buf;  ///< size, event, size, event...
+};
 
-	/** Write one event (@a in) */
-	size_t write(const MidiEvent& in); // deep copies in
 
-	/** Read events all events up to time @a end into @a out, leaving stamps intact.
-	 * Any events before @a start will be dropped. */
-	size_t read(MidiBuffer& out, nframes_t start, nframes_t end);
+/** Peek at the ringbuffer (read w/o advancing read pointer).
+ *
+ * Note that a full read may not be done if the data wraps around.
+ * Caller must check return value and call again if necessary, or use the 
+ * full_peek method which does this automatically.
+ */
+template<typename T>
+size_t
+MidiRingBufferBase<T>::peek(size_t size, T* dst)
+{
+	const size_t priv_read_ptr = g_atomic_int_get(&_read_ptr);
 
-	/** Write all events from @a in, applying @a offset to all time stamps */
-	size_t write(const MidiBuffer& in, nframes_t offset = 0);
+	const size_t read_size = (priv_read_ptr + size < _size)
+			? size
+			: _size - priv_read_ptr;
+	
+	memcpy(dst, &_buf[priv_read_ptr], read_size);
+        
+	return read_size;
+}
 
-	inline void clear_event(size_t index);
 
-private:
+template<typename T>
+bool
+MidiRingBufferBase<T>::full_peek(size_t size, T* dst)
+{
+	if (read_space() < size)
+		return false;
 
-	// _event_ indices
-	mutable gint _write_ptr;
-	mutable gint _read_ptr;
+	const size_t read_size = peek(size, dst);
 	
-	size_t     _size;           // size (capacity) in events
-	size_t     _max_event_size; // ratio of raw_buf size to ev_buf size
-	MidiEvent* _ev_buf;         // these point into...
-	RawMidi*   _raw_buf;        // this
+	if (read_size < size)
+		peek(size - read_size, dst + read_size);
 
-};
+	return true;
+}
 
-/** Just for sanity checking */
-inline void
-MidiRingBuffer::clear_event(size_t index)
+
+/** Read from the ringbuffer.
+ *
+ * Note that a full read may not be done if the data wraps around.
+ * Caller must check return value and call again if necessary, or use the 
+ * full_read method which does this automatically.
+ */
+template<typename T>
+size_t
+MidiRingBufferBase<T>::read(size_t size, T* dst)
 {
-	memset(&_ev_buf[index].buffer, 0, _max_event_size);
-	_ev_buf[index].time = 0;
-	_ev_buf[index].size = 0;
-	_ev_buf[index].buffer = 0;
+	const size_t priv_read_ptr = g_atomic_int_get(&_read_ptr);
+
+	const size_t read_size = (priv_read_ptr + size < _size)
+			? size
+			: _size - priv_read_ptr;
+	
+	memcpy(dst, &_buf[priv_read_ptr], read_size);
+        
+	g_atomic_int_set(&_read_ptr, (priv_read_ptr + read_size) % _size);
 
+	return read_size;
 }
 
-inline size_t
-MidiRingBuffer::write (const MidiEvent& ev)
+
+template<typename T>
+bool
+MidiRingBufferBase<T>::full_read(size_t size, T* dst)
 {
-	//static nframes_t last_write_time = 0;
+	if (read_space() < size)
+		return false;
+
+	const size_t read_size = read(size, dst);
 	
-	assert(ev.size > 0);
+	if (read_size < size)
+		read(size - read_size, dst + read_size);
 
-	size_t priv_write_ptr = g_atomic_int_get(&_write_ptr);
+	return true;
+}
 
-	if (write_space () == 0) {
-		return 0;
+
+template<typename T>
+inline void
+MidiRingBufferBase<T>::write(size_t size, const T* src)
+{
+	const size_t priv_write_ptr = g_atomic_int_get(&_write_ptr);
+	
+	if (priv_write_ptr + size <= _size) {
+		memcpy(&_buf[priv_write_ptr], src, size);
+        g_atomic_int_set(&_write_ptr, (priv_write_ptr + size) % _size);
 	} else {
-		//assert(ev.time >= last_write_time);
+		const size_t this_size = _size - priv_write_ptr;
+		assert(this_size < size);
+		assert(priv_write_ptr + this_size <= _size);
+		memcpy(&_buf[priv_write_ptr], src, this_size);
+		memcpy(&_buf[0], src+this_size, size - this_size);
+        g_atomic_int_set(&_write_ptr, size - this_size);
+	}
+}
 
-		const size_t raw_index = priv_write_ptr * _max_event_size;
 
-		MidiEvent* const write_ev = &_ev_buf[priv_write_ptr];
-		*write_ev = ev;
+/* ******************************************************************** */
+	
+
+/** A MIDI RingBuffer.
+ *
+ * This is timestamps and MIDI packed sequentially into a single buffer, similarly
+ * to LV2 MIDI.  The buffer looks like this:
+ *
+ * [timestamp][size][size bytes of raw MIDI][timestamp][size][etc..]
+ */
+class MidiRingBuffer : public MidiRingBufferBase<Byte> {
+public:
+
+	/** @param size Size in bytes.
+	 */
+	MidiRingBuffer(size_t size)
+		: 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 read(MidiBuffer& dst, nframes_t start, nframes_t end);
+};
 
-		memcpy(&_raw_buf[raw_index], ev.buffer, ev.size);
-		write_ev->buffer = &_raw_buf[raw_index];
-        g_atomic_int_set(&_write_ptr, (priv_write_ptr + 1) % _size);
-		
-		//printf("MRB - wrote %xd %d %d with time %u at index %zu (raw index %zu)\n",
-		//	write_ev->buffer[0], write_ev->buffer[1], write_ev->buffer[2], write_ev->time,
-		//	priv_write_ptr, raw_index);
-		
-		assert(write_ev->size = ev.size);
 
-		//last_write_time = ev.time;
-		//printf("(W) read space: %zu\n", read_space());
+inline bool
+MidiRingBuffer::read(nframes_t time, size_t* size, Byte* buf)
+{
+	bool success = MidiRingBufferBase<Byte>::full_read(sizeof(nframes_t), (Byte*)time);
+	if (success)
+		success = MidiRingBufferBase<Byte>::full_read(sizeof(size_t), (Byte*)size);
+	if (success)
+		success = MidiRingBufferBase<Byte>::full_read(*size, buf);
+
+	return success;
+}
+
+
+inline size_t
+MidiRingBuffer::write(nframes_t time, size_t size, const Byte* buf)
+{
+	assert(size > 0);
 
-		return 1;
+	if (write_space() < (sizeof(nframes_t) + sizeof(size_t) + size)) {
+		return 0;
+	} else {
+		MidiRingBufferBase<Byte>::write(sizeof(nframes_t), (Byte*)&time);
+		MidiRingBufferBase<Byte>::write(sizeof(size_t), (Byte*)&size);
+		MidiRingBufferBase<Byte>::write(size, buf);
+		return size;
 	}
 }
 
+
 inline size_t
 MidiRingBuffer::read(MidiBuffer& dst, nframes_t start, nframes_t end)
 {
 	if (read_space() == 0)
 		return 0;
 
-	size_t         priv_read_ptr = g_atomic_int_get(&_read_ptr);
-	nframes_t time          = _ev_buf[priv_read_ptr].time;
-	size_t         count         = 0;
-	size_t         limit         = read_space();
+	MidiEvent ev;
 
-	while (time <= end && limit > 0) {
-		MidiEvent* const read_ev = &_ev_buf[priv_read_ptr];
-		if (time >= start) {
-			dst.push_back(*read_ev);
+	size_t count = 0;
+
+	while (read_space() > sizeof(nframes_t) + sizeof(size_t)) {
+	
+		full_peek(sizeof(nframes_t), (Byte*)&ev.time);
+	
+		if (ev.time > end)
+			break;
+
+		bool success = MidiRingBufferBase<Byte>::full_read(sizeof(nframes_t), (Byte*)&ev.time);
+		if (success)
+			success = MidiRingBufferBase<Byte>::full_read(sizeof(size_t), (Byte*)&ev.size);
+		
+		if (!success) {
+			cerr << "MRB: READ ERROR (time/size)" << endl;
+			continue;
+		}
+
+		if (ev.time >= start) {
+			Byte* write_loc = dst.reserve(ev.time, ev.size);
+			success = MidiRingBufferBase<Byte>::full_read(ev.size, write_loc);
+		
+			if (!success)
+				cerr << "MRB: READ ERROR (data)" << endl;
+			
 			//printf("MRB - read %#X %d %d with time %u at index %zu\n",
-			//	read_ev->buffer[0], read_ev->buffer[1], read_ev->buffer[2], read_ev->time,
+			//	ev.buffer[0], ev.buffer[1], ev.buffer[2], ev.time,
 			//	priv_read_ptr);
+			//
 		} else {
-			printf("MRB - SKIPPING - %#X %d %d with time %u at index %zu\n",
-				read_ev->buffer[0], read_ev->buffer[1], read_ev->buffer[2], read_ev->time,
-				priv_read_ptr);
+			printf("MRB - SKIPPING - %#X %d %d with time %u\n",
+					ev.buffer[0], ev.buffer[1], ev.buffer[2], ev.time);
 			break;
 		}
 
-		clear_event(priv_read_ptr);
-
 		++count;
-		--limit;
-		
-		priv_read_ptr = (priv_read_ptr + 1) % _size;
-		
-		assert(read_ev->time <= end);
-		time = _ev_buf[priv_read_ptr].time;
+
+		assert(ev.time <= end);
 	}
 	
-	g_atomic_int_set(&_read_ptr, priv_read_ptr);
-	
 	//printf("(R) read space: %zu\n", read_space());
 
 	return count;
 }
 
-inline size_t
-MidiRingBuffer::write(const MidiBuffer& in, nframes_t offset)
-{
-	size_t num_events = in.size();
-	size_t to_write = std::min(write_space(), num_events);
-
-	// FIXME: double copy :/
-	for (size_t i=0; i < to_write; ++i) {
-		MidiEvent ev = in[i];
-		ev.time += offset;
-		write(ev);
-	}
-
-	return to_write;
-}
 
 } // namespace ARDOUR
 
diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h
index 5504db6ab6..dec9536c74 100644
--- a/libs/ardour/ardour/midi_source.h
+++ b/libs/ardour/ardour/midi_source.h
@@ -30,6 +30,7 @@
 
 #include <ardour/source.h>
 #include <ardour/ardour.h>
+#include <ardour/buffer.h>
 #include <pbd/stateful.h>
 #include <pbd/xml++.h>
 
@@ -62,9 +63,7 @@ class MidiSource : public Source
 	static sigc::signal<void,MidiSource*> MidiSourceCreated;
 	       
 	// The MIDI equivalent to "peaks"
-	static int  start_view_data_thread ();
-	static void stop_view_data_thread ();
-	mutable sigc::signal<void,nframes_t,nframes_t> ViewDataRangeReady;
+	mutable sigc::signal<void,boost::shared_ptr<MidiBuffer>,nframes_t,nframes_t> ViewDataRangeReady;
 	
 	XMLNode& get_state ();
 	int set_state (const XMLNode&);
diff --git a/libs/ardour/ardour/smf_source.h b/libs/ardour/ardour/smf_source.h
index d422a8af9e..6ab81a2ac3 100644
--- a/libs/ardour/ardour/smf_source.h
+++ b/libs/ardour/ardour/smf_source.h
@@ -86,6 +86,8 @@ class SMFSource : public MidiSource {
 	XMLNode& get_state ();
 	int set_state (const XMLNode&);
 
+	void seek_to(nframes_t time);
+
   private:
 
 	int init (string idstr, bool must_exist);
@@ -96,9 +98,9 @@ class SMFSource : public MidiSource {
 	bool find (std::string path, bool must_exist, bool& is_new);
 	bool removable() const;
 	bool writable() const { return _flags & Writable; }
-	
+
 	int open();
-	
+
 	void     write_chunk_header(char id[4], uint32_t length);
 	void     write_chunk(char id[4], uint32_t length, void* data);
 	size_t   write_var_len(uint32_t val);
diff --git a/libs/ardour/ardour/source.h b/libs/ardour/ardour/source.h
index 807ea089e6..2a6e0c8638 100644
--- a/libs/ardour/ardour/source.h
+++ b/libs/ardour/ardour/source.h
@@ -76,18 +76,18 @@ class Source : public PBD::StatefulDestructible
   protected:
 	void update_length (nframes_t pos, nframes_t cnt);
 	
-	Session&          _session;
-	string            _name;
-	DataType          _type;
-	time_t            _timestamp;
-	nframes_t    _length;
+	Session&  _session;
+	string    _name;
+	DataType  _type;
+	time_t    _timestamp;
+	nframes_t _length;
 
 	Glib::Mutex playlist_lock;
 	typedef std::map<boost::shared_ptr<ARDOUR::Playlist>, uint32_t > PlaylistMap;
 	PlaylistMap _playlists;
 
   private:
-	uint32_t          _in_use;
+	uint32_t _in_use;
 };
 
 }
diff --git a/libs/ardour/ardour/types.h b/libs/ardour/ardour/types.h
index 766c3d4907..813c75f673 100644
--- a/libs/ardour/ardour/types.h
+++ b/libs/ardour/ardour/types.h
@@ -60,7 +60,7 @@ namespace ARDOUR {
 	typedef uint32_t                    nframes_t;
 
 	typedef jack_midi_event_t MidiEvent;
-	typedef unsigned char     RawMidi;
+	typedef unsigned char     Byte;
 
 	enum IOChange {
 		NoChange = 0,
diff --git a/libs/ardour/audio_diskstream.cc b/libs/ardour/audio_diskstream.cc
index 7f79196349..8ff9c04442 100644
--- a/libs/ardour/audio_diskstream.cc
+++ b/libs/ardour/audio_diskstream.cc
@@ -1587,7 +1587,7 @@ AudioDiskstream::transport_stopped (struct tm& when, time_t twhen, bool abort_ca
 
 		// cerr << _name << ": there are " << capture_info.size() << " capture_info records\n";
 		
-                XMLNode &before = _playlist->get_state();
+		XMLNode &before = _playlist->get_state();
 		_playlist->freeze ();
 		
 		for (buffer_position = c->front()->write_source->last_capture_start_frame(), ci = capture_info.begin(); ci != capture_info.end(); ++ci) {
diff --git a/libs/ardour/audio_playlist.cc b/libs/ardour/audio_playlist.cc
index 246384689e..d28d88488e 100644
--- a/libs/ardour/audio_playlist.cc
+++ b/libs/ardour/audio_playlist.cc
@@ -123,7 +123,6 @@ ARDOUR::nframes_t
 AudioPlaylist::read (Sample *buf, Sample *mixdown_buffer, float *gain_buffer, nframes_t start,
 		     nframes_t cnt, unsigned chan_n)
 {
-	nframes_t ret = cnt;
 	nframes_t end;
 
 	/* optimizing this memset() away involves a lot of conditionals
@@ -200,7 +199,7 @@ AudioPlaylist::read (Sample *buf, Sample *mixdown_buffer, float *gain_buffer, nf
 		}
 	}
 
-	return ret;
+	return cnt;
 }
 
 
diff --git a/libs/ardour/buffer.cc b/libs/ardour/buffer.cc
index 7635ea2199..0328122940 100644
--- a/libs/ardour/buffer.cc
+++ b/libs/ardour/buffer.cc
@@ -80,10 +80,10 @@ MidiBuffer::MidiBuffer(size_t capacity)
 
 #ifdef NO_POSIX_MEMALIGN
 	_events =  (MidiEvent *) malloc(sizeof(MidiEvent) * capacity);
-	_data =  (RawMidi *) malloc(sizeof(RawMidi) * capacity * MAX_EVENT_SIZE);
+	_data =  (Byte *) malloc(sizeof(Byte) * capacity * MAX_EVENT_SIZE);
 #else
 	posix_memalign((void**)&_events, CPU_CACHE_ALIGN, sizeof(MidiEvent) * capacity);
-	posix_memalign((void**)&_data, CPU_CACHE_ALIGN, sizeof(RawMidi) * capacity * MAX_EVENT_SIZE);
+	posix_memalign((void**)&_data, CPU_CACHE_ALIGN, sizeof(Byte) * capacity * MAX_EVENT_SIZE);
 #endif	
 	assert(_data);
 	assert(_events);
@@ -138,7 +138,7 @@ MidiBuffer::push_back(const MidiEvent& ev)
 	if (_size == _capacity)
 		return false;
 
-	RawMidi* const write_loc = _data + (_size * MAX_EVENT_SIZE);
+	Byte* const write_loc = _data + (_size * MAX_EVENT_SIZE);
 
 	memcpy(write_loc, ev.buffer, ev.size);
 	_events[_size] = ev;
@@ -153,6 +153,36 @@ MidiBuffer::push_back(const MidiEvent& ev)
 }
 
 
+/** Reserve space for a new event in the buffer.
+ *
+ * This call is for copying MIDI directly into the buffer, the data location
+ * (of sufficient size to write \a size bytes) is returned, or NULL on failure.
+ * This call MUST be immediately followed by a write to the returned data
+ * location, or the buffer will be corrupted and very nasty things will happen.
+ */
+Byte*
+MidiBuffer::reserve(nframes_t time, size_t size)
+{
+	assert(size < MAX_EVENT_SIZE);
+
+	if (_size == _capacity)
+		return NULL;
+
+	Byte* const write_loc = _data + (_size * MAX_EVENT_SIZE);
+
+	_events[_size].time = time;
+	_events[_size].size = size;
+	_events[_size].buffer = write_loc;
+	++_size;
+
+	//cerr << "MidiBuffer: reserved, size = " << _size << endl;
+
+	_silent = false;
+
+	return write_loc;
+}
+
+
 void
 MidiBuffer::silence(nframes_t dur, nframes_t offset)
 {
@@ -161,7 +191,7 @@ MidiBuffer::silence(nframes_t dur, nframes_t offset)
 	//assert(dur == _capacity);
 
 	memset(_events, 0, sizeof(MidiEvent) * _capacity);
-	memset(_data, 0, sizeof(RawMidi) * _capacity * MAX_EVENT_SIZE);
+	memset(_data, 0, sizeof(Byte) * _capacity * MAX_EVENT_SIZE);
 	_size = 0;
 	_silent = true;
 }
diff --git a/libs/ardour/midi_diskstream.cc b/libs/ardour/midi_diskstream.cc
index 50fc1cc8a7..8c033f0d50 100644
--- a/libs/ardour/midi_diskstream.cc
+++ b/libs/ardour/midi_diskstream.cc
@@ -121,8 +121,6 @@ MidiDiskstream::init (Diskstream::Flag f)
 	set_block_size (_session.get_block_size());
 	allocate_temporary_buffers ();
 
-	//_playback_wrap_buffer = new RawMidi[wrap_buffer_size];
-	//_capture_wrap_buffer = new RawMidi[wrap_buffer_size];
 	_playback_buf = new MidiRingBuffer (_session.diskstream_buffer_size());
 	_capture_buf = new MidiRingBuffer (_session.diskstream_buffer_size());
 	_capture_transition_buf = new RingBufferNPT<CaptureTransition> (128);
@@ -148,7 +146,6 @@ MidiDiskstream::non_realtime_input_change ()
 		}
 
 		if (input_change_pending & ConfigurationChanged) {
-
 			assert(_io->n_inputs() == _n_channels);
 		} 
 
@@ -163,6 +160,8 @@ MidiDiskstream::non_realtime_input_change ()
 		}
 
 		input_change_pending = NoChange;
+		
+		/* implicit unlock */
 	}
 
 	/* reset capture files */
@@ -353,7 +352,7 @@ MidiDiskstream::check_record_status (nframes_t transport_frame, nframes_t nframe
 			} else {
 				first_recordable_frame += _roll_delay;
   			}
-
+		
 		} else {
 
 			/* was rolling, but record state changed */
@@ -434,15 +433,12 @@ int
 MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t offset, bool can_record, bool rec_monitors_input)
 {
 	// FIXME: waay too much code to duplicate (AudioDiskstream::process)
-	int            ret = -1;
+	int       ret = -1;
 	nframes_t rec_offset = 0;
 	nframes_t rec_nframes = 0;
-	bool           nominally_recording;
-	bool           re = record_enabled ();
-	bool           collect_playback = false;
-
-	/*_current_capture_buffer = 0;
-	  _current_playback_buffer = 0;*/
+	bool      nominally_recording;
+	bool      re = record_enabled ();
+	bool      collect_playback = false;
 
 	/* if we've already processed the frames corresponding to this call,
 	   just return. this allows multiple routes that are taking input
@@ -455,6 +451,8 @@ MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t
 	if (_processed) {
 		return 0;
 	}
+	
+	commit_should_unlock = false;
 
 	check_record_status (transport_frame, nframes, can_record);
 
@@ -474,7 +472,7 @@ MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t
 	if (!state_lock.trylock()) {
 		return 1;
 	}
-
+	commit_should_unlock = true;
 	adjust_capture_position = 0;
 
 	if (nominally_recording || (_session.get_record_enabled() && Config->get_punch_in())) {
@@ -537,19 +535,16 @@ MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t
 
 		assert(_source_port);
 
-		// Pump entire port buffer into the ring buffer (FIXME!)
-		_capture_buf->write(_source_port->get_midi_buffer(), transport_frame);
+		// Pump entire port buffer into the ring buffer (FIXME: split cycles?)
+		//_capture_buf->write(_source_port->get_midi_buffer(), transport_frame);
+		size_t num_events = _source_port->get_midi_buffer().size();
+		size_t to_write = std::min(_capture_buf->write_space(), num_events);
 
-		// FIXME: hackitty hack, don't come back
-		//_write_source->ViewDataRangeReady (_write_source->length(), rec_nframes); /* EMIT SIGNAL */
-		/*
-		   for (size_t i=0; i < _source_port->size(); ++i) {
-		   cerr << "DISKSTREAM GOT EVENT(1) " << i << "!!\n";
-		   }
-
-		   if (_source_port->size() == 0)
-		   cerr << "No events :/ (1)\n";
-		   */
+		for (size_t i=0; i < to_write; ++i) {
+			MidiEvent& ev = _source_port->get_midi_buffer()[i];
+			_capture_buf->write(ev.time + transport_frame, ev.size, ev.buffer);
+		}
+	
 	} else {
 
 		if (was_recording) {
@@ -561,16 +556,23 @@ MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t
 	if (rec_nframes) {
 
 		/* XXX XXX XXX XXX XXX XXX XXX XXX */
+		
 		/* data will be written to disk */
 
+		if (rec_nframes == nframes && rec_offset == 0) {
+
+			playback_distance = nframes;
+		} else {
+		
+			collect_playback = true;
+		}
+
 		adjust_capture_position = rec_nframes;
 
 	} else if (nominally_recording) {
 
 		/* can't do actual capture yet - waiting for latency effects to finish before we start*/
 
-		// Ummm.. well, I suppose we'll just hang out for a bit?
-
 		playback_distance = nframes;
 
 	} else {
@@ -593,7 +595,8 @@ MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t
 		}
 
 		// XXX XXX XXX XXX XXX XXX XXX XXX XXX XXX
-		// Write into playback buffer here, and whatnot
+		// Write into playback buffer here, and whatnot?
+		cerr << "MDS FIXME: collect playback" << endl;
 
 	}
 
@@ -607,14 +610,11 @@ MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t
 		   be called. unlock the state lock.
 		   */
 
+		commit_should_unlock = false;
 		state_lock.unlock();
 	} 
 
 	return ret;
-
-	_processed = true;
-
-	return 0;
 }
 
 bool
@@ -628,15 +628,6 @@ MidiDiskstream::commit (nframes_t nframes)
 		playback_sample += playback_distance;
 	}
 
-	/* XXX XXX XXX XXX XXX XXX XXX XXX XXX XXX */
-
-	/*
-	_playback_buf->increment_read_ptr (playback_distance);
-
-	if (adjust_capture_position) {
-		_capture_buf->increment_write_ptr (adjust_capture_position);
-	}
-*/
 	if (adjust_capture_position != 0) {
 		capture_captured += adjust_capture_position;
 		adjust_capture_position = 0;
@@ -649,7 +640,9 @@ MidiDiskstream::commit (nframes_t nframes)
 			|| _capture_buf->read_space() >= disk_io_chunk_frames;
 	}
 	
-	state_lock.unlock();
+	if (commit_should_unlock) {
+		state_lock.unlock();
+	}
 
 	_processed = false;
 
@@ -684,7 +677,6 @@ MidiDiskstream::seek (nframes_t frame, bool complete_refill)
 
 	playback_sample = frame;
 	file_frame = frame;
-	_last_flush_frame = frame;
 
 	if (complete_refill) {
 		while ((ret = do_refill_with_alloc ()) > 0) ;
@@ -781,8 +773,9 @@ MidiDiskstream::read (nframes_t& start, nframes_t dur, bool reversed)
 		
 		if (reversed) {
 
-			cerr << "Reversed MIDI.. that's just crazy talk." << endl;
-			// Swap note ons with note offs here
+			// Swap note ons with note offs here.  etc?
+			// Fully reversing MIDI required look-ahead (well, behind) to find previous
+			// CC values etc.  hard.
 
 		} else {
 			
@@ -796,6 +789,7 @@ MidiDiskstream::read (nframes_t& start, nframes_t dur, bool reversed)
 		} 
 
 		dur -= this_read;
+		//offset += this_read;
 	}
 
 	return 0;
@@ -825,6 +819,8 @@ MidiDiskstream::do_refill ()
 	   */
 
 	// FIXME: using disk_io_chunk_frames as an event count, not good
+	// count vs duration semantic differences are nonexistant for audio,
+	// which makes translating for MIDI code confusing...
 	if (_playback_buf->write_space() >= (_slaved?3:2) * disk_io_chunk_frames) {
 		ret = 1;
 	}
@@ -921,16 +917,14 @@ MidiDiskstream::do_flush (Session::RunContext context, bool force_flush)
 
 	_write_data_count = 0;
 
-	if (_last_flush_frame > _session.transport_frame()) {
+	if (_last_flush_frame > _session.transport_frame()
+			|| _last_flush_frame < capture_start_frame) {
 		_last_flush_frame = _session.transport_frame();
 	}
 
 	total = _session.transport_frame() - _last_flush_frame;
 
-
-	// FIXME: put this condition back in! (removed for testing)
-	if (total == 0 || (total < disk_io_chunk_frames && !force_flush && was_recording)) {
-		//cerr << "MDS - no flush 1\n";
+	if (total == 0 || _capture_buf->read_space() == 0  && _session.transport_speed() == 0 || (total < disk_io_chunk_frames && !force_flush && was_recording)) {
 		goto out;
 	}
 
@@ -954,20 +948,18 @@ MidiDiskstream::do_flush (Session::RunContext context, bool force_flush)
 
 	assert(!destructive());
 
-	if ((!_write_source) || _write_source->write (*_capture_buf, to_write) != to_write) {
-		//cerr << "MDS - no flush 2\n";
-		error << string_compose(_("MidiDiskstream %1: cannot write to disk"), _id) << endmsg;
-		return -1;
-	} else {
-		_last_flush_frame = _session.transport_frame();
-		//cerr << "MDS - flushed\n";
+	if (record_enabled() && _session.transport_frame() - _last_flush_frame > disk_io_chunk_frames) {
+		if ((!_write_source) || _write_source->write (*_capture_buf, to_write) != to_write) {
+			error << string_compose(_("MidiDiskstream %1: cannot write to disk"), _id) << endmsg;
+			return -1;
+		} else {
+			_last_flush_frame = _session.transport_frame();
+		}
 	}
 
-	//(*chan).curr_capture_cnt += to_write;
-
 out:
 	//return ret;
-	return 0;
+	return 0; // FIXME: everything's fine!  always!  honest!
 }
 
 void
@@ -1029,30 +1021,21 @@ MidiDiskstream::transport_stopped (struct tm& when, time_t twhen, bool abort_cap
 		}
 
 		/* figure out the name for this take */
-
-		boost::shared_ptr<SMFSource> s = _write_source;
-
-		if (s) {
-
-			srcs.push_back (s);
-
-			cerr << "MidiDiskstream: updating source after capture\n";
-			s->update_header (capture_info.front()->start, when, twhen);
-
-			s->set_captured_for (_name);
-
-		}
+	
+		srcs.push_back (_write_source);
+		_write_source->update_header (capture_info.front()->start, when, twhen);
+		_write_source->set_captured_for (_name);
 
 		string whole_file_region_name;
 		whole_file_region_name = region_name_from_path (_write_source->name(), true);
+
 		/* Register a new region with the Session that
 		   describes the entire source. Do this first
 		   so that any sub-regions will obviously be
 		   children of this one (later!)
 		   */
+
 		try {
-			assert(_write_source);
-			
 			boost::shared_ptr<Region> rx (RegionFactory::create (srcs, _write_source->last_capture_start_frame(), total_capture, 
 									     whole_file_region_name, 
 									     0, Region::Flag (Region::DefaultFlags|Region::Automatic|Region::WholeFile)));
@@ -1077,6 +1060,7 @@ MidiDiskstream::transport_stopped (struct tm& when, time_t twhen, bool abort_cap
 		for (buffer_position = _write_source->last_capture_start_frame(), ci = capture_info.begin(); ci != capture_info.end(); ++ci) {
 
 			string region_name;
+
 			_session.region_name (region_name, _write_source->name(), false);
 
 			// cerr << _name << ": based on ci of " << (*ci)->start << " for " << (*ci)->frames << " add a region\n";
@@ -1090,6 +1074,8 @@ MidiDiskstream::transport_stopped (struct tm& when, time_t twhen, bool abort_cap
 				error << _("MidiDiskstream: could not create region for captured midi!") << endmsg;
 				continue; /* XXX is this OK? */
 			}
+			
+			region->GoingAway.connect (bind (mem_fun (*this, &Diskstream::remove_region_from_last_capture), boost::weak_ptr<Region>(region)));
 
 			_last_capture_regions.push_back (region);
 
@@ -1106,11 +1092,11 @@ MidiDiskstream::transport_stopped (struct tm& when, time_t twhen, bool abort_cap
 		XMLNode &after = _playlist->get_state();
 		_session.add_command (new MementoCommand<Playlist>(*_playlist, &before, &after));
 
-		mark_write_completed = true;
+	}
 
-		reset_write_sources (mark_write_completed);
+	mark_write_completed = true;
 
-	}
+	reset_write_sources (mark_write_completed);
 
 	for (ci = capture_info.begin(); ci != capture_info.end(); ++ci) {
 		delete *ci;
@@ -1394,9 +1380,10 @@ MidiDiskstream::reset_write_sources (bool mark_write_complete, bool force)
 	if (_write_source && mark_write_complete) {
 		_write_source->mark_streaming_write_completed ();
 	}
-
-	if (!_write_source) {
-		use_new_write_source ();
+	use_new_write_source (0);
+			
+	if (record_enabled()) {
+		//_capturing_sources.push_back (_write_source);
 	}
 }
 
@@ -1473,8 +1460,8 @@ MidiDiskstream::use_pending_capture_data (XMLNode& node)
 	return 0;
 }
 
-/** Writes playback events in the given range to dst, translating time stamps
- * so that an event at start has time = 0
+/** Writes playback events in the given range to \a dst, translating time stamps
+ * so that an event at \a start has time = 0
  */
 void
 MidiDiskstream::get_playback(MidiBuffer& dst, nframes_t start, nframes_t end)
@@ -1491,7 +1478,7 @@ MidiDiskstream::get_playback(MidiBuffer& dst, nframes_t start, nframes_t end)
 	cerr << "MIDI Diskstream pretending to read" << endl;
 
 	MidiEvent ev;
-	RawMidi data[4];
+	Byte data[4];
 
 	const char note = rand()%30 + 30;
 	
diff --git a/libs/ardour/midi_playlist.cc b/libs/ardour/midi_playlist.cc
index 2e36e34e43..f99ef299a7 100644
--- a/libs/ardour/midi_playlist.cc
+++ b/libs/ardour/midi_playlist.cc
@@ -128,10 +128,9 @@ MidiPlaylist::read (MidiRingBuffer& dst, nframes_t start,
 
 	Glib::Mutex::Lock rm (region_lock);
 
-	nframes_t ret         = 0;
-	nframes_t end         = start + dur - 1;
+	nframes_t end = start + dur - 1;
 
-	//_read_data_count = 0;
+	_read_data_count = 0;
 
 	// relevent regions overlapping start <--> end
 	vector<boost::shared_ptr<Region> > regs;
@@ -150,12 +149,9 @@ MidiPlaylist::read (MidiRingBuffer& dst, nframes_t start,
 		// FIXME: ensure time is monotonic here
 		boost::shared_ptr<MidiRegion> mr = boost::dynamic_pointer_cast<MidiRegion>(*i);
 		mr->read_at (dst, start, dur, chan_n);
-		ret += mr->read_data_count();
+		_read_data_count += mr->read_data_count();
 	}
 
-	_read_data_count += ret;
-	
-	//return ret; FIXME?
 	return dur;
 }
 
diff --git a/libs/ardour/midi_region.cc b/libs/ardour/midi_region.cc
index 16972c37eb..27861677fd 100644
--- a/libs/ardour/midi_region.cc
+++ b/libs/ardour/midi_region.cc
@@ -165,7 +165,7 @@ MidiRegion::_read_at (const SourceList& srcs, MidiRingBuffer& dst, nframes_t pos
 		return 0; /* "read nothing" */
 	}
 
-	_read_data_count += src->read_data_count(); // FIXME: semantics?
+	_read_data_count += src->read_data_count();
 
 	return to_read;
 }
diff --git a/libs/ardour/smf_source.cc b/libs/ardour/smf_source.cc
index d41b47bd68..52a17997a0 100644
--- a/libs/ardour/smf_source.cc
+++ b/libs/ardour/smf_source.cc
@@ -147,7 +147,7 @@ SMFSource::open()
 		uint32_t track_size_be = 0;
 		fread(&track_size_be, 4, 1, _fd);
 		_track_size = GUINT32_FROM_BE(track_size_be);
-		cerr << "SMF - read track size " << _track_size;
+		cerr << "SMF - read track size " << _track_size << endl;
 
 	// We're making a new file
 	} else {
@@ -294,6 +294,8 @@ SMFSource::read_unlocked (MidiRingBuffer& dst, nframes_t start, nframes_t cnt, n
 
 	nframes_t time = 0;
 
+	_read_data_count = 0;
+
 	// FIXME: ugh
 	unsigned char ev_buf[MidiBuffer::max_event_size()];
 	MidiEvent ev;
@@ -326,9 +328,11 @@ SMFSource::read_unlocked (MidiRingBuffer& dst, nframes_t start, nframes_t cnt, n
 				break;
 			} else {
 				ev.time += stamp_offset;
-				dst.write(ev);
+				dst.write(ev.time, ev.size, ev.buffer);
 			}
 		}
+
+		_read_data_count += ev.size;
 	}
 	
 	return cnt;
@@ -337,9 +341,10 @@ SMFSource::read_unlocked (MidiRingBuffer& dst, nframes_t start, nframes_t cnt, n
 nframes_t
 SMFSource::write_unlocked (MidiRingBuffer& src, nframes_t cnt)
 {
-	//cerr << "SMF WRITE -- " << _length << "--" << cnt << endl;
-	
-	MidiBuffer buf(1024); // FIXME: allocation, size?
+	_write_data_count = 0;
+
+	boost::shared_ptr<MidiBuffer> buf_ptr(new MidiBuffer(1024)); // FIXME: size?
+	MidiBuffer& buf = *buf_ptr.get();
 	src.read(buf, /*_length*/0, _length + cnt); // FIXME?
 
 	fseek(_fd, 0, SEEK_END);
@@ -362,15 +367,17 @@ SMFSource::write_unlocked (MidiRingBuffer& src, nframes_t cnt)
 		fwrite(ev.buffer, 1, ev.size, _fd);
 		_last_ev_time += delta_time;
 		_track_size += stamp_size + ev.size;
+
+		_write_data_count += ev.size;
 	}
 
 	fflush(_fd);
 
-	if (buf.size() > 0) {
-		ViewDataRangeReady (_length, cnt); /* EMIT SIGNAL */
-	}
+	const nframes_t oldlen = _length;
+	update_length(oldlen, cnt);
 
-	update_length(_length, cnt);
+	ViewDataRangeReady (buf_ptr, oldlen, cnt); /* EMIT SIGNAL */
+	
 	return cnt;
 }