add MidiModel::sync_to_source() to push model state into its existing source; mark SMFSource as non-removable after its called SMF::end_write(); some indentation fixes

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

5 files changed, 1005 insertions, 960 deletions

diff --git a/libs/ardour/ardour/midi_model.h b/libs/ardour/ardour/midi_model.h
index f879c201ee..4e348af287 100644
--- a/libs/ardour/ardour/midi_model.h
+++ b/libs/ardour/ardour/midi_model.h
@@ -128,9 +128,10 @@ public:
 	void                     apply_command(Session& session, Command* cmd);
 	void                     apply_command_as_subcommand(Session& session, Command* cmd);
 
+	bool sync_to_source ();
 	bool write_to(boost::shared_ptr<MidiSource> source);
-	bool write_section_to(boost::shared_ptr<MidiSource> source, Evoral::MusicalTime begin = Evoral::MinMusicalTime,
-                              Evoral::MusicalTime end = Evoral::MaxMusicalTime);
+	bool write_section_to (boost::shared_ptr<MidiSource> source, Evoral::MusicalTime begin = Evoral::MinMusicalTime,
+                               Evoral::MusicalTime end = Evoral::MaxMusicalTime);
 
 	// MidiModel doesn't use the normal AutomationList serialisation code
 	// since controller data is stored in the .mid
diff --git a/libs/ardour/midi_diskstream.cc b/libs/ardour/midi_diskstream.cc
index c3c5f53d50..9c0abe53ef 100644
--- a/libs/ardour/midi_diskstream.cc
+++ b/libs/ardour/midi_diskstream.cc
@@ -869,9 +869,9 @@ MidiDiskstream::do_flush (RunContext /*context*/, bool force_flush)
 
 	assert(!destructive());
 
