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

gtk2_ardour/audio_streamview.cc

@@ -18,6 +18,7 @@
 
 #include <cmath>
 #include <cassert>
+#include <utility>
 
 #include <gtkmm.h>
 
@@ -50,6 +51,7 @@
 
 #include "i18n.h"
 
+using namespace std;
 using namespace ARDOUR;
 using namespace PBD;
 using namespace Editing;
@@ -111,17 +113,15 @@ AudioStreamView::set_amplitude_above_axis (gdouble app)
 	return 0;
 }
 
-void
+RegionView*
 AudioStreamView::add_region_view_internal (boost::shared_ptr<Region> r, bool wait_for_waves)
 {
 	AudioRegionView *region_view = 0;
 
-	ENSURE_GUI_THREAD (bind (mem_fun (*this, &AudioStreamView::add_region_view), r));
-
 	boost::shared_ptr<AudioRegion> region = boost::dynamic_pointer_cast<AudioRegion> (r);
 
 	if (region == 0) {
-		return;
+		return NULL;
 	}
 
 	for (list<RegionView *>::iterator i = region_views.begin(); i != region_views.end(); ++i) {
@@ -138,7 +138,7 @@ AudioStreamView::add_region_view_internal (boost::shared_ptr<Region> r, bool wai
 				arv->set_waveform_shape (_waveform_shape);
 			}
 				
-			return;
+			return NULL;
 		}
 	}
 
@@ -199,6 +199,8 @@ AudioStreamView::add_region_view_internal (boost::shared_ptr<Region> r, bool wai
 	region->GoingAway.connect (bind (mem_fun (*this, &AudioStreamView::remove_region_view), boost::weak_ptr<Region> (r)));
 
 	RegionViewAdded (region_view);
+
+	return region_view;
 }
 
 void
@@ -493,7 +495,7 @@ AudioStreamView::setup_rec_box ()
 				
 				nframes_t start = 0;
 				if (rec_regions.size() > 0) {
-					start = rec_regions.back()->start() + _trackview.get_diskstream()->get_captured_frames(rec_regions.size()-1);
+					start = rec_regions.back().first->start() + _trackview.get_diskstream()->get_captured_frames(rec_regions.size()-1);
 				}
 				
 				boost::shared_ptr<AudioRegion> region (boost::dynamic_pointer_cast<AudioRegion>
@@ -501,7 +503,7 @@ AudioStreamView::setup_rec_box ()
 				assert(region);
 				region->set_position (_trackview.session().transport_frame(), this);
 
-				rec_regions.push_back (region);
+				rec_regions.push_back (make_pair(region, (RegionView*)0));
 			}
 			
 			/* start a new rec box */
@@ -538,6 +540,7 @@ AudioStreamView::setup_rec_box ()
 			rec_rect->property_y2() = (double) _trackview.height - 1;
 			rec_rect->property_outline_color_rgba() = color_map[cRecordingRectOutline];
 			rec_rect->property_fill_color_rgba() = fill_color;
+			rec_rect->lower_to_bottom();
 			
 			RecBoxInfo recbox;
 			recbox.rectangle = rec_rect;
@@ -577,17 +580,16 @@ AudioStreamView::setup_rec_box ()
 
 			rec_updating = false;
 			rec_active = false;
-			last_rec_data_frame = 0;
 			
 			/* remove temp regions */
 
