MIDI region combine: fix conceptual errors

libs/ardour/ardour/midi_playlist.h

@@ -84,7 +84,7 @@ public:
 
 	std::set<Evoral::Parameter> contained_automation();
 
-	boost::shared_ptr<Region> combine (const RegionList&);
+	boost::shared_ptr<Region> combine (const RegionList&, boost::shared_ptr<Track>);
 	void uncombine (boost::shared_ptr<Region>);
 
   protected:

libs/ardour/ardour/playlist.h

@@ -59,6 +59,7 @@ namespace ARDOUR {
 class Session;
 class Playlist;
 class Crossfade;
+class Track;
 
 namespace Properties {
 	/* fake the type, since regions are handled by SequenceProperty which doesn't
@@ -186,7 +187,7 @@ public:
 	void duplicate_range (TimelineRange&, float times);
 	void duplicate_ranges (std::list<TimelineRange>&, float times);
 	void nudge_after (timepos_t const & start, timecnt_t const & distance, bool forwards);
-	virtual boost::shared_ptr<Region> combine (const RegionList&);
+	virtual boost::shared_ptr<Region> combine (const RegionList&, boost::shared_ptr<Track>);
 	virtual void uncombine (boost::shared_ptr<Region>);
 	void fade_range (std::list<TimelineRange>&);
 	void remove_gaps (timecnt_t const & gap_threshold, timecnt_t const & leave_gap, boost::function<void (timepos_t, timecnt_t)> gap_callback);

libs/ardour/midi_playlist.cc

@@ -440,7 +440,7 @@ MidiPlaylist::rendered ()
 }
 
 boost::shared_ptr<Region>
-MidiPlaylist::combine (RegionList const & rl)
+MidiPlaylist::combine (RegionList const & rl, boost::shared_ptr<Track> trk)
 {
 	RegionWriteLock rwl (this, true);
 
@@ -452,8 +452,6 @@ MidiPlaylist::combine (RegionList const & rl)
 	sorted.sort (RegionSortByLayerAndPosition());
 
 	boost::shared_ptr<Region> first = sorted.front();
-	RegionList::const_iterator i = sorted.begin();
-	++i;
 
 	timepos_t earliest (timepos_t::max (Temporal::BeatTime));
 	timepos_t latest (Temporal::BeatTime);
@@ -468,19 +466,20 @@ MidiPlaylist::combine (RegionList const & rl)
 		}
 	}
 
-	boost::shared_ptr<MidiRegion> new_region = boost::dynamic_pointer_cast<MidiRegion> (RegionFactory::create (first, true, true, &rwl.thawlist));
+	boost::shared_ptr<MidiSource> ms (session().create_midi_source_by_stealing_name (trk));
+	boost::shared_ptr<MidiRegion> new_region = boost::dynamic_pointer_cast<MidiRegion> (RegionFactory::create (ms, first->derive_properties (false), true, &rwl.thawlist));
 
 	timepos_t pos (first->position());
+	new_region->set_position (pos);
 
-	remove_region_internal (first, rwl.thawlist);
-
-	while (i != sorted.end()) {
-		new_region->merge (boost::dynamic_pointer_cast<MidiRegion> (*i));
-		remove_region_internal (*i, rwl.thawlist);
-		++i;
+	for (auto const & other : sorted) {
+		new_region->merge (boost::dynamic_pointer_cast<MidiRegion> (other));
+		remove_region_internal (other, rwl.thawlist);
 	}
 
-	new_region->set_length (earliest.distance (latest));
+	/* write MIDI to disk */
+
+	new_region->midi_source (0)->session_saved ();
 
 	add_region_internal (new_region, pos, rwl.thawlist);
 

libs/ardour/midi_region.cc

@@ -550,46 +550,37 @@ MidiRegion::set_name (const std::string& str)
 void
 MidiRegion::merge (boost::shared_ptr<MidiRegion const> other_region)
 {
-	Temporal::Beats last_event_time;
+	boost::shared_ptr<MidiModel const> other = other_region->model();
+	boost::shared_ptr<MidiModel> self = model();
 
-	{
-		Source::WriterLock lm (midi_source(0)->mutex());
-		boost::shared_ptr<MidiModel const> other = other_region->model();
-		boost::shared_ptr<MidiModel> self = model();
+	Temporal::Beats other_region_start (other_region->start().beats());
+	Temporal::Beats other_region_end ((other_region->start() + other_region->length()).beats());
 
-		Temporal::Beats other_region_start (other_region->start().beats());
-		Temporal::Beats other_region_end ((other_region->start() + other_region->length()).beats());
+	self->start_write ();
 
-		midi_source (0)->mark_streaming_midi_write_started (lm, Sustained);
+	for (Evoral::Sequence<Temporal::Beats>::const_iterator e = other->begin(); e != other->end(); ++e) {
 
-		for (Evoral::Sequence<Temporal::Beats>::const_iterator e = other->begin(); e != other->end(); ++e) {
-
-			if (e->time() < other_region_start) {
-				continue;
-			}
-
-			/* other_region_end is an inclusive end, not
-			 * exclusive, since we allow simultaneous MIDI events
-			 * (given appropriate semantic sorting)
-			 */
-
-			if (e->time() > other_region_end) {
-				break;
-			}
-
-			Evoral::Event<Temporal::Beats> ev (*e, true);
-
-			Temporal::Beats abs_time = other_region->source_beats_to_absolute_time (ev.time()).beats ();
-			Temporal::Beats srt = absolute_time_to_source_beats (timepos_t (abs_time));
-			ev.set_time (srt);
-			self->append (ev, Evoral::next_event_id());
-			last_event_time = abs_time;
+		if (e->time() < other_region_start) {
+			continue;
 		}
 
-		midi_source (0)->mark_streaming_write_completed (lm);
+		/* other_region_end is an inclusive end, not
+		 * exclusive, since we allow simultaneous MIDI events
+		 * (given appropriate semantic sorting)
+		 */
+
+		if (e->time() > other_region_end) {
+			break;
+		}
+
+		Evoral::Event<Temporal::Beats> ev (*e, true);
+		timepos_t abs_time (other_region->source_beats_to_absolute_time (ev.time()));
+		Temporal::Beats srt = position().distance (abs_time).beats();
+		ev.set_time (srt);
+
+		self->append (ev, Evoral::next_event_id());
 	}
 
-	Temporal::Beats len = last_event_time - position().beats();
-
-	set_length (timecnt_t (len, position()));
+	set_length (position().distance (other_region->end()));
+	self->end_write (Evoral::Sequence<Temporal::Beats>::ResolveStuckNotes, length().beats());
 }

libs/ardour/playlist.cc

@@ -3063,7 +3063,7 @@ Playlist::find_next_top_layer_position (timepos_t const & t) const
 }
 
 boost::shared_ptr<Region>
-Playlist::combine (const RegionList& rl)
+Playlist::combine (const RegionList& rl, boost::shared_ptr<Track>)
 {
 	if (rl.empty()) {
 		return boost::shared_ptr<Region>();