-	if (record_enabled()
-			&& (   (_session.transport_frame() - _last_flush_frame > disk_io_chunk_frames)
-				|| force_flush)) {
+	if (record_enabled() && 
+            ((_session.transport_frame() - _last_flush_frame > disk_io_chunk_frames) || 
+             force_flush)) {
 		if ((!_write_source) || _write_source->midi_write (*_capture_buf, capture_start_frame, to_write) != to_write) {
 			error << string_compose(_("MidiDiskstream %1: cannot write to disk"), _id) << endmsg;
 			return -1;
diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc
index 5926d403c3..6e2c477a99 100644
--- a/libs/ardour/midi_model.cc
+++ b/libs/ardour/midi_model.cc
@@ -128,983 +128,1011 @@ MidiModel::DiffCommand::side_effect_remove(const NotePtr note)
 	side_effect_removals.insert (note);
 }
 
- void
- MidiModel::DiffCommand::change(const NotePtr note, Property prop,
-                                uint8_t new_value)
- {
-         NoteChange change;
-
-         switch (prop) {
-         case NoteNumber:
-                 if (new_value == note->note()) {
-                         return;
-                 }
-                 change.old_value = note->note();
-                 break;
-         case Velocity:
-                 if (new_value == note->velocity()) {
-                         return;
-                 }
-                 change.old_value = note->velocity();
-                 break;
-         case Channel:
-                 if (new_value == note->channel()) {
-                         return;
-                 }
-                 change.old_value = note->channel();
-                 break;
-
-
-         case StartTime:
-                 fatal << "MidiModel::DiffCommand::change() with integer argument called for start time" << endmsg;
-                 /*NOTREACHED*/
-                 break;
-         case Length:
-                 fatal << "MidiModel::DiffCommand::change() with integer argument called for length" << endmsg;
-                 /*NOTREACHED*/
-                 break;
-         }
-
-         change.note = note;
-         change.property = prop;
-         change.new_value = new_value;
-
-         _changes.push_back (change);
- }
-
- void
- MidiModel::DiffCommand::change(const NotePtr note, Property prop,
-                                TimeType new_time)
- {
-         NoteChange change;
-
-         switch (prop) {
-         case NoteNumber:
-         case Channel:
-         case Velocity:
-                 fatal << "MidiModel::DiffCommand::change() with time argument called for note, channel or velocity" << endmsg;
-                 break;
-
-         case StartTime:
-                 if (Evoral::musical_time_equal (note->time(), new_time)) {
-                         return;
-                 }
-                 change.old_time = note->time();
-                 break;
-         case Length:
-                 if (Evoral::musical_time_equal (note->length(), new_time)) {
-                         return;
-                 }
-                 change.old_time = note->length();
-                 break;
-         }
-
-         change.note = note;
-         change.property = prop;
-         change.new_time = new_time;
-
-         _changes.push_back (change);
- }
-
- MidiModel::DiffCommand&
- MidiModel::DiffCommand::operator+= (const DiffCommand& other)
- {
-         if (this == &other) {
-                 return *this;
-         }
-
-         if (_model != other._model) {
-                 return *this;
-         }
-
-         _added_notes.insert (_added_notes.end(), other._added_notes.begin(), other._added_notes.end());
-         _removed_notes.insert (_removed_notes.end(), other._removed_notes.begin(), other._removed_notes.end());
-         side_effect_removals.insert (other.side_effect_removals.begin(), other.side_effect_removals.end());
-         _changes.insert (_changes.end(), other._changes.begin(), other._changes.end());
-
-         return *this;
- }
-
- void
- MidiModel::DiffCommand::operator()()
- {
-         {
-                 MidiModel::WriteLock lock(_model->edit_lock());
-
-                 for (NoteList::iterator i = _added_notes.begin(); i != _added_notes.end(); ++i) {
-                         if (!_model->add_note_unlocked(*i)) {
-                                 /* failed to add it, so don't leave it in the removed list, to
-                                    avoid apparent errors on undo.
-                                  */
-                                 _removed_notes.remove (*i);
-                         }
-                 }
-
-                 for (NoteList::iterator i = _removed_notes.begin(); i != _removed_notes.end(); ++i) {
-                         _model->remove_note_unlocked(*i);
-                 }
-
-                 /* notes we modify in a way that requires remove-then-add to maintain ordering */
-                 set<NotePtr> temporary_removals;
-
-                 for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) {
-                         Property prop = i->property;
-                         switch (prop) {
-                         case NoteNumber:
-                                 if (temporary_removals.find (i->note) == temporary_removals.end()) {
-                                         _model->remove_note_unlocked (i->note);
-                                         temporary_removals.insert (i->note);
-                                 }
-                                 i->note->set_note (i->new_value);
-                                 break;
-
-                         case StartTime:
-                                 if (temporary_removals.find (i->note) == temporary_removals.end()) {
-                                         _model->remove_note_unlocked (i->note);
-                                         temporary_removals.insert (i->note);
-
-                                 }
-                                 i->note->set_time (i->new_time);
-                                 break;
-
-                         case Channel:
-                                 if (temporary_removals.find (i->note) == temporary_removals.end()) {
-                                         _model->remove_note_unlocked (i->note);
-                                         temporary_removals.insert (i->note);
-                                 }
-                                 i->note->set_channel (i->new_value);
-                                 break;
-
-                         /* no remove-then-add required for these properties, since we do not index them
-                          */
-
-                         case Velocity:
-                                 i->note->set_velocity (i->new_value);
-                                 break;
-
-                         case Length:
-                                 i->note->set_length (i->new_time);
-                                 break;
-
-                         }
-                 }
-
-
-                 for (set<NotePtr>::iterator i = temporary_removals.begin(); i != temporary_removals.end(); ++i) {
-                         DiffCommand side_effects (model(), "side effects");
-                         _model->add_note_unlocked (*i, &side_effects);
-                         *this += side_effects;
-                 }
-
-                 if (!side_effect_removals.empty()) {
-                         cerr << "SER: \n";
-                         for (set<NotePtr>::iterator i = side_effect_removals.begin(); i != side_effect_removals.end(); ++i) {
-                                 cerr << "\t" << *i << ' ' << **i << endl;
-                         }
-                 }
-         }
-
-         _model->ContentsChanged(); /* EMIT SIGNAL */
- }
-
- void
- MidiModel::DiffCommand::undo()
- {
-         {
-                 MidiModel::WriteLock lock(_model->edit_lock());
-
-                 for (NoteList::iterator i = _added_notes.begin(); i != _added_notes.end(); ++i) {
-                         _model->remove_note_unlocked(*i);
-                 }
-
-                 for (NoteList::iterator i = _removed_notes.begin(); i != _removed_notes.end(); ++i) {
-                         _model->add_note_unlocked(*i);
-                 }
-
-                 /* notes we modify in a way that requires remove-then-add to maintain ordering */
-                 set<NotePtr> temporary_removals;
-
-                 for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) {
-                         Property prop = i->property;
-                         switch (prop) {
-                         case NoteNumber:
-                                 if (temporary_removals.find (i->note) == temporary_removals.end()) {
-                                         _model->remove_note_unlocked (i->note);
-                                         temporary_removals.insert (i->note);
-                                 }
-                                 i->note->set_note (i->old_value);
-                                 break;
-                         case Velocity:
-                                 i->note->set_velocity (i->old_value);
-                                 break;
-                         case StartTime:
-                                 if (temporary_removals.find (i->note) == temporary_removals.end()) {
-                                         _model->remove_note_unlocked (i->note);
-                                         temporary_removals.insert (i->note);
-                                 }
-                                 i->note->set_time (i->old_time);
-                                 break;
-                         case Length:
-                                 i->note->set_length (i->old_time);
-                                 break;
-                         case Channel:
-                                 if (temporary_removals.find (i->note) == temporary_removals.end()) {
-                                         _model->remove_note_unlocked (i->note);
-                                         temporary_removals.insert (i->note);
-                                 }
-                                 i->note->set_channel (i->old_value);
-                                 break;
-                         }
-                 }
-
-                 for (set<NotePtr>::iterator i = temporary_removals.begin(); i != temporary_removals.end(); ++i) {
-                         _model->add_note_unlocked (*i);
-                 }
-
-                 /* finally add back notes that were removed by the "do". we don't care
-                    about side effects here since the model should be back to its original
-                    state once this is done.
-                  */
-
-                 for (set<NotePtr>::iterator i = side_effect_removals.begin(); i != side_effect_removals.end(); ++i) {
-                         _model->add_note_unlocked (*i);
-                 }
-         }
-
-         _model->ContentsChanged(); /* EMIT SIGNAL */
- }
-
- XMLNode&
- MidiModel::DiffCommand::marshal_note(const NotePtr note)
- {
-         XMLNode* xml_note = new XMLNode("note");
-
-         cerr << "Marshalling note: " << *note << endl;
-
-         ostringstream note_str(ios::ate);
-         note_str << int(note->note());
-         xml_note->add_property("note", note_str.str());
-
-         ostringstream channel_str(ios::ate);
-         channel_str << int(note->channel());
-         xml_note->add_property("channel", channel_str.str());
-
-         ostringstream time_str(ios::ate);
-         time_str << note->time();
-         xml_note->add_property("time", time_str.str());
-
-         ostringstream length_str(ios::ate);
-         length_str << note->length();
-         xml_note->add_property("length", length_str.str());
-
-         ostringstream velocity_str(ios::ate);
-         velocity_str << (unsigned int) note->velocity();
-         xml_note->add_property("velocity", velocity_str.str());
-
-         return *xml_note;
- }
-
- Evoral::Sequence<MidiModel::TimeType>::NotePtr
- MidiModel::DiffCommand::unmarshal_note(XMLNode *xml_note)
- {
-         unsigned int note;
-         XMLProperty* prop;
-         unsigned int channel;
-         unsigned int time;
-         unsigned int length;
-         unsigned int velocity;
-
-         if ((prop = xml_note->property("note")) != 0) {
-                 istringstream note_str(prop->value());
-                 note_str >> note;
-         } else {
-                 warning << "note information missing note value" << endmsg;
-                 note = 127;
-         }
-
-         if ((prop = xml_note->property("channel")) != 0) {
-                 istringstream channel_str(prop->value());
-                 channel_str >> channel;
-         } else {
-                 warning << "note information missing channel" << endmsg;
-                 channel = 0;
-         }
-
-         if ((prop = xml_note->property("time")) != 0) {
-                 istringstream time_str(prop->value());
-                 time_str >> time;
-         } else {
-                 warning << "note information missing time" << endmsg;
-                 time = 0;
-         }
-
-         if ((prop = xml_note->property("length")) != 0) {
-                 istringstream length_str(prop->value());
-                 length_str >> length;
-         } else {
-                 warning << "note information missing length" << endmsg;
-                 length = 1;
-         }
-
-         if ((prop = xml_note->property("velocity")) != 0) {
-                 istringstream velocity_str(prop->value());
-                 velocity_str >> velocity;
-         } else {
-                 warning << "note information missing velocity" << endmsg;
-                 velocity = 127;
-         }
-
-         NotePtr note_ptr(new Evoral::Note<TimeType>(channel, time, length, note, velocity));
-
-         return note_ptr;
- }
-
- XMLNode&
- MidiModel::DiffCommand::marshal_change(const NoteChange& change)
- {
-         XMLNode* xml_change = new XMLNode("change");
-
-         /* first, the change itself */
-
-         xml_change->add_property ("property", enum_2_string (change.property));
-
-         {
-                 ostringstream old_value_str (ios::ate);
-                 if (change.property == StartTime || change.property == Length) {
-                         old_value_str << change.old_time;
-                 } else {
-                         old_value_str << (unsigned int) change.old_value;
-                 }
-                 xml_change->add_property ("old", old_value_str.str());
-         }
-
-         {
-                 ostringstream new_value_str (ios::ate);
-                 if (change.property == StartTime || change.property == Length) {
-                         new_value_str << change.new_time;
-                 } else {
-                         new_value_str << (unsigned int) change.new_value;
-                 }
-                 xml_change->add_property ("new", new_value_str.str());
-         }
-
-         /* now the rest of the note */
-
-         const SMFSource* smf = dynamic_cast<const SMFSource*> (_model->midi_source());
-
-         if (change.property != NoteNumber) {
-                 ostringstream note_str;
-                 note_str << int(change.note->note());
-                 xml_change->add_property("note", note_str.str());
-         }
-
-         if (change.property != Channel) {
-                 ostringstream channel_str;
-                 channel_str << int(change.note->channel());
-                 xml_change->add_property("channel", channel_str.str());
-         }
-
-         if (change.property != StartTime) {
-                 ostringstream time_str;
-                 if (smf) {
-                         time_str << smf->round_to_file_precision (change.note->time());
-                 } else {
-                         time_str << change.note->time();
-                 }
-                 xml_change->add_property("time", time_str.str());
-         }
-
-         if (change.property != Length) {
-                 ostringstream length_str;
-                 if (smf) {
-                         length_str << smf->round_to_file_precision (change.note->length());
-                 } else {
-                         length_str << change.note->length();
-                 }
-                 xml_change->add_property ("length", length_str.str());
-         }
-
-         if (change.property != Velocity) {
-                 ostringstream velocity_str;
-                 velocity_str << int (change.note->velocity());
-                 xml_change->add_property("velocity", velocity_str.str());
-         }
-
-         /* and now notes that were remove as a side-effect */
-
-         return *xml_change;
- }
-
- MidiModel::DiffCommand::NoteChange
- MidiModel::DiffCommand::unmarshal_change(XMLNode *xml_change)
- {
-         XMLProperty* prop;
-         NoteChange change;
-         unsigned int note;
-         unsigned int channel;
-         unsigned int velocity;
-         Evoral::MusicalTime time;
-         Evoral::MusicalTime length;
-
-         if ((prop = xml_change->property("property")) != 0) {
-                 change.property = (Property) string_2_enum (prop->value(), change.property);
-         } else {
-                 fatal << "!!!" << endmsg;
-                 /*NOTREACHED*/
-         }
-
-         if ((prop = xml_change->property ("old")) != 0) {
-                 istringstream old_str (prop->value());
-                 if (change.property == StartTime || change.property == Length) {
-                         old_str >> change.old_time;
-                 } else {
-                         int integer_value_so_that_istream_does_the_right_thing;
-                         old_str >> integer_value_so_that_istream_does_the_right_thing;
-                         change.old_value = integer_value_so_that_istream_does_the_right_thing;
-                 }
-         } else {
-                 fatal << "!!!" << endmsg;
-                 /*NOTREACHED*/
-         }
-
-         if ((prop = xml_change->property ("new")) != 0) {
-                 istringstream new_str (prop->value());
-                 if (change.property == StartTime || change.property == Length) {
-                         new_str >> change.new_time;
-                 } else {
-                         int integer_value_so_that_istream_does_the_right_thing;
-                         new_str >> integer_value_so_that_istream_does_the_right_thing;
-                         change.new_value = integer_value_so_that_istream_does_the_right_thing;
-                 }
-         } else {
-                 fatal << "!!!" << endmsg;
-                 /*NOTREACHED*/
-         }
-
-         if (change.property != NoteNumber) {
-                 if ((prop = xml_change->property("note")) != 0) {
-                         istringstream note_str(prop->value());
-                         note_str >> note;
-                 } else {
-                         warning << "note information missing note value" << endmsg;
-                         note = 127;
-                 }
-         } else {
-                 note = change.new_value;
-         }
-
-         if (change.property != Channel) {
-                 if ((prop = xml_change->property("channel")) != 0) {
-                         istringstream channel_str(prop->value());
-                         channel_str >> channel;
-                 } else {
-                         warning << "note information missing channel" << endmsg;
-                         channel = 0;
-                 }
-         } else {
-                 channel = change.new_value;
-         }
-
-         if (change.property != StartTime) {
-                 if ((prop = xml_change->property("time")) != 0) {
-                         istringstream time_str(prop->value());
-                         time_str >> time;
-                 } else {
-                         warning << "note information missing time" << endmsg;
-                         time = 0;
-                 }
-         } else {
-                 time = change.new_time;
-         }
-
-         if (change.property != Length) {
-                 if ((prop = xml_change->property("length")) != 0) {
-                         istringstream length_str(prop->value());
-                         length_str >> length;
-                 } else {
-                         warning << "note information missing length" << endmsg;
-                         length = 1;
-                 }
-         } else {
-                 length = change.new_time;
-         }
-
-         if (change.property != Velocity) {
-                 if ((prop = xml_change->property("velocity")) != 0) {
-                         istringstream velocity_str(prop->value());
-                         velocity_str >> velocity;
-                 } else {
-                         warning << "note information missing velocity" << endmsg;
-                         velocity = 127;
-                 }
-         } else {
-                 velocity = change.new_value;
-         }
-
-         /* we must point at the instance of the note that is actually in the model.
-            so go look for it ...
-         */
+void
+MidiModel::DiffCommand::change(const NotePtr note, Property prop,
+                               uint8_t new_value)
+{
+        NoteChange change;
+        
+        switch (prop) {
+        case NoteNumber:
+                if (new_value == note->note()) {
+                        return;
+                }
+                change.old_value = note->note();
+                break;
+        case Velocity:
+                if (new_value == note->velocity()) {
+                        return;
+                }
+                change.old_value = note->velocity();
+                break;
+        case Channel:
+                if (new_value == note->channel()) {
+                        return;
+                }
+                change.old_value = note->channel();
+                break;
+
+
+        case StartTime:
+                fatal << "MidiModel::DiffCommand::change() with integer argument called for start time" << endmsg;
+                /*NOTREACHED*/
+                break;
+        case Length:
+                fatal << "MidiModel::DiffCommand::change() with integer argument called for length" << endmsg;
+                /*NOTREACHED*/
+                break;
+        }
+
+        change.note = note;
+        change.property = prop;
+        change.new_value = new_value;
+
+        _changes.push_back (change);
+}
+
+void
+MidiModel::DiffCommand::change(const NotePtr note, Property prop,
+                               TimeType new_time)
+{
+        NoteChange change;
+
+        switch (prop) {
+        case NoteNumber:
+        case Channel:
+        case Velocity:
+                fatal << "MidiModel::DiffCommand::change() with time argument called for note, channel or velocity" << endmsg;
+                break;
+
+        case StartTime:
+                if (Evoral::musical_time_equal (note->time(), new_time)) {
+                        return;
+                }
+                change.old_time = note->time();
+                break;
+        case Length:
+                if (Evoral::musical_time_equal (note->length(), new_time)) {
+                        return;
+                }
+                change.old_time = note->length();
+                break;
+        }
+
+        change.note = note;
+        change.property = prop;
+        change.new_time = new_time;
+
+        _changes.push_back (change);
+}
 
-         NotePtr new_note (new Evoral::Note<TimeType> (channel, time, length, note, velocity));
+MidiModel::DiffCommand&
+MidiModel::DiffCommand::operator+= (const DiffCommand& other)
+{
+        if (this == &other) {
+                return *this;
+        }
 
-         change.note = _model->find_note (new_note);
+        if (_model != other._model) {
+                return *this;
+        }
 
-         if (!change.note) {
-                 warning << "MIDI note " << *new_note << " not found in model - programmers should investigate this" << endmsg;
-                 /* use the actual new note */
-                 change.note = new_note;
-         }
+        _added_notes.insert (_added_notes.end(), other._added_notes.begin(), other._added_notes.end());
+        _removed_notes.insert (_removed_notes.end(), other._removed_notes.begin(), other._removed_notes.end());
+        side_effect_removals.insert (other.side_effect_removals.begin(), other.side_effect_removals.end());
+        _changes.insert (_changes.end(), other._changes.begin(), other._changes.end());
 
-         return change;
- }
+        return *this;
+}
+
+void
+MidiModel::DiffCommand::operator()()
+{
+        {
+                MidiModel::WriteLock lock(_model->edit_lock());
+
+                for (NoteList::iterator i = _added_notes.begin(); i != _added_notes.end(); ++i) {
+                        if (!_model->add_note_unlocked(*i)) {
+                                /* failed to add it, so don't leave it in the removed list, to
+                                   avoid apparent errors on undo.
+                                */
+                                _removed_notes.remove (*i);
+                        }
+                }
+
+                for (NoteList::iterator i = _removed_notes.begin(); i != _removed_notes.end(); ++i) {
+                        _model->remove_note_unlocked(*i);
+                }
+
+                /* notes we modify in a way that requires remove-then-add to maintain ordering */
+                set<NotePtr> temporary_removals;
+
+                for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) {
+                        Property prop = i->property;
+                        switch (prop) {
+                        case NoteNumber:
+                                if (temporary_removals.find (i->note) == temporary_removals.end()) {
+                                        _model->remove_note_unlocked (i->note);
+                                        temporary_removals.insert (i->note);
+                                }
+                                i->note->set_note (i->new_value);
+                                break;
+
+                        case StartTime:
+                                if (temporary_removals.find (i->note) == temporary_removals.end()) {
+                                        _model->remove_note_unlocked (i->note);
+                                        temporary_removals.insert (i->note);
+
+                                }
+                                i->note->set_time (i->new_time);
+                                break;
+
+                        case Channel:
+                                if (temporary_removals.find (i->note) == temporary_removals.end()) {
+                                        _model->remove_note_unlocked (i->note);
+                                        temporary_removals.insert (i->note);
+                                }
+                                i->note->set_channel (i->new_value);
+                                break;
+
+                                /* no remove-then-add required for these properties, since we do not index them
+                                 */
 
- int
- MidiModel::DiffCommand::set_state(const XMLNode& diff_command, int /*version*/)
- {
-         if (diff_command.name() != string(DIFF_COMMAND_ELEMENT)) {
-                 return 1;
-         }
-
-         /* additions */
+                        case Velocity:
+                                i->note->set_velocity (i->new_value);
+                                break;
 
-         _added_notes.clear();
-         XMLNode* added_notes = diff_command.child(ADDED_NOTES_ELEMENT);
-         if (added_notes) {
-                 XMLNodeList notes = added_notes->children();
-                 transform(notes.begin(), notes.end(), back_inserter(_added_notes),
-                           boost::bind (&DiffCommand::unmarshal_note, this, _1));
-         }
+                        case Length:
+                                i->note->set_length (i->new_time);
+                                break;
 
+                        }
+                }
 
-         /* removals */
 
-         _removed_notes.clear();
-         XMLNode* removed_notes = diff_command.child(REMOVED_NOTES_ELEMENT);
-         if (removed_notes) {
-                 XMLNodeList notes = removed_notes->children();
-                 transform(notes.begin(), notes.end(), back_inserter(_removed_notes),
-                           boost::bind (&DiffCommand::unmarshal_note, this, _1));
-         }
+                for (set<NotePtr>::iterator i = temporary_removals.begin(); i != temporary_removals.end(); ++i) {
+                        DiffCommand side_effects (model(), "side effects");
+                        _model->add_note_unlocked (*i, &side_effects);
+                        *this += side_effects;
+                }
 
+                if (!side_effect_removals.empty()) {
+                        cerr << "SER: \n";
+                        for (set<NotePtr>::iterator i = side_effect_removals.begin(); i != side_effect_removals.end(); ++i) {
+                                cerr << "\t" << *i << ' ' << **i << endl;
+                        }
+                }
+        }
 
-         /* changes */
+        _model->ContentsChanged(); /* EMIT SIGNAL */
+}
 
