The Big Change: Store time in MidiModel as tempo time, not frame time.

The time stamp of an event is now always tempo, from file to model and
back again.  Frame time is only relevant at playback or recording time,
in the audio thread (MidiModel and MidiBuffer).

I think perhaps we don't need to change the actual time from double (which is
convenient for math), it is the time base conversion that caused problems.
Using a correct equality comparison (i.e.  not == which is not correct for
floating point) should probably make the undo issues go away, in 99.99% of
cases anyway.

There's almost certainly some regressions in here somewhere, but they do not
seem to be time related.  The bugs I'm hitting in testing are old ones that
seem unrelated now, so it's checkpoint time.

This sets us up for fancy things like tempo map import and tempo/meter changes
halfway through MIDI regions, but for now it's still assumed that the tempo
at the start of the region is valid for the duration of the entire region.


git-svn-id: svn://localhost/ardour2/branches/3.0@4582 d708f5d6-7413-0410-9779-e7cbd77b26cf

9 files changed, 151 insertions, 161 deletions

diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h
index 2126b4b162..f5c660d9cb 100644
--- a/libs/ardour/ardour/midi_source.h
+++ b/libs/ardour/ardour/midi_source.h
@@ -57,10 +57,12 @@ class MidiSource : public Source
 	virtual uint32_t    n_channels () const { return 1; }
 	
 	// FIXME: integrate this with the Readable::read interface somehow
-	virtual nframes_t midi_read (MidiRingBuffer<nframes_t>& dst, nframes_t start, nframes_t cnt, nframes_t stamp_offset, nframes_t negative_stamp_offset) const;
+	virtual nframes_t midi_read (MidiRingBuffer<nframes_t>& dst, nframes_t start, nframes_t cnt,
+			nframes_t stamp_offset, nframes_t negative_stamp_offset) const;
 	virtual nframes_t midi_write (MidiRingBuffer<nframes_t>& src, nframes_t cnt);
 
-	virtual void append_event_unlocked(EventTimeUnit unit, const Evoral::Event<TimeType>& ev) = 0;
+	virtual void append_event_unlocked_beats(const Evoral::Event<double>& ev) = 0;
+	virtual void append_event_unlocked_frames(const Evoral::Event<nframes_t>& ev) = 0;
 
 	virtual void mark_for_remove() = 0;
 	virtual void mark_streaming_midi_write_started (NoteMode mode, nframes_t start_time);
@@ -95,6 +97,7 @@ class MidiSource : public Source
 
 	boost::shared_ptr<MidiModel> model() { return _model; }
 	void set_model(boost::shared_ptr<MidiModel> m) { _model = m; }
+	void drop_model() { _model.reset(); }
 
   protected:
 	virtual void flush_midi() = 0;