-			for (list<boost::shared_ptr<Region> >::iterator iter = rec_regions.begin(); iter != rec_regions.end(); ) {
+			for (list<pair<boost::shared_ptr<Region>,RegionView*> >::iterator iter = rec_regions.begin(); iter != rec_regions.end(); ) {
-				list<boost::shared_ptr<Region> >::iterator tmp;
+				list<pair<boost::shared_ptr<Region>,RegionView*> >::iterator tmp;
 
 				tmp = iter;
 				++tmp;
 
-				(*iter)->drop_references ();
+				(*iter).first->drop_references ();
 
 				iter = tmp;
 			}
@@ -648,9 +650,9 @@ AudioStreamView::update_rec_regions ()
 
 		uint32_t n = 0;
 
-		for (list<boost::shared_ptr<Region> >::iterator iter = rec_regions.begin(); iter != rec_regions.end(); n++) {
+		for (list<pair<boost::shared_ptr<Region>,RegionView*> >::iterator iter = rec_regions.begin(); iter != rec_regions.end(); n++) {
 
-			list<boost::shared_ptr<Region> >::iterator tmp;
+			list<pair<boost::shared_ptr<Region>,RegionView*> >::iterator tmp;
 
 			tmp = iter;
 			++tmp;
@@ -661,14 +663,14 @@ AudioStreamView::update_rec_regions ()
 				continue;
 			}
 			
-			boost::shared_ptr<AudioRegion> region = boost::dynamic_pointer_cast<AudioRegion>(*iter);
+			boost::shared_ptr<AudioRegion> region = boost::dynamic_pointer_cast<AudioRegion>(iter->first);
 			if (!region) {
 				continue;
 			}
 
 			nframes_t origlen = region->length();
 
-			if (region == rec_regions.back() && rec_active) {
+			if (region == rec_regions.back().first && rec_active) {
 
 				if (last_rec_data_frame > region->start()) {
 

gtk2_ardour/audio_streamview.h

@@ -83,7 +83,7 @@ class AudioStreamView : public StreamView
 	void rec_peak_range_ready (nframes_t start, nframes_t cnt, boost::weak_ptr<ARDOUR::Source> src); 
 	void update_rec_regions ();
 	
-	void add_region_view_internal (boost::shared_ptr<ARDOUR::Region>, bool wait_for_waves);
+	RegionView* add_region_view_internal (boost::shared_ptr<ARDOUR::Region>, bool wait_for_waves);
 	void remove_region_view (boost::weak_ptr<ARDOUR::Region> );
 	void remove_audio_region_view (boost::shared_ptr<ARDOUR::AudioRegion> );
 
@@ -109,6 +109,8 @@ class AudioStreamView : public StreamView
 
 	WaveformShape     _waveform_shape;
 	WaveformScale     _waveform_scale;
+	
+	map<boost::shared_ptr<ARDOUR::Source>, bool> rec_data_ready_map;
 };
 
 #endif /* __ardour_audio_streamview_h__ */

gtk2_ardour/midi_streamview.cc

@@ -18,6 +18,7 @@
 
 #include <cmath>
 #include <cassert>
+#include <utility>
 
 #include <gtkmm.h>
 
@@ -45,6 +46,7 @@
 #include "utils.h"
 #include "color.h"
 
+using namespace std;
 using namespace ARDOUR;
 using namespace PBD;
 using namespace Editing;
@@ -69,15 +71,13 @@ MidiStreamView::~MidiStreamView ()
 }
 
 
-void
+RegionView*
 MidiStreamView::add_region_view_internal (boost::shared_ptr<Region> r, bool wait_for_waves)
 {
-	ENSURE_GUI_THREAD (bind (mem_fun (*this, &MidiStreamView::add_region_view), r));
-
 	boost::shared_ptr<MidiRegion> region = boost::dynamic_pointer_cast<MidiRegion> (r);
 
 	if (region == 0) {
-		return;
+		return NULL;
 	}
 
 	MidiRegionView *region_view;
@@ -89,7 +89,7 @@ MidiStreamView::add_region_view_internal (boost::shared_ptr<Region> r, bool wait
 			/* great. we already have a MidiRegionView for this Region. use it again. */
 
 			(*i)->set_valid (true);
-			return;
+			return NULL;
 		}
 	}
 	
@@ -112,6 +112,8 @@ MidiStreamView::add_region_view_internal (boost::shared_ptr<Region> r, bool wait
 	region->GoingAway.connect (bind (mem_fun (*this, &MidiStreamView::remove_region_view), region));
 	
 	RegionViewAdded (region_view);
+
+	return region_view;
 }
 
 // FIXME: code duplication with AudioStreamVIew
@@ -165,26 +167,31 @@ MidiStreamView::setup_rec_box ()
 
 				MidiRegion::SourceList sources;
 				
+				for (list<sigc::connection>::iterator prc = rec_data_ready_connections.begin(); prc != rec_data_ready_connections.end(); ++prc) {
+					(*prc).disconnect();
+				}
+				rec_data_ready_connections.clear();
+
 				// FIXME
 				boost::shared_ptr<MidiDiskstream> mds = boost::dynamic_pointer_cast<MidiDiskstream>(_trackview.get_diskstream());
 				assert(mds);
 
 				sources.push_back(mds->write_source());
 				
-				rec_data_ready_connections.push_back (mds->write_source()->ViewDataRangeReady.connect (bind (mem_fun (*this, &MidiStreamView::rec_data_range_ready), mds->write_source()))); 
+				rec_data_ready_connections.push_back (mds->write_source()->ViewDataRangeReady.connect (bind (mem_fun (*this, &MidiStreamView::rec_data_range_ready), boost::weak_ptr<Source>(mds->write_source())))); 
 
 				// handle multi
 				
 				jack_nframes_t start = 0;
 				if (rec_regions.size() > 0) {
-					start = rec_regions.back()->start() + _trackview.get_diskstream()->get_captured_frames(rec_regions.size()-1);
+					start = rec_regions.back().first->start() + _trackview.get_diskstream()->get_captured_frames(rec_regions.size()-1);
 				}
 				
 				boost::shared_ptr<MidiRegion> region (boost::dynamic_pointer_cast<MidiRegion>
 					(RegionFactory::create (sources, start, 1 , "", 0, (Region::Flag)(Region::DefaultFlags | Region::DoNotSaveState), false)));
 				assert(region);
 				region->set_position (_trackview.session().transport_frame(), this);
-				rec_regions.push_back (region);
+				rec_regions.push_back (make_pair(region, (RegionView*)0));
 				
 				// rec regions are destroyed in setup_rec_box
 
@@ -212,6 +219,7 @@ MidiStreamView::setup_rec_box ()
 			rec_rect->property_y2() = (double) _trackview.height - 1;
 			rec_rect->property_outline_color_rgba() = color_map[cRecordingRectOutline];
 			rec_rect->property_fill_color_rgba() = fill_color;
+			rec_rect->lower_to_bottom();
 			
 			RecBoxInfo recbox;
 			recbox.rectangle = rec_rect;
@@ -251,15 +259,17 @@ MidiStreamView::setup_rec_box ()
 
 			rec_updating = false;
 			rec_active = false;
-			last_rec_data_frame = 0;
 			
 			/* remove temp regions */
 			
-			for (list<boost::shared_ptr<Region> >::iterator iter = rec_regions.begin(); iter != rec_regions.end();) {
+			for (list<pair<boost::shared_ptr<Region>,RegionView*> >::iterator iter = rec_regions.begin(); iter != rec_regions.end();) {
-				list<boost::shared_ptr<Region> >::iterator tmp;
+				list<pair<boost::shared_ptr<Region>,RegionView*> >::iterator tmp;
+				
 				tmp = iter;
 				++tmp;
-				(*iter)->drop_references ();
+
+				(*iter).first->drop_references ();
+
 				iter = tmp;
 			}
 			
@@ -280,16 +290,17 @@ MidiStreamView::setup_rec_box ()
 }
 
 void
-MidiStreamView::update_rec_regions ()
+MidiStreamView::update_rec_regions (boost::shared_ptr<MidiBuffer> data, nframes_t start, nframes_t dur)
 {
-	if (use_rec_regions) {
+	ENSURE_GUI_THREAD (bind (mem_fun (*this, &MidiStreamView::update_rec_regions), data, start, dur));
 
+	if (use_rec_regions) {
 
 		uint32_t n = 0;
 
-		for (list<boost::shared_ptr<Region> >::iterator iter = rec_regions.begin(); iter != rec_regions.end(); n++) {
+		for (list<pair<boost::shared_ptr<Region>,RegionView*> >::iterator iter = rec_regions.begin(); iter != rec_regions.end(); n++) {
 
-			list<boost::shared_ptr<Region> >::iterator tmp;
+			list<pair<boost::shared_ptr<Region>,RegionView*> >::iterator tmp;
 
 			tmp = iter;
 			++tmp;
@@ -300,16 +311,21 @@ MidiStreamView::update_rec_regions ()
 				continue;
 			}
 			
-			boost::shared_ptr<MidiRegion> region = boost::dynamic_pointer_cast<MidiRegion>(*iter);
+			boost::shared_ptr<MidiRegion> region = boost::dynamic_pointer_cast<MidiRegion>(iter->first);
-			assert(region);
+			if (!region) {
+				continue;
+			}
 
-			jack_nframes_t origlen = region->length();
+			nframes_t origlen = region->length();
 			
-			if (region == rec_regions.back() && rec_active) {
+			//cerr << "MIDI URR: " << start << " * " << dur
+			//	<< " (origlen " << origlen << ")" << endl;
 
-				if (last_rec_data_frame > region->start()) {
+			if (region == rec_regions.back().first && rec_active) {
 
-					jack_nframes_t nlen = last_rec_data_frame - region->start();
+				if (start >= region->start()) {
+
+					nframes_t nlen = start + dur - region->start();
 
 					if (nlen != region->length()) {
 
@@ -320,7 +336,7 @@ MidiStreamView::update_rec_regions ()
 
 						if (origlen == 1) {
 							/* our special initial length */
-							add_region_view_internal (region, false);
+							iter->second = add_region_view_internal (region, false);
 						}
 
 						/* also update rect */
@@ -332,7 +348,7 @@ MidiStreamView::update_rec_regions ()
 
 			} else {
 
-				jack_nframes_t nlen = _trackview.get_diskstream()->get_captured_frames(n);
+				nframes_t nlen = _trackview.get_diskstream()->get_captured_frames(n);
 
 				if (nlen != region->length()) {
 
@@ -345,7 +361,7 @@ MidiStreamView::update_rec_regions ()
 						
 						if (origlen == 1) {
 							/* our special initial length */
-							add_region_view_internal (region, false);
+							iter->second = add_region_view_internal (region, false);
 						}
 						
 						/* also hide rect */
@@ -362,26 +378,18 @@ MidiStreamView::update_rec_regions ()
 }
 
 void
-MidiStreamView::rec_data_range_ready (jack_nframes_t start, jack_nframes_t cnt, boost::shared_ptr<Source> src)
+MidiStreamView::rec_data_range_ready (boost::shared_ptr<MidiBuffer> data, jack_nframes_t start, jack_nframes_t dur, boost::weak_ptr<Source> weak_src)
 {
 	// this is called from the butler thread for now
-	// yeah we need a "peak" building thread or something, though there's not really any
-	// work for it to do...  whatever. :)
 	
-	ENSURE_GUI_THREAD(bind (mem_fun (*this, &MidiStreamView::rec_data_range_ready), start, cnt, src));
+	ENSURE_GUI_THREAD(bind (mem_fun (*this, &MidiStreamView::rec_data_range_ready), data, start, dur, weak_src));
 	
-	//cerr << "REC DATA: " << start << " --- " << cnt << endl;
+	boost::shared_ptr<SMFSource> src (boost::dynamic_pointer_cast<SMFSource>(weak_src.lock()));
 	
-	if (rec_data_ready_map.size() == 0 || start+cnt > last_rec_data_frame) {
+	//cerr << src.get() << " MIDI READY: " << start << " * " << dur
-		last_rec_data_frame = start + cnt;
+	//	<< " -- " << data->size() << " events!" << endl;
-	}
 	
-	rec_data_ready_map[src] = true;
+	this->update_rec_regions (data, start, dur);
-
-	if (rec_data_ready_map.size() == _trackview.get_diskstream()->n_channels().n_midi()) {
-		this->update_rec_regions ();
-		rec_data_ready_map.clear();
-	}
 }
 
 void

gtk2_ardour/midi_streamview.h

@@ -20,7 +20,6 @@
 #define __ardour_midi_streamview_h__
 
 #include <list>
-#include <map>
 #include <cmath>
 
 #include <ardour/location.h>
@@ -62,10 +61,10 @@ class MidiStreamView : public StreamView
 
   private:
 	void setup_rec_box ();
-	void rec_data_range_ready (jack_nframes_t start, jack_nframes_t cnt, boost::shared_ptr<ARDOUR::Source> src); 
+	void rec_data_range_ready (boost::shared_ptr<ARDOUR::MidiBuffer> data, jack_nframes_t start, jack_nframes_t dur, boost::weak_ptr<ARDOUR::Source> src); 
-	void update_rec_regions ();
+	void update_rec_regions (boost::shared_ptr<ARDOUR::MidiBuffer> data, jack_nframes_t start, jack_nframes_t dur);
 	
-	void add_region_view_internal (boost::shared_ptr<ARDOUR::Region>, bool wait_for_waves);
+	RegionView* add_region_view_internal (boost::shared_ptr<ARDOUR::Region>, bool wait_for_waves);
 
 	void redisplay_diskstream ();
 

gtk2_ardour/streamview.h

@@ -20,7 +20,6 @@
 #define __ardour_streamview_h__
 
 #include <list>
-#include <map>
 #include <cmath>
 
 #include <ardour/location.h>
@@ -107,9 +106,9 @@ protected:
 	void         sess_rec_enable_changed();
 	virtual void setup_rec_box () = 0;
 	void         update_rec_box ();
-	virtual void update_rec_regions () = 0;
+	//virtual void update_rec_regions () = 0;
 	
-	virtual void add_region_view_internal (boost::shared_ptr<ARDOUR::Region>, bool wait_for_waves) = 0;
+	virtual RegionView* add_region_view_internal (boost::shared_ptr<ARDOUR::Region>, bool wait_for_waves) = 0;
 	virtual void remove_region_view (boost::weak_ptr<ARDOUR::Region> );
 	//void         remove_rec_region (boost::shared_ptr<ARDOUR::Region>); (unused)
 
@@ -137,7 +136,7 @@ protected:
 
 	sigc::connection       screen_update_connection;
 	vector<RecBoxInfo>     rec_rects;
-	list<boost::shared_ptr<ARDOUR::Region> > rec_regions;
+	list< std::pair<boost::shared_ptr<ARDOUR::Region>,RegionView* > > rec_regions;
 	bool                   rec_updating;
 	bool                   rec_active;
 	bool                   use_rec_regions;
@@ -154,7 +153,6 @@ protected:
 
 	list<sigc::connection> rec_data_ready_connections;
 	jack_nframes_t         last_rec_data_frame;
-	map<boost::shared_ptr<ARDOUR::Source>, bool> rec_data_ready_map;
 };
 
 #endif /* __ardour_streamview_h__ */

libs/ardour/ardour/buffer.h

@@ -198,17 +198,13 @@ public:
 	void read_from(const Buffer& src, nframes_t nframes, nframes_t offset);
 
 	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);            
@@ -222,7 +218,7 @@ private:
 	 */
 
 	MidiEvent* _events; ///< Event structs that point to offsets in _data
-	RawMidi*   _data;       ///< MIDI, straight up.  No time stamps.
+	Byte*      _data;   ///< MIDI, straight up.  No time stamps.
 };
 
 } // namespace ARDOUR

libs/ardour/ardour/midi_diskstream.h

@@ -145,18 +145,11 @@ 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;
 	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;
 };
 

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.
  *
- * ALL publically accessible sizes refer to event COUNTS.  What actually goes
+ * This is Raul::RingBuffer, lifted for MIDIRingBuffer to inherit from as it works
- * on in here is none of the callers business :)
+ * 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...
+ *
+ * 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())
+		, _buf(new T[size])
-		, _ev_buf(new MidiEvent[size])
-		, _raw_buf(new RawMidi[size * _max_event_size])
 	{
-		reset ();
+		reset();
 		assert(read_space() == 0);
 		assert(write_space() == size - 1);
 	}
 	
-	virtual ~MidiRingBuffer() {
+	virtual ~MidiRingBufferBase() {
-		delete[] _ev_buf;
+		delete[] _buf;
-		delete[] _raw_buf;
 	}
 
-	void reset () {
+	/** Reset(empty) the ringbuffer.
-		/* !!! NOT THREAD SAFE !!! */
+	 * NOT thread safe.
-		g_atomic_int_set (&_write_ptr, 0);
+	 */
-		g_atomic_int_set (&_read_ptr, 0);
+	void reset() {
+		g_atomic_int_set(&_write_ptr, 0);
+		g_atomic_int_set(&_read_ptr, 0);
 	}
 
-	size_t write_space () {
+	size_t write_space() const {
-		size_t w, r;
 		
-		w = g_atomic_int_get (&_write_ptr);
+		const size_t w = g_atomic_int_get(&_write_ptr);
-		r = g_atomic_int_get (&_read_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 read_space() const {
-		size_t w, r;
 		
-		w = g_atomic_int_get (&_write_ptr);
+		const size_t w = g_atomic_int_get(&_write_ptr);
-		r = g_atomic_int_get (&_read_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 peek(size_t size, T* dst);
-	//size_t read(MidiBuffer& out);
+	bool   full_peek(size_t size, T* dst);
 
-	/** Write one event (@a in) */
+	size_t read(size_t size, T* dst);
-	size_t write(const MidiEvent& in); // deep copies in
+	bool   full_read(size_t size, T* dst);
 	
-	/** Read events all events up to time @a end into @a out, leaving stamps intact.
+	void   write(size_t size, const T* src);
-	 * Any events before @a start will be dropped. */
-	size_t read(MidiBuffer& out, nframes_t start, nframes_t end);
 
-	/** Write all events from @a in, applying @a offset to all time stamps */
+protected:
-	size_t write(const MidiBuffer& in, nframes_t offset = 0);
-
-	inline void clear_event(size_t index);
-
-private:
-
-	// _event_ indices
 	mutable gint _write_ptr;
 	mutable gint _read_ptr;
 	
-	size_t     _size;           // size (capacity) in events
+	size_t _size; ///< Size (capacity) in bytes
-	size_t     _max_event_size; // ratio of raw_buf size to ev_buf size
+	T*  _buf;  ///< size, event, size, event...
-	MidiEvent* _ev_buf;         // these point into...
-	RawMidi*   _raw_buf;        // this
-
 };
 
-/** Just for sanity checking */
-inline void
-MidiRingBuffer::clear_event(size_t index)
-{
-	memset(&_ev_buf[index].buffer, 0, _max_event_size);
-	_ev_buf[index].time = 0;
-	_ev_buf[index].size = 0;
-	_ev_buf[index].buffer = 0;
 
+/** 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);
+
+	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 size_t
+
-MidiRingBuffer::write (const MidiEvent& ev)
+template<typename T>
+bool
+MidiRingBufferBase<T>::full_peek(size_t size, T* dst)
 {
-	//static nframes_t last_write_time = 0;
+	if (read_space() < size)
+		return false;
 
-	assert(ev.size > 0);
+	const size_t read_size = peek(size, dst);
 	
-	size_t priv_write_ptr = g_atomic_int_get(&_write_ptr);
+	if (read_size < size)
+		peek(size - read_size, dst + read_size);
 
-	if (write_space () == 0) {
+	return true;
-		return 0;
+}
+
+
+/** 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)
+{
+	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;
+}
+
+
+template<typename T>
+bool
+MidiRingBufferBase<T>::full_read(size_t size, T* dst)
+{
+	if (read_space() < size)
+		return false;
+
+	const size_t read_size = read(size, dst);
+	
+	if (read_size < size)
+		read(size - read_size, dst + read_size);
+
+	return true;
+}
+
+
+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);
-		const size_t raw_index = priv_write_ptr * _max_event_size;
+		assert(priv_write_ptr + this_size <= _size);
-
+		memcpy(&_buf[priv_write_ptr], src, this_size);
-		MidiEvent* const write_ev = &_ev_buf[priv_write_ptr];
+		memcpy(&_buf[0], src+this_size, size - this_size);
-		*write_ev = ev;
+        g_atomic_int_set(&_write_ptr, size - this_size);
-
-		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());
-
-		return 1;
 	}
 }
 
+
+/* ******************************************************************** */
+	
+
+/** 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);
+};
+
+
+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);
+
+	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);
+	MidiEvent ev;
-	nframes_t time          = _ev_buf[priv_read_ptr].time;
+
-	size_t         count         = 0;
+	size_t count = 0;
-	size_t         limit         = read_space();
+
+	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;
 			
-	while (time <= end && limit > 0) {
-		MidiEvent* const read_ev = &_ev_buf[priv_read_ptr];
-		if (time >= start) {
-			dst.push_back(*read_ev);
 			//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",
+			printf("MRB - SKIPPING - %#X %d %d with time %u\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);
 			break;
 		}
 
-		clear_event(priv_read_ptr);
-
 		++count;
-		--limit;
 
-		priv_read_ptr = (priv_read_ptr + 1) % _size;
+		assert(ev.time <= end);
-		
-		assert(read_ev->time <= end);
-		time = _ev_buf[priv_read_ptr].time;
 	}
 	
-	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
 

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 ();
+	mutable sigc::signal<void,boost::shared_ptr<MidiBuffer>,nframes_t,nframes_t> ViewDataRangeReady;
-	static void stop_view_data_thread ();
-	mutable sigc::signal<void,nframes_t,nframes_t> ViewDataRangeReady;
 	
 	XMLNode& get_state ();
 	int set_state (const XMLNode&);

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);

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,

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;
 }
 
 

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;
 }

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 */
@@ -441,9 +440,6 @@ MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t
 	bool      re = record_enabled ();
 	bool      collect_playback = false;
 
-	/*_current_capture_buffer = 0;
-	  _current_playback_buffer = 0;*/
-
 	/* if we've already processed the frames corresponding to this call,
 	   just return. this allows multiple routes that are taking input
 	   from this diskstream to call our ::process() method, but have
@@ -456,6 +452,8 @@ MidiDiskstream::process (nframes_t transport_frame, nframes_t nframes, nframes_t
 		return 0;
 	}
 	
+	commit_should_unlock = false;
+
 	check_record_status (transport_frame, nframes, can_record);
 
 	nominally_recording = (can_record && re);
@@ -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!)
+		// Pump entire port buffer into the ring buffer (FIXME: split cycles?)
-		_capture_buf->write(_source_port->get_midi_buffer(), transport_frame);
+		//_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
+		for (size_t i=0; i < to_write; ++i) {
-		//_write_source->ViewDataRangeReady (_write_source->length(), rec_nframes); /* EMIT SIGNAL */
+			MidiEvent& ev = _source_port->get_midi_buffer()[i];
-		/*
+			_capture_buf->write(ev.time + transport_frame, ev.size, ev.buffer);
-		   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";
-		   */
 	} 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;
 	}
 	
+	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.  etc?
-			// Swap note ons with note offs here
+			// 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;
 
-
+	if (total == 0 || _capture_buf->read_space() == 0  && _session.transport_speed() == 0 || (total < disk_io_chunk_frames && !force_flush && was_recording)) {
-	// 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";
 		goto out;
 	}
 
@@ -954,20 +948,18 @@ MidiDiskstream::do_flush (Session::RunContext context, bool force_flush)
 
 	assert(!destructive());
 
+	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) {
-		//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";
 		}
-
+	}
-	//(*chan).curr_capture_cnt += to_write;
 
 out:
 	//return ret;
-	return 0;
+	return 0; // FIXME: everything's fine!  always!  honest!
 }
 
 void
@@ -1030,29 +1022,20 @@ 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;
+		srcs.push_back (_write_source);
-
+		_write_source->update_header (capture_info.front()->start, when, twhen);
-		if (s) {
+		_write_source->set_captured_for (_name);
-
-			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);
-
-		}
 
 		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);
 
+		try {
 			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";
@@ -1091,6 +1075,8 @@ MidiDiskstream::transport_stopped (struct tm& when, time_t twhen, bool abort_cap
 				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);
 
 			// cerr << "add new region, buffer position = " << buffer_position << " @ " << (*ci)->start << endl;
@@ -1106,12 +1092,12 @@ 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);
 
-	}
-
 	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 ();
 	}
+	use_new_write_source (0);
 			
-	if (!_write_source) {
+	if (record_enabled()) {
-		use_new_write_source ();
+		//_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
+/** Writes playback events in the given range to \a dst, translating time stamps
- * so that an event at start has time = 0
+ * 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;
 	

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;
 
-	//_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;
 }
 

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;
 }

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;
+	_write_data_count = 0;
 
-	MidiBuffer buf(1024); // FIXME: allocation, size?
+	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) {
+	const nframes_t oldlen = _length;
-		ViewDataRangeReady (_length, cnt); /* EMIT SIGNAL */
+	update_length(oldlen, cnt);
-	}
+
+	ViewDataRangeReady (buf_ptr, oldlen, cnt); /* EMIT SIGNAL */
 	
-	update_length(_length, cnt);
 	return cnt;
 }