dramatic change in logic and naming for operations related to adding a MIDI region on demand and cloning/unlinking

Existing code would cause data loss due to creation of two Source objects referring the same path, one with removable flags and one without. Careful code review suggested a variety of thinkos, function naming problems and other confusion that caused this. I have tried ot extensively comment what is going on with these operations, because it is one key area in which MIDI differs from audio: with audio, capture is the only way to add a new audio region, but for MIDI there are GUI input events that can add a new region.

5 files changed, 21 insertions, 13 deletions

diff --git a/libs/ardour/ardour/audiofilesource.h b/libs/ardour/ardour/audiofilesource.h
index 53819c1c9e..e0a199fd72 100644
--- a/libs/ardour/ardour/audiofilesource.h
+++ b/libs/ardour/ardour/audiofilesource.h
@@ -39,10 +39,6 @@ class LIBARDOUR_API AudioFileSource : public AudioSource, public FileSource {
 public:
 	virtual ~AudioFileSource ();
 
-	bool set_name (const std::string& newname) {
-		return (set_source_name(newname, destructive()) == 0);
-	}
-
 	std::string peak_path (std::string audio_path);
 	std::string find_broken_peakfile (std::string missing_peak_path,
 	                                  std::string audio_path);
diff --git a/libs/ardour/ardour/midi_region.h b/libs/ardour/ardour/midi_region.h
index b326bb30d8..38229b998b 100644
--- a/libs/ardour/ardour/midi_region.h
+++ b/libs/ardour/ardour/midi_region.h
@@ -64,6 +64,7 @@ class LIBARDOUR_API MidiRegion : public Region
 	~MidiRegion();
 
 	boost::shared_ptr<MidiRegion> clone (std::string path = std::string()) const;
+	boost::shared_ptr<MidiRegion> clone (boost::shared_ptr<MidiSource>) const;
 
 	boost::shared_ptr<MidiSource> midi_source (uint32_t n=0) const;
 
diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h
index ba50102ec9..07a32c5bfc 100644
--- a/libs/ardour/ardour/midi_source.h
+++ b/libs/ardour/ardour/midi_source.h
@@ -49,9 +49,21 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	MidiSource (Session& session, const XMLNode&);
 	virtual ~MidiSource ();
 
-	boost::shared_ptr<MidiSource> clone (const std::string& path,
-					     Evoral::MusicalTime begin = Evoral::MinMusicalTime,
-	                                     Evoral::MusicalTime end = Evoral::MaxMusicalTime);
+	/** Write the data in the given time range to another MidiSource
+	 * \param newsrc MidiSource to which data will be written. Should be a
+	 *        new, empty source. If it already has contents, the results are
+	 *        undefined. Source must be writable.
+	 *
+	 * \param begin time of earliest event that can be written.
+	 * \param end time of latest event that can be written.
+	 *
+	 * Returns zero on success, non-zero if the write failed for any
+	 * reason.
+	 *
+	 */
+	int write_to (boost::shared_ptr<MidiSource> newsrc,
+		      Evoral::MusicalTime begin = Evoral::MinMusicalTime,
+		      Evoral::MusicalTime end = Evoral::MaxMusicalTime);
 
 	/** Read the data in a given time range from the MIDI source.
 	 * All time stamps in parameters are in audio frames (even if the source has tempo time).
diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h
index d183494f87..ecb47a102a 100644
--- a/libs/ardour/ardour/session.h
+++ b/libs/ardour/ardour/session.h
@@ -194,7 +194,7 @@ class LIBARDOUR_API Session : public PBD::StatefulDestructible, public PBD::Scop
 
 	std::string peak_path (std::string) const;
 
-	std::string change_source_path_by_name (std::string oldpath, std::string oldname, std::string newname, bool destructive);
+	std::string generate_new_source_path_from_name (std::string oldpath, std::string oldname, std::string newname, bool destructive);
 
 	std::string peak_path_from_audio_path (std::string) const;
 	std::string new_audio_source_name (const std::string&, uint32_t nchans, uint32_t chan, bool destructive);
@@ -582,11 +582,12 @@ class LIBARDOUR_API Session : public PBD::StatefulDestructible, public PBD::Scop
 	boost::shared_ptr<AudioFileSource> create_audio_source_for_session (
 		size_t, std::string const &, uint32_t, bool destructive);
 
-	boost::shared_ptr<MidiSource> create_midi_source_for_session (
-		Track*, std::string const &);
+	boost::shared_ptr<MidiSource> create_midi_source_for_session (std::string const &);
+	boost::shared_ptr<MidiSource> create_midi_source_by_stealing_name (boost::shared_ptr<Track>);
 
 	boost::shared_ptr<Source> source_by_id (const PBD::ID&);
-	boost::shared_ptr<Source> source_by_path_and_channel (const std::string&, uint16_t);
+	boost::shared_ptr<AudioFileSource> source_by_path_and_channel (const std::string&, uint16_t) const;
+	boost::shared_ptr<MidiSource> source_by_path (const std::string&) const;
 	uint32_t count_sources_by_origin (const std::string&);
 
 	void add_playlist (boost::shared_ptr<Playlist>, bool unused = false);
diff --git a/libs/ardour/ardour/smf_source.h b/libs/ardour/ardour/smf_source.h
index 6e69b7b2f3..193330ef36 100644
--- a/libs/ardour/ardour/smf_source.h
+++ b/libs/ardour/ardour/smf_source.h
@@ -49,8 +49,6 @@ public:
 		return safe_midi_file_extension(path);
 	}
 
-	bool set_name (const std::string& newname) { return (set_source_name(newname, false) == 0); }
-
 	void append_event_unlocked_beats (const Evoral::Event<Evoral::MusicalTime>& ev);
 	void append_event_unlocked_frames (const Evoral::Event<framepos_t>& ev, framepos_t source_start);