diff --git a/libs/ardour/ardour/smf_source.h b/libs/ardour/ardour/smf_source.h
index 82f41ebf2d..6cbb449633 100644
--- a/libs/ardour/ardour/smf_source.h
+++ b/libs/ardour/ardour/smf_source.h
@@ -63,8 +63,9 @@ class SMFSource : public MidiSource, public Evoral::SMF {
 
 	void set_allow_remove_if_empty (bool yn);
 	void mark_for_remove();
-
-	void append_event_unlocked(EventTimeUnit unit, const Evoral::Event<double>& ev);
+	
+	void append_event_unlocked_beats(const Evoral::Event<double>& ev);
+	void append_event_unlocked_frames(const Evoral::Event<nframes_t>& ev);
 
 	int move_to_trash (const string trash_dir_name);
 
@@ -83,8 +84,6 @@ class SMFSource : public MidiSource, public Evoral::SMF {
 	void load_model(bool lock=true, bool force_reload=false);
 	void destroy_model();
 
-	double last_event_time() const { return _last_ev_time; }
-
 	void flush_midi();
 
   private:
@@ -111,7 +110,8 @@ class SMFSource : public MidiSource, public Evoral::SMF {
 	Flag           _flags;
 	string         _take_id;
 	bool           _allow_remove_if_empty;
-	double         _last_ev_time;
+	double         _last_ev_time_beats;
+	nframes_t      _last_ev_time_frames;
 
 	static string _search_path;
 };
diff --git a/libs/ardour/ardour/types.h b/libs/ardour/ardour/types.h
index 7532c63312..bbdf0da1d6 100644
--- a/libs/ardour/ardour/types.h
+++ b/libs/ardour/ardour/types.h
@@ -148,11 +148,6 @@ namespace ARDOUR {
 		TrackColor
 	};
 
-	enum EventTimeUnit {
-		Frames,
-		Beats
-	};
-
 	struct BBT_Time {
 	    uint32_t bars;
 	    uint32_t beats;
diff --git a/libs/ardour/audiofilesource.cc b/libs/ardour/audiofilesource.cc
index 0c8e21b503..0064fd0b8e 100644
--- a/libs/ardour/audiofilesource.cc
+++ b/libs/ardour/audiofilesource.cc
@@ -86,11 +86,11 @@ struct SizedSampleBuffer {
 
 Glib::StaticPrivate<SizedSampleBuffer> thread_interleave_buffer = GLIBMM_STATIC_PRIVATE_INIT;
 
+/** Constructor used for existing internal-to-session files.  File must exist. */
 AudioFileSource::AudioFileSource (Session& s, ustring path, Flag flags)
 	: AudioSource (s, path), _flags (flags),
 	  _channel (0)
 {
-	/* constructor used for existing external to session files. file must exist already */
 	_is_embedded = AudioFileSource::determine_embeddedness (path);
 
 	if (init (path, true)) {
@@ -99,11 +99,11 @@ AudioFileSource::AudioFileSource (Session& s, ustring path, Flag flags)
 
 }
 
+/** Constructor used for new internal-to-session files.  File cannot exist. */
 AudioFileSource::AudioFileSource (Session& s, ustring path, Flag flags, SampleFormat samp_format, HeaderFormat hdr_format)
 	: AudioSource (s, path), _flags (flags),
 	  _channel (0)
 {
-	/* constructor used for new internal-to-session files. file cannot exist */
 	_is_embedded = false;
 
 	if (init (path, false)) {
@@ -111,12 +111,11 @@ AudioFileSource::AudioFileSource (Session& s, ustring path, Flag flags, SampleFo
 	}
 }
 
+/** Constructor used for existing internal-to-session files.  File must exist. */
 AudioFileSource::AudioFileSource (Session& s, const XMLNode& node, bool must_exist)
 	: AudioSource (s, node), _flags (Flag (Writable|CanRename))
-          /* _channel is set in set_state() or init() */
+	/* _channel is set in set_state() or init() */
 {
-	/* constructor used for existing internal-to-session files. file must exist */
-
 	if (set_state (node)) {
 		throw failed_constructor ();
 	}
@@ -363,31 +362,23 @@ AudioFileSource::move_to_trash (const ustring& trash_dir_name)
 		return -1;
 	}
 
-	ustring newpath;
-
 	if (!writable()) {
 		return -1;
 	}
 
-	/* don't move the file across filesystems, just
-	   stick it in the `trash_dir_name' directory
-	   on whichever filesystem it was already on.
+	/* don't move the file across filesystems, just stick it in the
+	   trash_dir_name directory on whichever filesystem it was already on
 	*/
 	
+	ustring newpath;
 	newpath = Glib::path_get_dirname (_path);
 	newpath = Glib::path_get_dirname (newpath); 
 
-	cerr << "from " << _path << " dead dir looks like " << newpath << endl;
-
-	newpath += '/';
-	newpath += trash_dir_name;
-	newpath += '/';
+	newpath += string("/") + trash_dir_name + "/";
 	newpath += Glib::path_get_basename (_path);
 
+	/* the new path already exists, try versioning */
 	if (access (newpath.c_str(), F_OK) == 0) {
-
-		/* the new path already exists, try versioning */
-		
 		char buf[PATH_MAX+1];
 		int version = 1;
 		ustring newpath_v;
@@ -401,23 +392,19 @@ AudioFileSource::move_to_trash (const ustring& trash_dir_name)
 		}
 		
 		if (version == 999) {
-			error << string_compose (_("there are already 1000 files with names like %1; versioning discontinued"),
-					  newpath)
-			      << endmsg;
+			PBD::error << string_compose (
+					_("there are already 1000 files with names like %1; versioning discontinued"),
+					newpath)
+				<< endmsg;
 		} else {
 			newpath = newpath_v;
 		}
-
-	} else {
-
-		/* it doesn't exist, or we can't read it or something */
-
 	}
 
 	if (::rename (_path.c_str(), newpath.c_str()) != 0) {
-		error << string_compose (_("cannot rename audio file source from %1 to %2 (%3)"),
-				  _path, newpath, strerror (errno))
-		      << endmsg;
+		PBD::error << string_compose (
+				_("cannot rename midi file source from %1 to %2 (%3)"),
+				_path, newpath, strerror (errno)) << endmsg;
 		return -1;
 	}
 
@@ -434,7 +421,6 @@ AudioFileSource::move_to_trash (const ustring& trash_dir_name)
 	peakpath = "";
 	
 	/* file can not be removed twice, since the operation is not idempotent */
-
 	_flags = Flag (_flags & ~(RemoveAtDestroy|Removable|RemovableIfEmpty));
 
 	return 0;
@@ -467,7 +453,6 @@ AudioFileSource::find (ustring& pathstr, bool must_exist, bool& isnew, uint16_t&
 		cnt = 0;
 		
 		for (vector<ustring>::iterator i = dirs.begin(); i != dirs.end(); ++i) {
-
 			fullpath = *i;
 			if (fullpath[fullpath.length()-1] != '/') {
 				fullpath += '/';
@@ -544,15 +529,14 @@ AudioFileSource::find (ustring& pathstr, bool must_exist, bool& isnew, uint16_t&
 		}
 
 		/* Current find() is unable to parse relative path names to yet non-existant
-                   sources. QuickFix(tm) */
-                if (keeppath == "") {
-                        if (must_exist) {
-                                error << "AudioFileSource::find(), keeppath = \"\", but the file must exist" << endl;
-                        } else {
-                                keeppath = pathstr;
-                        }
-                        
-                }
+		   sources. QuickFix(tm) */
+		if (keeppath == "") {
+			if (must_exist) {
+				error << "AudioFileSource::find(), keeppath = \"\", but the file must exist" << endl;
+			} else {
+				keeppath = pathstr;
+			}
+		}
 
 		_name = pathstr;
 		_path = keeppath;
diff --git a/libs/ardour/import.cc b/libs/ardour/import.cc
index 1b6d604d8f..824cdb43f7 100644
--- a/libs/ardour/import.cc
+++ b/libs/ardour/import.cc
@@ -320,8 +320,8 @@ write_midi_data_to_new_files (Evoral::SMF* source, Session::import_status& statu
 	try {
 
 	for (unsigned i = 1; i <= source->num_tracks(); ++i) {
-	
 		boost::shared_ptr<SMFSource> smfs = boost::dynamic_pointer_cast<SMFSource>(newfiles[i-1]);
+		smfs->drop_model();
 		
 		source->seek_to_track(i);
 	
@@ -346,11 +346,10 @@ write_midi_data_to_new_files (Evoral::SMF* source, Session::import_status& statu
 				continue;
 			}
 
-			smfs->append_event_unlocked(Beats, Evoral::Event<double>(
-						0,
-						(double)t / (double)source->ppqn(),
-						size,
-						buf));
+			smfs->append_event_unlocked_beats(Evoral::Event<double>(0,
+					(double)t / (double)source->ppqn(),
+					size,
+					buf));
 
 			if (status.progress < 0.99)
 				status.progress += 0.01;
diff --git a/libs/ardour/meter.cc b/libs/ardour/meter.cc
index 31a88b16f0..af6b5a0a36 100644
--- a/libs/ardour/meter.cc
+++ b/libs/ardour/meter.cc
@@ -44,7 +44,6 @@ PeakMeter::run_in_place (BufferSet& bufs, nframes_t start_frame, nframes_t end_f
 
 	// Meter what we have (midi)
 	for ( ; n < limit; ++n) {
-	
 		float val = 0;
 		
 		// GUI needs a better MIDI meter, not much information can be
diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc
index 4e755d8717..6573bcfd68 100644
--- a/libs/ardour/midi_model.cc
+++ b/libs/ardour/midi_model.cc
@@ -295,9 +295,11 @@ MidiModel::write_to(boost::shared_ptr<MidiSource> source)
 
 	const bool old_percussive = percussive();
 	set_percussive(false);
+
+	source->drop_model();
 	
 	for (Evoral::Sequence<TimeType>::const_iterator i = begin(); i != end(); ++i) {
-		source->append_event_unlocked(Frames, *i);
+		source->append_event_unlocked_beats(*i);
 	}
 		
 	set_percussive(old_percussive);
diff --git a/libs/ardour/midi_source.cc b/libs/ardour/midi_source.cc
index 4307749e4a..e5b1d813c5 100644
--- a/libs/ardour/midi_source.cc
+++ b/libs/ardour/midi_source.cc
@@ -32,11 +32,13 @@
 #include <pbd/pthread_utils.h>
 #include <pbd/basename.h>
 
-#include <ardour/midi_source.h>
+#include <ardour/audioengine.h>
 #include <ardour/midi_ring_buffer.h>
+#include <ardour/midi_source.h>
 #include <ardour/session.h>
 #include <ardour/session_directory.h>
 #include <ardour/source_factory.h>
+#include <ardour/tempo.h>
 
 #include "i18n.h"
 
@@ -105,28 +107,39 @@ MidiSource::set_state (const XMLNode& node)
 }
 
 nframes_t
-MidiSource::midi_read (MidiRingBuffer<nframes_t>& dst, nframes_t start, nframes_t cnt, nframes_t stamp_offset, nframes_t negative_stamp_offset) const
+MidiSource::midi_read (MidiRingBuffer<nframes_t>& dst, nframes_t start, nframes_t cnt,
+		nframes_t stamp_offset, nframes_t negative_stamp_offset) const
 {
+
 	Glib::Mutex::Lock lm (_lock);
 	if (_model) {
+		// FIXME: assumes tempo never changes after start
+		const Tempo& tempo = _session.tempo_map().tempo_at(_timeline_position);
+		const double frames_per_beat = tempo.frames_per_beat(
+				_session.engine().frame_rate(),
+				_session.tempo_map().meter_at(_timeline_position));
+#define BEATS_TO_FRAMES(t) (((t) * frames_per_beat) + stamp_offset - negative_stamp_offset)
+
 		Evoral::Sequence<double>::const_iterator& i = _model_iter;
 		
 		if (_last_read_end == 0 || start != _last_read_end) {
-			i = _model->begin();
-			cerr << "MidiSource::midi_read seeking to " << start << endl;
-			while (i != _model->end() && i->time() < start)
-				++i;
+			cerr << "MidiSource::midi_read seeking to frame " << start << endl;
+			for (i = _model->begin(); i != _model->end(); ++i) {
+				if (BEATS_TO_FRAMES(i->time()) >= start) {
+					break;
+				}
+			}
 		}
 		
 		_last_read_end = start + cnt;
 
-		if (i == _model->end()) {
-			return cnt;
-		}
-
-		while (i->time() < start + cnt && i != _model->end()) {
-			dst.write(i->time(), i->event_type(), i->size(), i->buffer());
-			++i;
+		for (; i != _model->end(); ++i) {
+			const nframes_t time_frames = BEATS_TO_FRAMES(i->time());
+			if (time_frames < start + cnt) {
+				dst.write(time_frames, i->event_type(), i->size(), i->buffer());
+			} else {
+				break;
+			}
 		}
 		return cnt;
 	} else {
@@ -148,7 +161,7 @@ MidiSource::file_changed (string path)
 
 	int e1 = stat (path.c_str(), &stat_file);
 	
-	return ( !e1 );
+	return !e1;
 }
 
 void
diff --git a/libs/ardour/smf_source.cc b/libs/ardour/smf_source.cc
index efa99c2c00..7072cc8634 100644
--- a/libs/ardour/smf_source.cc
+++ b/libs/ardour/smf_source.cc
@@ -48,15 +48,15 @@ using namespace ARDOUR;
 
 string SMFSource::_search_path;
 
+/** Constructor used for new internal-to-session files.  File cannot exist. */
 SMFSource::SMFSource(Session& s, std::string path, Flag flags)
 	: MidiSource(s, region_name_from_path(path, false))
 	, Evoral::SMF()
 	, _flags(flags)
 	, _allow_remove_if_empty(true)
-	, _last_ev_time(0)
+	, _last_ev_time_beats(0.0)
+	, _last_ev_time_frames(0)
 {
-	/* Constructor used for new internal-to-session files.  File cannot exist. */
-
 	if (init(path, false)) {
 		throw failed_constructor ();
 	}
@@ -68,14 +68,14 @@ SMFSource::SMFSource(Session& s, std::string path, Flag flags)
 	assert(_name.find("/") == string::npos);
 }
 
+/** Constructor used for existing internal-to-session files.  File must exist. */
 SMFSource::SMFSource(Session& s, const XMLNode& node)
 	: MidiSource(s, node)
 	, _flags(Flag(Writable|CanRename))
 	, _allow_remove_if_empty(true)
-	, _last_ev_time(0)
+	, _last_ev_time_beats(0.0)
+	, _last_ev_time_frames(0)
 {
-	/* Constructor used for existing internal-to-session files.  File must exist. */
-
 	if (set_state(node)) {
 		throw failed_constructor ();
 	}
@@ -146,7 +146,6 @@ SMFSource::read_unlocked (MidiRingBuffer<nframes_t>& dst, nframes_t start, nfram
 
 	// FIXME: assumes tempo never changes after start
 	const Tempo& tempo = _session.tempo_map().tempo_at(_timeline_position);
-	
 	const double frames_per_beat = tempo.frames_per_beat(
 			_session.engine().frame_rate(),
 			_session.tempo_map().meter_at(_timeline_position));
@@ -205,7 +204,7 @@ SMFSource::read_unlocked (MidiRingBuffer<nframes_t>& dst, nframes_t start, nfram
 
 /** All stamps in audio frames */
 nframes_t
-SMFSource::write_unlocked (MidiRingBuffer<nframes_t>& src, nframes_t cnt)
+SMFSource::write_unlocked (MidiRingBuffer<nframes_t>& src, nframes_t dur)
 {
 	_write_data_count = 0;
 		
@@ -220,11 +219,11 @@ SMFSource::write_unlocked (MidiRingBuffer<nframes_t>& src, nframes_t cnt)
 		_model->start_write();
 	}
 
-	Evoral::MIDIEvent<double> ev(0, 0.0, 4, NULL, true);
+	Evoral::MIDIEvent<nframes_t> ev(0, 0.0, 4, NULL, true);
 
 	while (true) {
 		bool ret = src.peek_time(&time);
-		if (!ret || time - _timeline_position > _length + cnt) {
+		if (!ret || time - _timeline_position > _length + dur) {
 			break;
 		}
 
@@ -255,11 +254,7 @@ SMFSource::write_unlocked (MidiRingBuffer<nframes_t>& src, nframes_t cnt)
 			continue;
 		}
 		
-		append_event_unlocked(Frames, ev);
-
-		if (_model) {
-			_model->append(ev);
-		}
+		append_event_unlocked_frames(ev);
 	}
 
 	if (_model) {
@@ -270,55 +265,83 @@ SMFSource::write_unlocked (MidiRingBuffer<nframes_t>& src, nframes_t cnt)
 	free(buf);
 
 	const nframes_t oldlen = _length;
-	update_length(oldlen, cnt);
+	update_length(oldlen, dur);
 
-	ViewDataRangeReady(_timeline_position + oldlen, cnt); /* EMIT SIGNAL */
-	
-	return cnt;
+	ViewDataRangeReady(_timeline_position + oldlen, dur); /* EMIT SIGNAL */
+
+	return dur;
 }
 		
 
+/** Append an event with a timestamp in beats (double) */
 void
-SMFSource::append_event_unlocked(EventTimeUnit unit, const Evoral::Event<double>& ev)
+SMFSource::append_event_unlocked_beats(const Evoral::Event<double>& ev)
 {
 	if (ev.size() == 0)  {
-		cerr << "SMFSource: Warning: skipping empty event" << endl;
 		return;
 	}
 
-	/*
-	printf("SMFSource: %s - append_event_unlocked time = %lf, size = %u, data = ",
+	/*printf("SMFSource: %s - append_event_unlocked_beats time = %lf, size = %u, data = ",
 			name().c_str(), ev.time(), ev.size()); 
-	for (size_t i=0; i < ev.size(); ++i) {
-		printf("%X ", ev.buffer()[i]);
-	} printf("\n");
-	*/
+	for (size_t i = 0; i < ev.size(); ++i) printf("%X ", ev.buffer()[i]); printf("\n");*/
 	
 	assert(ev.time() >= 0);
+	if (ev.time() < _last_ev_time_beats) {
+		cerr << "SMFSource: Warning: Skipping event with non-monotonic time" << endl;
+		return;
+	}
 	
-	if (ev.time() < last_event_time()) {
-		cerr << "SMFSource: Warning: Skipping event with ev.time() < last.time()" << endl;
+	const double delta_time_beats   = ev.time() - _last_ev_time_beats;
+	const uint32_t delta_time_ticks = (uint32_t)lrint(delta_time_beats * (double)ppqn());
+
+	Evoral::SMF::append_event_delta(delta_time_ticks, ev.size(), ev.buffer());
+	_last_ev_time_beats = ev.time();
+
+	_write_data_count += ev.size();
+
+	if (_model) {
+		_model->append(ev);
+	}
+}
+
+/** Append an event with a timestamp in frames (nframes_t) */
+void
+SMFSource::append_event_unlocked_frames(const Evoral::Event<nframes_t>& ev)
+{
+	if (ev.size() == 0)  {
 		return;
 	}
+
+	/*printf("SMFSource: %s - append_event_unlocked_frames time = %u, size = %u, data = ",
+			name().c_str(), ev.time(), ev.size()); 
+	for (size_t i=0; i < ev.size(); ++i) printf("%X ", ev.buffer()[i]); printf("\n");*/
 	
-	uint32_t delta_time = 0;
+	assert(ev.time() >= 0);
+	if (ev.time() < _last_ev_time_frames) {
+		cerr << "SMFSource: Warning: Skipping event with non-monotonic time" << endl;
+		return;
+	}
 	
-	if (unit == Frames) {
-		// FIXME: assumes tempo never changes after start
-		const double frames_per_beat = _session.tempo_map().tempo_at(_timeline_position).frames_per_beat(
-				_session.engine().frame_rate(),
-				_session.tempo_map().meter_at(_timeline_position));
+	// FIXME: assumes tempo never changes after start
+	const Tempo& tempo = _session.tempo_map().tempo_at(_timeline_position);
+	const double frames_per_beat = tempo.frames_per_beat(
+			_session.engine().frame_rate(),
+			_session.tempo_map().meter_at(_timeline_position));
 
-		delta_time = (uint32_t)((ev.time() - last_event_time()) / frames_per_beat * ppqn());
-	} else {
-		assert(unit == Beats);
-		delta_time = (uint32_t)((ev.time() - last_event_time()) * ppqn());
-	}
+	uint32_t delta_time = (uint32_t)((ev.time() - _last_ev_time_frames)
+			/ frames_per_beat * (double)ppqn());
 
 	Evoral::SMF::append_event_delta(delta_time, ev.size(), ev.buffer());
-	_last_ev_time = ev.time();
+	_last_ev_time_frames = ev.time();
 
 	_write_data_count += ev.size();
+
+	if (_model) {
+		double beat_time = ev.time() / frames_per_beat;
+		const Evoral::Event<double> beat_ev(
+				ev.event_type(), beat_time, ev.size(), (uint8_t*)ev.buffer());
+		_model->append(beat_ev);
+	}
 }
 
 
@@ -368,7 +391,8 @@ SMFSource::mark_streaming_midi_write_started (NoteMode mode, nframes_t start_fra
 {
 	MidiSource::mark_streaming_midi_write_started (mode, start_frame);
 	Evoral::SMF::begin_write ();
-	_last_ev_time = 0;
+	_last_ev_time_beats = 0.0;
+	_last_ev_time_frames = 0;
 }
 
 void
@@ -395,29 +419,23 @@ SMFSource::mark_take (string id)
 int
 SMFSource::move_to_trash (const string trash_dir_name)
 {
-	string newpath;
-
 	if (!writable()) {
 		return -1;
 	}
 
-	/* don't move the file across filesystems, just
-	   stick it in the 'trash_dir_name' directory
-	   on whichever filesystem it was already on.
+	/* don't move the file across filesystems, just stick it in the
+	   trash_dir_name directory on whichever filesystem it was already on
 	*/
-
+	
+	Glib::ustring newpath;
 	newpath = Glib::path_get_dirname (_path);
-	newpath = Glib::path_get_dirname (newpath);
+	newpath = Glib::path_get_dirname (newpath); 
 
-	newpath += '/';
-	newpath += trash_dir_name;
-	newpath += '/';
+	newpath += string("/") + trash_dir_name + "/";
 	newpath += Glib::path_get_basename (_path);
 
+	/* the new path already exists, try versioning */
 	if (access (newpath.c_str(), F_OK) == 0) {
-
-		/* the new path already exists, try versioning */
-		
 		char buf[PATH_MAX+1];
 		int version = 1;
 		string newpath_v;
@@ -431,28 +449,24 @@ SMFSource::move_to_trash (const string trash_dir_name)
 		}
 		
 		if (version == 999) {
-			PBD::error << string_compose (_("there are already 1000 files with names like %1; versioning discontinued"),
-					  newpath)
-			      << endmsg;
+			PBD::error << string_compose (
+					_("there are already 1000 files with names like %1; versioning discontinued"),
+					newpath) << endmsg;
 		} else {
 			newpath = newpath_v;
 		}
-
-	} else {
-
-		/* it doesn't exist, or we can't read it or something */
-
 	}
 
 	if (::rename (_path.c_str(), newpath.c_str()) != 0) {
-		PBD::error << string_compose (_("cannot rename midi file source from %1 to %2 (%3)"),
-				  _path, newpath, strerror (errno))
-		      << endmsg;
+		PBD::error << string_compose (
+				_("cannot rename midi file source from %1 to %2 (%3)"),
+				_path, newpath, strerror (errno)) << endmsg;
 		return -1;
 	}
 	
+	_path = newpath;
+	
 	/* file can not be removed twice, since the operation is not idempotent */
-
 	_flags = Flag (_flags & ~(RemoveAtDestroy|Removable|RemovableIfEmpty));
 
 	return 0;
@@ -468,19 +482,10 @@ SMFSource::safe_file_extension(const Glib::ustring& file)
 bool
 SMFSource::find (string pathstr, bool must_exist, bool& isnew)
 {
-	string::size_type pos;
 	bool ret = false;
 
 	isnew = false;
 
-	/* clean up PATH:CHANNEL notation so that we are looking for the correct path */
-
-	if ((pos = pathstr.find_last_of (':')) == string::npos) {
-		pathstr = pathstr;
-	} else {
-		pathstr = pathstr.substr (0, pos);
-	}
-
 	if (pathstr[0] != '/') {
 
 		/* non-absolute pathname: find pathstr in search path */
@@ -500,7 +505,6 @@ SMFSource::find (string pathstr, bool must_exist, bool& isnew)
 		cnt = 0;
 		
 		for (vector<string>::iterator i = dirs.begin(); i != dirs.end(); ++i) {
-
 			fullpath = *i;
 			if (fullpath[fullpath.length()-1] != '/') {
 				fullpath += '/';
@@ -640,24 +644,15 @@ SMFSource::load_model(bool lock, bool force_reload)
 	
 	size_t scratch_size = 0; // keep track of scratch and minimize reallocs
 	
-	// FIXME: assumes tempo never changes after start
-	const Tempo& tempo = _session.tempo_map().tempo_at(_timeline_position);
-	
-	const double frames_per_beat = tempo.frames_per_beat(
-			_session.engine().frame_rate(),
-			_session.tempo_map().meter_at(_timeline_position));
-	
 	uint32_t delta_t = 0;
 	uint32_t size    = 0;
 	uint8_t* buf     = NULL;
 	int ret;
 	while ((ret = read_event(&delta_t, &size, &buf)) >= 0) {
-		ev.set(buf, size, 0.0);
 		time += delta_t;
+		ev.set(buf, size, time / (double)ppqn());
 		
 		if (ret > 0) { // didn't skip (meta) event
-			// make ev.time absolute time in frames
-			ev.time() = time * frames_per_beat / (double)ppqn();
 			ev.set_event_type(EventTypeMap::instance().midi_event_type(buf[0]));
 			_model->append(ev);
 		}