-         _changes.clear();
+void
+MidiModel::DiffCommand::undo()
+{
+        {
+                MidiModel::WriteLock lock(_model->edit_lock());
+
+                for (NoteList::iterator i = _added_notes.begin(); i != _added_notes.end(); ++i) {
+                        _model->remove_note_unlocked(*i);
+                }
+
+                for (NoteList::iterator i = _removed_notes.begin(); i != _removed_notes.end(); ++i) {
+                        _model->add_note_unlocked(*i);
+                }
+
+                /* notes we modify in a way that requires remove-then-add to maintain ordering */
+                set<NotePtr> temporary_removals;
+
+                for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) {
+                        Property prop = i->property;
+                        switch (prop) {
+                        case NoteNumber:
+                                if (temporary_removals.find (i->note) == temporary_removals.end()) {
+                                        _model->remove_note_unlocked (i->note);
+                                        temporary_removals.insert (i->note);
+                                }
+                                i->note->set_note (i->old_value);
+                                break;
+                        case Velocity:
+                                i->note->set_velocity (i->old_value);
+                                break;
+                        case StartTime:
+                                if (temporary_removals.find (i->note) == temporary_removals.end()) {
+                                        _model->remove_note_unlocked (i->note);
+                                        temporary_removals.insert (i->note);
+                                }
+                                i->note->set_time (i->old_time);
+                                break;
+                        case Length:
+                                i->note->set_length (i->old_time);
+                                break;
+                        case Channel:
+                                if (temporary_removals.find (i->note) == temporary_removals.end()) {
+                                        _model->remove_note_unlocked (i->note);
+                                        temporary_removals.insert (i->note);
+                                }
+                                i->note->set_channel (i->old_value);
+                                break;
+                        }
+                }
+
+                for (set<NotePtr>::iterator i = temporary_removals.begin(); i != temporary_removals.end(); ++i) {
+                        _model->add_note_unlocked (*i);
+                }
+
+                /* finally add back notes that were removed by the "do". we don't care
+                   about side effects here since the model should be back to its original
+                   state once this is done.
+                */
+
+                for (set<NotePtr>::iterator i = side_effect_removals.begin(); i != side_effect_removals.end(); ++i) {
+                        _model->add_note_unlocked (*i);
+                }
+        }
+
+        _model->ContentsChanged(); /* EMIT SIGNAL */
+}
 
-         XMLNode* changed_notes = diff_command.child(DIFF_NOTES_ELEMENT);
+XMLNode&
+MidiModel::DiffCommand::marshal_note(const NotePtr note)
+{
+        XMLNode* xml_note = new XMLNode("note");
 
-         if (changed_notes) {
-                 XMLNodeList notes = changed_notes->children();
-                 transform (notes.begin(), notes.end(), back_inserter(_changes),
-                            boost::bind (&DiffCommand::unmarshal_change, this, _1));
+        cerr << "Marshalling note: " << *note << endl;
 
-         }
+        ostringstream note_str(ios::ate);
+        note_str << int(note->note());
+        xml_note->add_property("note", note_str.str());
 
-         /* side effect removals caused by changes */
+        ostringstream channel_str(ios::ate);
+        channel_str << int(note->channel());
+        xml_note->add_property("channel", channel_str.str());
 
-         side_effect_removals.clear();
+        ostringstream time_str(ios::ate);
+        time_str << note->time();
+        xml_note->add_property("time", time_str.str());
 
-         XMLNode* side_effect_notes = diff_command.child(SIDE_EFFECT_REMOVALS_ELEMENT);
+        ostringstream length_str(ios::ate);
+        length_str << note->length();
+        xml_note->add_property("length", length_str.str());
 
-         if (side_effect_notes) {
-                 XMLNodeList notes = side_effect_notes->children();
-                 for (XMLNodeList::iterator n = notes.begin(); n != notes.end(); ++n) {
-                         side_effect_removals.insert (unmarshal_note (*n));
-                 }
-         }
+        ostringstream velocity_str(ios::ate);
+        velocity_str << (unsigned int) note->velocity();
+        xml_note->add_property("velocity", velocity_str.str());
 
-         return 0;
- }
+        return *xml_note;
+}
 
- XMLNode&
- MidiModel::DiffCommand::get_state ()
- {
-         XMLNode* diff_command = new XMLNode(DIFF_COMMAND_ELEMENT);
-         diff_command->add_property("midi-source", _model->midi_source()->id().to_s());
+Evoral::Sequence<MidiModel::TimeType>::NotePtr
+MidiModel::DiffCommand::unmarshal_note(XMLNode *xml_note)
+{
+        unsigned int note;
+        XMLProperty* prop;
+        unsigned int channel;
+        unsigned int time;
+        unsigned int length;
+        unsigned int velocity;
+
+        if ((prop = xml_note->property("note")) != 0) {
+                istringstream note_str(prop->value());
+                note_str >> note;
+        } else {
+                warning << "note information missing note value" << endmsg;
+                note = 127;
+        }
+
+        if ((prop = xml_note->property("channel")) != 0) {
+                istringstream channel_str(prop->value());
+                channel_str >> channel;
+        } else {
+                warning << "note information missing channel" << endmsg;
+                channel = 0;
+        }
+
+        if ((prop = xml_note->property("time")) != 0) {
+                istringstream time_str(prop->value());
+                time_str >> time;
+        } else {
+                warning << "note information missing time" << endmsg;
+                time = 0;
+        }
+
+        if ((prop = xml_note->property("length")) != 0) {
+                istringstream length_str(prop->value());
+                length_str >> length;
+        } else {
+                warning << "note information missing length" << endmsg;
+                length = 1;
+        }
+
+        if ((prop = xml_note->property("velocity")) != 0) {
+                istringstream velocity_str(prop->value());
+                velocity_str >> velocity;
+        } else {
+                warning << "note information missing velocity" << endmsg;
+                velocity = 127;
+        }
+
+        NotePtr note_ptr(new Evoral::Note<TimeType>(channel, time, length, note, velocity));
+
+        return note_ptr;
+}
 
-         XMLNode* changes = diff_command->add_child(DIFF_NOTES_ELEMENT);
-         for_each(_changes.begin(), _changes.end(), 
-                  boost::bind (
-                          boost::bind (&XMLNode::add_child_nocopy, changes, _1),
-                          boost::bind (&DiffCommand::marshal_change, this, _1)));
+XMLNode&
+MidiModel::DiffCommand::marshal_change(const NoteChange& change)
+{
+        XMLNode* xml_change = new XMLNode("change");
+
+        /* first, the change itself */
+
+        xml_change->add_property ("property", enum_2_string (change.property));
+
+        {
+                ostringstream old_value_str (ios::ate);
+                if (change.property == StartTime || change.property == Length) {
+                        old_value_str << change.old_time;
+                } else {
+                        old_value_str << (unsigned int) change.old_value;
+                }
+                xml_change->add_property ("old", old_value_str.str());
+        }
+
+        {
+                ostringstream new_value_str (ios::ate);
+                if (change.property == StartTime || change.property == Length) {
+                        new_value_str << change.new_time;
+                } else {
+                        new_value_str << (unsigned int) change.new_value;
+                }
+                xml_change->add_property ("new", new_value_str.str());
+        }
+
+        /* now the rest of the note */
+
+        const SMFSource* smf = dynamic_cast<const SMFSource*> (_model->midi_source());
+
+        if (change.property != NoteNumber) {
+                ostringstream note_str;
+                note_str << int(change.note->note());
+                xml_change->add_property("note", note_str.str());
+        }
+
+        if (change.property != Channel) {
+                ostringstream channel_str;
+                channel_str << int(change.note->channel());
+                xml_change->add_property("channel", channel_str.str());
+        }
+
+        if (change.property != StartTime) {
+                ostringstream time_str;
+                if (smf) {
+                        time_str << smf->round_to_file_precision (change.note->time());
+                } else {
+                        time_str << change.note->time();
+                }
+                xml_change->add_property("time", time_str.str());
+        }
+
+        if (change.property != Length) {
+                ostringstream length_str;
+                if (smf) {
+                        length_str << smf->round_to_file_precision (change.note->length());
+                } else {
+                        length_str << change.note->length();
+                }
+                xml_change->add_property ("length", length_str.str());
+        }
+
+        if (change.property != Velocity) {
+                ostringstream velocity_str;
+                velocity_str << int (change.note->velocity());
+                xml_change->add_property("velocity", velocity_str.str());
+        }
+
+        /* and now notes that were remove as a side-effect */
+
+        return *xml_change;
+}
 
-         XMLNode* added_notes = diff_command->add_child(ADDED_NOTES_ELEMENT);
-         for_each(_added_notes.begin(), _added_notes.end(), 
-                  boost::bind(
-                          boost::bind (&XMLNode::add_child_nocopy, added_notes, _1),
-                          boost::bind (&DiffCommand::marshal_note, this, _1)));
+MidiModel::DiffCommand::NoteChange
+MidiModel::DiffCommand::unmarshal_change(XMLNode *xml_change)
+{
+        XMLProperty* prop;
+        NoteChange change;
+        unsigned int note;
+        unsigned int channel;
+        unsigned int velocity;
+        Evoral::MusicalTime time;
+        Evoral::MusicalTime length;
+
+        if ((prop = xml_change->property("property")) != 0) {
+                change.property = (Property) string_2_enum (prop->value(), change.property);
+        } else {
+                fatal << "!!!" << endmsg;
+                /*NOTREACHED*/
+        }
+
+        if ((prop = xml_change->property ("old")) != 0) {
+                istringstream old_str (prop->value());
+                if (change.property == StartTime || change.property == Length) {
+                        old_str >> change.old_time;
+                } else {
+                        int integer_value_so_that_istream_does_the_right_thing;
+                        old_str >> integer_value_so_that_istream_does_the_right_thing;
+                        change.old_value = integer_value_so_that_istream_does_the_right_thing;
+                }
+        } else {
+                fatal << "!!!" << endmsg;
+                /*NOTREACHED*/
+        }
+
+        if ((prop = xml_change->property ("new")) != 0) {
+                istringstream new_str (prop->value());
+                if (change.property == StartTime || change.property == Length) {
+                        new_str >> change.new_time;
+                } else {
+                        int integer_value_so_that_istream_does_the_right_thing;
+                        new_str >> integer_value_so_that_istream_does_the_right_thing;
+                        change.new_value = integer_value_so_that_istream_does_the_right_thing;
+                }
+        } else {
+                fatal << "!!!" << endmsg;
+                /*NOTREACHED*/
+        }
+
+        if (change.property != NoteNumber) {
+                if ((prop = xml_change->property("note")) != 0) {
+                        istringstream note_str(prop->value());
+                        note_str >> note;
+                } else {
+                        warning << "note information missing note value" << endmsg;
+                        note = 127;
+                }
+        } else {
+                note = change.new_value;
+        }
+
+        if (change.property != Channel) {
+                if ((prop = xml_change->property("channel")) != 0) {
+                        istringstream channel_str(prop->value());
+                        channel_str >> channel;
+                } else {
+                        warning << "note information missing channel" << endmsg;
+                        channel = 0;
+                }
+        } else {
+                channel = change.new_value;
+        }
+
+        if (change.property != StartTime) {
+                if ((prop = xml_change->property("time")) != 0) {
+                        istringstream time_str(prop->value());
+                        time_str >> time;
+                } else {
+                        warning << "note information missing time" << endmsg;
+                        time = 0;
+                }
+        } else {
+                time = change.new_time;
+        }
+
+        if (change.property != Length) {
+                if ((prop = xml_change->property("length")) != 0) {
+                        istringstream length_str(prop->value());
+                        length_str >> length;
+                } else {
+                        warning << "note information missing length" << endmsg;
+                        length = 1;
+                }
+        } else {
+                length = change.new_time;
+        }
+
+        if (change.property != Velocity) {
+                if ((prop = xml_change->property("velocity")) != 0) {
+                        istringstream velocity_str(prop->value());
+                        velocity_str >> velocity;
+                } else {
+                        warning << "note information missing velocity" << endmsg;
+                        velocity = 127;
+                }
+        } else {
+                velocity = change.new_value;
+        }
+
+        /* we must point at the instance of the note that is actually in the model.
+           so go look for it ...
+        */
+
+        NotePtr new_note (new Evoral::Note<TimeType> (channel, time, length, note, velocity));
+
+        change.note = _model->find_note (new_note);
+
+        if (!change.note) {
+                warning << "MIDI note " << *new_note << " not found in model - programmers should investigate this" << endmsg;
+                /* use the actual new note */
+                change.note = new_note;
+        }
+
+        return change;
+}
 
-         XMLNode* removed_notes = diff_command->add_child(REMOVED_NOTES_ELEMENT);
-         for_each(_removed_notes.begin(), _removed_notes.end(), 
-                  boost::bind (
-                          boost::bind (&XMLNode::add_child_nocopy, removed_notes, _1),
-                          boost::bind (&DiffCommand::marshal_note, this, _1)));
+int
+MidiModel::DiffCommand::set_state(const XMLNode& diff_command, int /*version*/)
+{
+        if (diff_command.name() != string(DIFF_COMMAND_ELEMENT)) {
+                return 1;
+        }
 
-         /* if this command had side-effects, store that state too 
-          */
+        /* additions */
 
-         if (!side_effect_removals.empty()) {
-                 XMLNode* side_effect_notes = diff_command->add_child(SIDE_EFFECT_REMOVALS_ELEMENT);
-                 for_each(side_effect_removals.begin(), side_effect_removals.end(), 
-                          boost::bind (
-                                  boost::bind (&XMLNode::add_child_nocopy, side_effect_notes, _1),
-                                  boost::bind (&DiffCommand::marshal_note, this, _1)));
-         }
-
-         return *diff_command;
- }
+        _added_notes.clear();
+        XMLNode* added_notes = diff_command.child(ADDED_NOTES_ELEMENT);
+        if (added_notes) {
+                XMLNodeList notes = added_notes->children();
+                transform(notes.begin(), notes.end(), back_inserter(_added_notes),
+                          boost::bind (&DiffCommand::unmarshal_note, this, _1));
+        }
 
- /** Write all of the model to a MidiSource (i.e. save the model).
-  * This is different from manually using read to write to a source in that
-  * note off events are written regardless of the track mode.  This is so the
-  * user can switch a recorded track (with note durations from some instrument)
-  * to percussive, save, reload, then switch it back to sustained without
-  * destroying the original note durations.
-  */
- bool
- MidiModel::write_to (boost::shared_ptr<MidiSource> source)
- {
-         ReadLock lock(read_lock());
 
-         const bool old_percussive = percussive();
-         set_percussive(false);
-
-         source->drop_model();
-         source->mark_streaming_midi_write_started(note_mode(), _midi_source->timeline_position());
+        /* removals */
 
-         for (Evoral::Sequence<TimeType>::const_iterator i = begin(); i != end(); ++i) {
-                 source->append_event_unlocked_beats(*i);
-         }
+        _removed_notes.clear();
+        XMLNode* removed_notes = diff_command.child(REMOVED_NOTES_ELEMENT);
+        if (removed_notes) {
+                XMLNodeList notes = removed_notes->children();
+                transform(notes.begin(), notes.end(), back_inserter(_removed_notes),
+                          boost::bind (&DiffCommand::unmarshal_note, this, _1));
+        }
 
-         set_percussive(old_percussive);
-         source->mark_streaming_write_completed();
-
-         set_edited(false);
+
+        /* changes */
+
+        _changes.clear();
+
+        XMLNode* changed_notes = diff_command.child(DIFF_NOTES_ELEMENT);
+
+        if (changed_notes) {
+                XMLNodeList notes = changed_notes->children();
+                transform (notes.begin(), notes.end(), back_inserter(_changes),
+                           boost::bind (&DiffCommand::unmarshal_change, this, _1));
+
+        }
+
+        /* side effect removals caused by changes */
+
+        side_effect_removals.clear();
+
+        XMLNode* side_effect_notes = diff_command.child(SIDE_EFFECT_REMOVALS_ELEMENT);
+
+        if (side_effect_notes) {
+                XMLNodeList notes = side_effect_notes->children();
+                for (XMLNodeList::iterator n = notes.begin(); n != notes.end(); ++n) {
+                        side_effect_removals.insert (unmarshal_note (*n));
+                }
+        }
+
+        return 0;
+}
+
+XMLNode&
+MidiModel::DiffCommand::get_state ()
+{
+        XMLNode* diff_command = new XMLNode(DIFF_COMMAND_ELEMENT);
+        diff_command->add_property("midi-source", _model->midi_source()->id().to_s());
+
+        XMLNode* changes = diff_command->add_child(DIFF_NOTES_ELEMENT);
+        for_each(_changes.begin(), _changes.end(), 
+                 boost::bind (
+                         boost::bind (&XMLNode::add_child_nocopy, changes, _1),
+                         boost::bind (&DiffCommand::marshal_change, this, _1)));
+
+        XMLNode* added_notes = diff_command->add_child(ADDED_NOTES_ELEMENT);
+        for_each(_added_notes.begin(), _added_notes.end(), 
+                 boost::bind(
+                         boost::bind (&XMLNode::add_child_nocopy, added_notes, _1),
+                         boost::bind (&DiffCommand::marshal_note, this, _1)));
+
+        XMLNode* removed_notes = diff_command->add_child(REMOVED_NOTES_ELEMENT);
+        for_each(_removed_notes.begin(), _removed_notes.end(), 
+                 boost::bind (
+                         boost::bind (&XMLNode::add_child_nocopy, removed_notes, _1),
+                         boost::bind (&DiffCommand::marshal_note, this, _1)));
+
+        /* if this command had side-effects, store that state too 
+         */
+
+        if (!side_effect_removals.empty()) {
+                XMLNode* side_effect_notes = diff_command->add_child(SIDE_EFFECT_REMOVALS_ELEMENT);
+                for_each(side_effect_removals.begin(), side_effect_removals.end(), 
+                         boost::bind (
+                                 boost::bind (&XMLNode::add_child_nocopy, side_effect_notes, _1),
+                                 boost::bind (&DiffCommand::marshal_note, this, _1)));
+        }
+
+        return *diff_command;
+}
+
+
+/** Write all of the model to a MidiSource (i.e. save the model).
+ * This is different from manually using read to write to a source in that
+ * note off events are written regardless of the track mode.  This is so the
+ * user can switch a recorded track (with note durations from some instrument)
+ * to percussive, save, reload, then switch it back to sustained without
+ * destroying the original note durations.
+ */
+bool
+MidiModel::write_to (boost::shared_ptr<MidiSource> source)
+{
+        ReadLock lock(read_lock());
+
+        const bool old_percussive = percussive();
+        set_percussive(false);
+
+        source->drop_model();
+        source->mark_streaming_midi_write_started(note_mode(), _midi_source->timeline_position());
+
+        for (Evoral::Sequence<TimeType>::const_iterator i = begin(); i != end(); ++i) {
+                source->append_event_unlocked_beats(*i);
+        }
+
+        set_percussive(old_percussive);
+        source->mark_streaming_write_completed();
+
+        set_edited(false);
         
-         return true;
- }
-
- /** Write part or all of the model to a MidiSource (i.e. save the model).
-  * This is different from manually using read to write to a source in that
-  * note off events are written regardless of the track mode.  This is so the
-  * user can switch a recorded track (with note durations from some instrument)
-  * to percussive, save, reload, then switch it back to sustained without
-  * destroying the original note durations.
-  */
- bool
- MidiModel::write_section_to (boost::shared_ptr<MidiSource> source, Evoral::MusicalTime begin_time, Evoral::MusicalTime end_time)
- {
-         ReadLock lock(read_lock());
-         MidiStateTracker mst;
-         Evoral::MusicalTime extra_note_on_time = end_time;
-
-         const bool old_percussive = percussive();
-         set_percussive(false);
-
-         source->drop_model();
-         source->mark_streaming_midi_write_started(note_mode(), _midi_source->timeline_position());
-
-         for (Evoral::Sequence<TimeType>::const_iterator i = begin(); i != end(); ++i) {
-                 const Evoral::Event<Evoral::MusicalTime>& ev (*i);
-
-                 if (ev.time() >= begin_time && ev.time() < end_time) {
-
-                         const Evoral::MIDIEvent<Evoral::MusicalTime>* mev = 
-                                 static_cast<const Evoral::MIDIEvent<Evoral::MusicalTime>* > (&ev);
-
-                         if (!mev) {
-                                 continue;
-                         }
-
-
-                         if (mev->is_note_off()) {
-
-                                 if (!mst.active (mev->note(), mev->channel())) {
-
-                                         /* add a note-on at the start of the range we're writing
-                                            to the file. velocity is just an arbitary reasonable value.
-                                         */
-
-                                         Evoral::MIDIEvent<Evoral::MusicalTime> on (mev->event_type(), extra_note_on_time, 3, 0, true);
-                                         on.set_type (mev->type());
-                                         on.set_note (mev->note());
-                                         on.set_channel (mev->channel());
-                                         on.set_velocity (mev->velocity());
-
-                                         cerr << "Add note on for odd note off, note = " << (int) on.note() << endl;
-                                         source->append_event_unlocked_beats (on);
-                                         mst.add (on.note(), on.channel());
-                                         mst.dump (cerr);
-                                         extra_note_on_time += 1.0/128.0;
-                                 }
-
-                                 cerr << "MIDI Note off (note = " << (int) mev->note() << endl;
-                                 source->append_event_unlocked_beats (*i);
-                                 mst.remove (mev->note(), mev->channel());
-                                 mst.dump (cerr);
-
-                         } else if (mev->is_note_on()) {
-                                 cerr << "MIDI Note on (note = " << (int) mev->note() << endl;
-                                 mst.add (mev->note(), mev->channel());
-                                 source->append_event_unlocked_beats(*i);
-                                 mst.dump (cerr);
-                         } else {
-                                 cerr << "MIDI other event type\n";
-                                 source->append_event_unlocked_beats(*i);
-                         }
-                 }
-         }
-
-         mst.resolve_notes (*source, end_time);
-
-         set_percussive(old_percussive);
-         source->mark_streaming_write_completed();
-
-         set_edited(false);
-
-         return true;
- }
-
- XMLNode&
- MidiModel::get_state()
- {
-         XMLNode *node = new XMLNode("MidiModel");
-         return *node;
- }
-
- Evoral::Sequence<MidiModel::TimeType>::NotePtr
- MidiModel::find_note (NotePtr other)
- {
-         Notes::iterator l = notes().lower_bound(other);
-
-         if (l != notes().end()) {
-                 for (; (*l)->time() == other->time(); ++l) {
-                         /* NB: compare note contents, not note pointers.
-                            If "other" was a ptr to a note already in
-                            the model, we wouldn't be looking for it,
-                            would we now?
-                          */
-                         if (**l == *other) {
-                                 return *l;
-                         }
-                 }
-         }
-
-         return NotePtr();
- }
-
- /** Lock and invalidate the source.
-  * This should be used by commands and editing things
-  */
- MidiModel::WriteLock
- MidiModel::edit_lock()
- {
-         Glib::Mutex::Lock* source_lock = new Glib::Mutex::Lock(_midi_source->mutex());
-         _midi_source->invalidate(); // Release cached iterator's read lock on model
-         return WriteLock(new WriteLockImpl(source_lock, _lock, _control_lock));
- }
-
- /** Lock just the model, the source lock must already be held.
-  * This should only be called from libardour/evoral places
-  */
- MidiModel::WriteLock
- MidiModel::write_lock()
- {
-         assert(!_midi_source->mutex().trylock());
-         return WriteLock(new WriteLockImpl(NULL, _lock, _control_lock));
- }
-
- int
- MidiModel::resolve_overlaps_unlocked (const NotePtr note, void* arg)
- {
-         using namespace Evoral;
-
-         if (_writing || insert_merge_policy() == InsertMergeRelax) {
-                 return 0;
-         }
-
-         DiffCommand* cmd = static_cast<DiffCommand*>(arg);
-
-         TimeType sa = note->time();
-         TimeType ea  = note->end_time();
-
-         const Pitches& p (pitches (note->channel()));
-         NotePtr search_note(new Note<TimeType>(0, 0, 0, note->note()));
-         set<NotePtr> to_be_deleted;
-         bool set_note_length = false;
-         bool set_note_time = false;
-         TimeType note_time = note->time();
-         TimeType note_length = note->length();
-
-         for (Pitches::const_iterator i = p.lower_bound (search_note); 
-              i != p.end() && (*i)->note() == note->note(); ++i) {
-
-                 TimeType sb = (*i)->time();
-                 TimeType eb = (*i)->end_time();
-                 OverlapType overlap = OverlapNone;
-
-                 if ((sb > sa) && (eb <= ea)) {
-                         overlap = OverlapInternal;
-                 } else if ((eb >= sa) && (eb <= ea)) {
-                         overlap = OverlapStart;
-                 } else if ((sb > sa) && (sb <= ea)) {
-                         overlap = OverlapEnd;
-                 } else if ((sa >= sb) && (sa <= eb) && (ea <= eb)) {
-                         overlap = OverlapExternal;
-                 } else {
-                         /* no overlap */
-                         continue;
-                 }
-
-                 if (insert_merge_policy() == InsertMergeReject) {
-                         return -1;
-                 }
-
-                 switch (overlap) {
-                 case OverlapStart:
-                         cerr << "OverlapStart\n";
-                         /* existing note covers start of new note */
-                         switch (insert_merge_policy()) {
-                         case InsertMergeReplace:
-                                 to_be_deleted.insert (*i);
-                                 break;
-                         case InsertMergeTruncateExisting:
-                                 if (cmd) {
-                                         cmd->change (*i, DiffCommand::Length, (note->time() - (*i)->time()));
-                                 }
-                                 (*i)->set_length (note->time() - (*i)->time());
-                                 break;
-                         case InsertMergeTruncateAddition:
-                                 set_note_time = true;
-                                 set_note_length = true;
-                                 note_time = (*i)->time() + (*i)->length();
-                                 note_length = min (note_length, (*i)->length() - ((*i)->end_time() - note->time()));
-                                 break;
-                         case InsertMergeExtend:
-                                 if (cmd) {
-                                         cmd->change ((*i), DiffCommand::Length, note->end_time() - (*i)->time());
-                                 } 
-                                 (*i)->set_length (note->end_time() - (*i)->time());
-                                 return -1; /* do not add the new note */
-                                 break;
-                         default:
-                                 /*NOTREACHED*/
-                                 /* stupid gcc */
-                                 break;
-                         }
-                         break;
-
-                 case OverlapEnd:
-                         cerr << "OverlapEnd\n";
-                         /* existing note covers end of new note */
-                         switch (insert_merge_policy()) {
-                         case InsertMergeReplace:
-                                 to_be_deleted.insert (*i);
-                                 break;
-
-                         case InsertMergeTruncateExisting:
-                                 /* resetting the start time of the existing note
-                                    is a problem because of time ordering.
-                                 */
-                                 break;
-
-                         case InsertMergeTruncateAddition:
-                                 set_note_length = true;
-                                 note_length = min (note_length, ((*i)->time() - note->time()));
-                                 break;
-
-                         case InsertMergeExtend:
-                                 /* we can't reset the time of the existing note because
-                                    that will corrupt time ordering. So remove the
-                                    existing note and change the position/length
-                                    of the new note (which has not been added yet)
-                                 */
-                                 to_be_deleted.insert (*i);
-                                 set_note_length = true;
-                                 note_length = min (note_length, (*i)->end_time() - note->time());
-                                 break;
-                         default:
-                                 /*NOTREACHED*/
-                                 /* stupid gcc */
-                                 break;
-                         }
-                         break;
-
-                 case OverlapExternal:
-                         cerr << "OverlapExt\n";
-                         /* existing note overlaps all the new note */
-                         switch (insert_merge_policy()) {
-                         case InsertMergeReplace:
-                                 to_be_deleted.insert (*i);
-                                 break;
-                         case InsertMergeTruncateExisting:
-                         case InsertMergeTruncateAddition:
-                         case InsertMergeExtend:
-                                 /* cannot add in this case */
-                                 return -1;
-                         default:
-                                 /*NOTREACHED*/
-                                 /* stupid gcc */
-                                 break;
-                         }
-                         break;
-
-                 case OverlapInternal:
-                         cerr << "OverlapInt\n";
-                         /* new note fully overlaps an existing note */
-                         switch (insert_merge_policy()) {
-                         case InsertMergeReplace:
-                         case InsertMergeTruncateExisting:
-                         case InsertMergeTruncateAddition:
-                         case InsertMergeExtend:
-                                 /* delete the existing note, the new one will cover it */
-                                 to_be_deleted.insert (*i);
-                                 break;
-                         default:
-                                 /*NOTREACHED*/
-                                 /* stupid gcc */
-                                 break;
-                         }
-                         break;
-
-                 default:
-                         /*NOTREACHED*/
-                         /* stupid gcc */
-                         break;
-                 }
-         }
-
-         for (set<NotePtr>::iterator i = to_be_deleted.begin(); i != to_be_deleted.end(); ++i) {
-                 remove_note_unlocked (*i);
-
-                 if (cmd) {
-                         cmd->side_effect_remove (*i);
-                 }
-         }
-
-         if (set_note_time) {
-                 if (cmd) {
-                         cmd->change (note, DiffCommand::StartTime, note_time);
-                 } 
-                 note->set_time (note_time);
-         }
-
-         if (set_note_length) {
-                 if (cmd) {
-                         cmd->change (note, DiffCommand::Length, note_length);
-                 } 
-                 note->set_length (note_length);
-         }
-
-         return 0;
- }
-
- InsertMergePolicy
- MidiModel::insert_merge_policy () const 
- {
-         /* XXX ultimately this should be a per-track or even per-model policy */
-
-         return _midi_source->session().config.get_insert_merge_policy();
+        return true;
+}
+
+/** very similar to ::write_to() but writes to the model's own
+    existing midi_source, without making it call MidiSource::drop_model().
+    the caller is a MidiSource that needs to catch up with the state
+    of the model.
+*/
+bool
+MidiModel::sync_to_source ()
+{
+        ReadLock lock(read_lock());
+
+        const bool old_percussive = percussive();
+        set_percussive(false);
+
+        _midi_source->mark_streaming_midi_write_started(note_mode(), _midi_source->timeline_position());
+
+        for (Evoral::Sequence<TimeType>::const_iterator i = begin(); i != end(); ++i) {
+                _midi_source->append_event_unlocked_beats(*i);
+        }
+
+        set_percussive (old_percussive);
+        _midi_source->mark_streaming_write_completed ();
+
+        set_edited (false);
+        
+        return true;
+}
+
+/** Write part or all of the model to a MidiSource (i.e. save the model).
+ * This is different from manually using read to write to a source in that
+ * note off events are written regardless of the track mode.  This is so the
+ * user can switch a recorded track (with note durations from some instrument)
+ * to percussive, save, reload, then switch it back to sustained without
+ * destroying the original note durations.
+ */
+bool
+MidiModel::write_section_to (boost::shared_ptr<MidiSource> source, Evoral::MusicalTime begin_time, Evoral::MusicalTime end_time)
+{
+        ReadLock lock(read_lock());
+        MidiStateTracker mst;
+        Evoral::MusicalTime extra_note_on_time = end_time;
+
+        const bool old_percussive = percussive();
+        set_percussive(false);
+
+        source->drop_model();
+        source->mark_streaming_midi_write_started(note_mode(), _midi_source->timeline_position());
+
+        for (Evoral::Sequence<TimeType>::const_iterator i = begin(); i != end(); ++i) {
+                const Evoral::Event<Evoral::MusicalTime>& ev (*i);
+
+                if (ev.time() >= begin_time && ev.time() < end_time) {
+
+                        const Evoral::MIDIEvent<Evoral::MusicalTime>* mev = 
+                                static_cast<const Evoral::MIDIEvent<Evoral::MusicalTime>* > (&ev);
+
+                        if (!mev) {
+                                continue;
+                        }
+
+
+                        if (mev->is_note_off()) {
+
+                                if (!mst.active (mev->note(), mev->channel())) {
+
+                                        /* add a note-on at the start of the range we're writing
+                                           to the file. velocity is just an arbitary reasonable value.
+                                        */
+
+                                        Evoral::MIDIEvent<Evoral::MusicalTime> on (mev->event_type(), extra_note_on_time, 3, 0, true);
+                                        on.set_type (mev->type());
+                                        on.set_note (mev->note());
+                                        on.set_channel (mev->channel());
+                                        on.set_velocity (mev->velocity());
+
+                                        cerr << "Add note on for odd note off, note = " << (int) on.note() << endl;
+                                        source->append_event_unlocked_beats (on);
+                                        mst.add (on.note(), on.channel());
+                                        mst.dump (cerr);
+                                        extra_note_on_time += 1.0/128.0;
+                                }
+
+                                cerr << "MIDI Note off (note = " << (int) mev->note() << endl;
+                                source->append_event_unlocked_beats (*i);
+                                mst.remove (mev->note(), mev->channel());
+                                mst.dump (cerr);
+
+                        } else if (mev->is_note_on()) {
+                                cerr << "MIDI Note on (note = " << (int) mev->note() << endl;
+                                mst.add (mev->note(), mev->channel());
+                                source->append_event_unlocked_beats(*i);
+                                mst.dump (cerr);
+                        } else {
+                                cerr << "MIDI other event type\n";
+                                source->append_event_unlocked_beats(*i);
+                        }
+                }
+        }
+
+        mst.resolve_notes (*source, end_time);
+
+        set_percussive(old_percussive);
+        source->mark_streaming_write_completed();
+
+        set_edited(false);
+
+        return true;
+}
+
+XMLNode&
+MidiModel::get_state()
+{
+        XMLNode *node = new XMLNode("MidiModel");
+        return *node;
+}
+
+Evoral::Sequence<MidiModel::TimeType>::NotePtr
+MidiModel::find_note (NotePtr other)
+{
+        Notes::iterator l = notes().lower_bound(other);
+
+        if (l != notes().end()) {
+                for (; (*l)->time() == other->time(); ++l) {
+                        /* NB: compare note contents, not note pointers.
+                           If "other" was a ptr to a note already in
+                           the model, we wouldn't be looking for it,
+                           would we now?
+                        */
+                        if (**l == *other) {
+                                return *l;
+                        }
+                }
+        }
+
+        return NotePtr();
+}
+
+/** Lock and invalidate the source.
+ * This should be used by commands and editing things
+ */
+MidiModel::WriteLock
+MidiModel::edit_lock()
+{
+        Glib::Mutex::Lock* source_lock = new Glib::Mutex::Lock(_midi_source->mutex());
+        _midi_source->invalidate(); // Release cached iterator's read lock on model
+        return WriteLock(new WriteLockImpl(source_lock, _lock, _control_lock));
+}
+
+/** Lock just the model, the source lock must already be held.
+ * This should only be called from libardour/evoral places
+ */
+MidiModel::WriteLock
+MidiModel::write_lock()
+{
+        assert(!_midi_source->mutex().trylock());
+        return WriteLock(new WriteLockImpl(NULL, _lock, _control_lock));
+}
+
+int
+MidiModel::resolve_overlaps_unlocked (const NotePtr note, void* arg)
+{
+        using namespace Evoral;
+
+        if (_writing || insert_merge_policy() == InsertMergeRelax) {
+                return 0;
+        }
+
+        DiffCommand* cmd = static_cast<DiffCommand*>(arg);
+
+        TimeType sa = note->time();
+        TimeType ea  = note->end_time();
+
+        const Pitches& p (pitches (note->channel()));
+        NotePtr search_note(new Note<TimeType>(0, 0, 0, note->note()));
+        set<NotePtr> to_be_deleted;
+        bool set_note_length = false;
+        bool set_note_time = false;
+        TimeType note_time = note->time();
+        TimeType note_length = note->length();
+
+        for (Pitches::const_iterator i = p.lower_bound (search_note); 
+             i != p.end() && (*i)->note() == note->note(); ++i) {
+
+                TimeType sb = (*i)->time();
+                TimeType eb = (*i)->end_time();
+                OverlapType overlap = OverlapNone;
+
+                if ((sb > sa) && (eb <= ea)) {
+                        overlap = OverlapInternal;
+                } else if ((eb >= sa) && (eb <= ea)) {
+                        overlap = OverlapStart;
+                } else if ((sb > sa) && (sb <= ea)) {
+                        overlap = OverlapEnd;
+                } else if ((sa >= sb) && (sa <= eb) && (ea <= eb)) {
+                        overlap = OverlapExternal;
+                } else {
+                        /* no overlap */
+                        continue;
+                }
+
+                if (insert_merge_policy() == InsertMergeReject) {
+                        return -1;
+                }
+
+                switch (overlap) {
+                case OverlapStart:
+                        cerr << "OverlapStart\n";
+                        /* existing note covers start of new note */
+                        switch (insert_merge_policy()) {
+                        case InsertMergeReplace:
+                                to_be_deleted.insert (*i);
+                                break;
+                        case InsertMergeTruncateExisting:
+                                if (cmd) {
+                                        cmd->change (*i, DiffCommand::Length, (note->time() - (*i)->time()));
+                                }
+                                (*i)->set_length (note->time() - (*i)->time());
+                                break;
+                        case InsertMergeTruncateAddition:
+                                set_note_time = true;
+                                set_note_length = true;
+                                note_time = (*i)->time() + (*i)->length();
+                                note_length = min (note_length, (*i)->length() - ((*i)->end_time() - note->time()));
+                                break;
+                        case InsertMergeExtend:
+                                if (cmd) {
+                                        cmd->change ((*i), DiffCommand::Length, note->end_time() - (*i)->time());
+                                } 
+                                (*i)->set_length (note->end_time() - (*i)->time());
+                                return -1; /* do not add the new note */
+                                break;
+                        default:
+                                /*NOTREACHED*/
+                                /* stupid gcc */
+                                break;
+                        }
+                        break;
+
+                case OverlapEnd:
+                        cerr << "OverlapEnd\n";
+                        /* existing note covers end of new note */
+                        switch (insert_merge_policy()) {
+                        case InsertMergeReplace:
+                                to_be_deleted.insert (*i);
+                                break;
+
+                        case InsertMergeTruncateExisting:
+                                /* resetting the start time of the existing note
+                                   is a problem because of time ordering.
+                                */
+                                break;
+
+                        case InsertMergeTruncateAddition:
+                                set_note_length = true;
+                                note_length = min (note_length, ((*i)->time() - note->time()));
+                                break;
+
+                        case InsertMergeExtend:
+                                /* we can't reset the time of the existing note because
+                                   that will corrupt time ordering. So remove the
+                                   existing note and change the position/length
+                                   of the new note (which has not been added yet)
+                                */
+                                to_be_deleted.insert (*i);
+                                set_note_length = true;
+                                note_length = min (note_length, (*i)->end_time() - note->time());
+                                break;
+                        default:
+                                /*NOTREACHED*/
+                                /* stupid gcc */
+                                break;
+                        }
+                        break;
+
+                case OverlapExternal:
+                        cerr << "OverlapExt\n";
+                        /* existing note overlaps all the new note */
+                        switch (insert_merge_policy()) {
+                        case InsertMergeReplace:
+                                to_be_deleted.insert (*i);
+                                break;
+                        case InsertMergeTruncateExisting:
+                        case InsertMergeTruncateAddition:
+                        case InsertMergeExtend:
+                                /* cannot add in this case */
+                                return -1;
+                        default:
+                                /*NOTREACHED*/
+                                /* stupid gcc */
+                                break;
+                        }
+                        break;
+
+                case OverlapInternal:
+                        cerr << "OverlapInt\n";
+                        /* new note fully overlaps an existing note */
+                        switch (insert_merge_policy()) {
+                        case InsertMergeReplace:
+                        case InsertMergeTruncateExisting:
+                        case InsertMergeTruncateAddition:
+                        case InsertMergeExtend:
+                                /* delete the existing note, the new one will cover it */
+                                to_be_deleted.insert (*i);
+                                break;
+                        default:
+                                /*NOTREACHED*/
+                                /* stupid gcc */
+                                break;
+                        }
+                        break;
+
+                default:
+                        /*NOTREACHED*/
+                        /* stupid gcc */
+                        break;
+                }
+        }
+
+        for (set<NotePtr>::iterator i = to_be_deleted.begin(); i != to_be_deleted.end(); ++i) {
+                remove_note_unlocked (*i);
+
+                if (cmd) {
+                        cmd->side_effect_remove (*i);
+                }
+        }
+
+        if (set_note_time) {
+                if (cmd) {
+                        cmd->change (note, DiffCommand::StartTime, note_time);
+                } 
+                note->set_time (note_time);
+        }
+
+        if (set_note_length) {
+                if (cmd) {
+                        cmd->change (note, DiffCommand::Length, note_length);
+                } 
+                note->set_length (note_length);
+        }
+
+        return 0;
+}
+
+InsertMergePolicy
+MidiModel::insert_merge_policy () const 
+{
+        /* XXX ultimately this should be a per-track or even per-model policy */
+
+        return _midi_source->session().config.get_insert_merge_policy();
 }
                         
 void
diff --git a/libs/ardour/midi_source.cc b/libs/ardour/midi_source.cc
index 36cd36795a..3f831b348d 100644
--- a/libs/ardour/midi_source.cc
+++ b/libs/ardour/midi_source.cc
@@ -293,22 +293,34 @@ MidiSource::session_saved()
         */
 
 	flush_midi();
-        cerr << name() << " @ " << this << " length at save = " << _length_beats << endl;
 
+	if (_model && _model->edited()) {
 #if 0 // old style: clone the source if necessary on every session save
       // and switch to the new source
-
-	if (_model && _model->edited()) {
-                cerr << "Model exists and is edited\n";
-
 		boost::shared_ptr<MidiSource> newsrc = clone ();
 
                 if (newsrc) {
-                        _model->set_midi_source (newsrc.get());
+                        _model->set_midi_source (newsrc);
                         Switched (newsrc); /* EMIT SIGNAL */
                 }
-	}
+#else
+                // new style: if the model is edited, write its contents into
+                // the current source file (overwiting previous contents.
+
+                /* temporarily drop our reference to the model so that
+                   as the model pushes its current state to us, we don't
+                   try to update it.
+                */
+
+                boost::shared_ptr<MidiModel> mm = _model ;
+                _model.reset ();   
+                mm->sync_to_source ();
+                _model = mm;
+                /* data is in the file now, its not removable */
 #endif
+        }
+
+        cerr << name() << " @ " << this << " length at save = " << _length_beats << endl;
 }
 
 void
@@ -322,7 +334,6 @@ MidiSource::set_note_mode(NoteMode mode)
 void
 MidiSource::drop_model ()
 {
-        cerr << name() << " drop model\n";
         _model.reset(); 
 	ModelChanged (); /* EMIT SIGNAL */
 }
diff --git a/libs/ardour/smf_source.cc b/libs/ardour/smf_source.cc
index 0df128c143..fe5a0f7c8f 100644
--- a/libs/ardour/smf_source.cc
+++ b/libs/ardour/smf_source.cc
@@ -392,6 +392,10 @@ SMFSource::mark_streaming_write_completed ()
 	}
 	
 	Evoral::SMF::end_write ();
+
+        /* data in the file now, not removable */
+
+        mark_nonremovable (); 
 }
 
 bool
@@ -500,10 +504,11 @@ void
 SMFSource::flush_midi ()
 {
         if (!writable()) {
-                cerr << "\n\n\n\n " << name() << " CANNOT FLUSH - not writable\n\n\n\n";
                 return;
         }
 
 	Evoral::SMF::end_write();
+        /* data in the file means its no longer removable */
+        mark_nonremovable (); 
 }