From c52f7957a9b350dbb0d290a39acd33eb4472a218 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Thu, 10 Apr 2014 08:58:04 -0400 Subject: redesign technique for naming/creating regions for MIDI clone (or other non-capture driven MIDI region creation operations). See comments in Session::new_midi_source_name() for details. --- libs/ardour/ardour/audio_diskstream.h | 1 - libs/ardour/ardour/diskstream.h | 2 ++ libs/ardour/ardour/file_source.h | 2 +- libs/ardour/ardour/midi_diskstream.h | 2 +- libs/ardour/ardour/public_diskstream.h | 2 +- libs/ardour/ardour/track.h | 2 +- libs/ardour/audio_diskstream.cc | 8 -------- libs/ardour/file_source.cc | 17 ++++++++++++---- libs/ardour/midi_diskstream.cc | 33 +++++++++++++++---------------- libs/ardour/midi_source.cc | 18 ++++++++--------- libs/ardour/session.cc | 36 ++++++++++++++++++++++++---------- libs/ardour/track.cc | 6 +++--- 12 files changed, 72 insertions(+), 57 deletions(-) (limited to 'libs/ardour') diff --git a/libs/ardour/ardour/audio_diskstream.h b/libs/ardour/ardour/audio_diskstream.h index c0b78bad13..2e838ff628 100644 --- a/libs/ardour/ardour/audio_diskstream.h +++ b/libs/ardour/ardour/audio_diskstream.h @@ -139,7 +139,6 @@ class LIBARDOUR_API AudioDiskstream : public Diskstream void set_block_size (pframes_t); int internal_playback_seek (framecnt_t distance); int can_internal_playback_seek (framecnt_t distance); - std::list > steal_write_sources(); void reset_write_sources (bool, bool force = false); void non_realtime_input_change (); void non_realtime_locate (framepos_t location); diff --git a/libs/ardour/ardour/diskstream.h b/libs/ardour/ardour/diskstream.h index 5bd18663b8..dd74d5cb52 100644 --- a/libs/ardour/ardour/diskstream.h +++ b/libs/ardour/ardour/diskstream.h @@ -71,6 +71,8 @@ class LIBARDOUR_API Diskstream : public SessionObject, public PublicDiskstream virtual bool set_name (const std::string& str); + virtual std::string steal_write_source_name () { return std::string(); } + boost::shared_ptr io() const { return _io; } void set_track (ARDOUR::Track *); diff --git a/libs/ardour/ardour/file_source.h b/libs/ardour/ardour/file_source.h index e0db1d25b7..79153f6568 100644 --- a/libs/ardour/ardour/file_source.h +++ b/libs/ardour/ardour/file_source.h @@ -44,7 +44,7 @@ class LIBARDOUR_API MissingSource : public std::exception /** A source associated with a file on disk somewhere */ class LIBARDOUR_API FileSource : virtual public Source { public: - virtual ~FileSource () {} + virtual ~FileSource (); virtual const std::string& path() const { return _path; } diff --git a/libs/ardour/ardour/midi_diskstream.h b/libs/ardour/ardour/midi_diskstream.h index 01e8904736..5a10042096 100644 --- a/libs/ardour/ardour/midi_diskstream.h +++ b/libs/ardour/ardour/midi_diskstream.h @@ -111,7 +111,7 @@ class LIBARDOUR_API MidiDiskstream : public Diskstream void set_block_size (pframes_t); int internal_playback_seek (framecnt_t distance); int can_internal_playback_seek (framecnt_t distance); - std::list > steal_write_sources(); + std::string steal_write_source_name(); void reset_write_sources (bool, bool force = false); void non_realtime_input_change (); void non_realtime_locate (framepos_t location); diff --git a/libs/ardour/ardour/public_diskstream.h b/libs/ardour/ardour/public_diskstream.h index 5b5cd48231..4700e7b6be 100644 --- a/libs/ardour/ardour/public_diskstream.h +++ b/libs/ardour/ardour/public_diskstream.h @@ -38,7 +38,7 @@ public: virtual bool destructive () const = 0; virtual std::list > & last_capture_sources () = 0; virtual void set_capture_offset () = 0; - virtual std::list > steal_write_sources () = 0; + virtual std::string steal_write_source_name () = 0; virtual void reset_write_sources (bool, bool force = false) = 0; virtual float playback_buffer_load () const = 0; virtual float capture_buffer_load () const = 0; diff --git a/libs/ardour/ardour/track.h b/libs/ardour/ardour/track.h index 2a6d3f7ad4..ee74fee46e 100644 --- a/libs/ardour/ardour/track.h +++ b/libs/ardour/ardour/track.h @@ -120,7 +120,7 @@ class LIBARDOUR_API Track : public Route, public PublicDiskstream bool destructive () const; std::list > & last_capture_sources (); void set_capture_offset (); - std::list > steal_write_sources(); + std::string steal_write_source_name (); void reset_write_sources (bool, bool force = false); float playback_buffer_load () const; float capture_buffer_load () const; diff --git a/libs/ardour/audio_diskstream.cc b/libs/ardour/audio_diskstream.cc index dd2dcf324e..ffb8ef8a1b 100644 --- a/libs/ardour/audio_diskstream.cc +++ b/libs/ardour/audio_diskstream.cc @@ -1926,14 +1926,6 @@ AudioDiskstream::use_new_write_source (uint32_t n) return 0; } -list > -AudioDiskstream::steal_write_sources() -{ - /* not possible to steal audio write sources */ - list > ret; - return ret; -} - void AudioDiskstream::reset_write_sources (bool mark_write_complete, bool /*force*/) { diff --git a/libs/ardour/file_source.cc b/libs/ardour/file_source.cc index 288a4b7034..2b8a65ab92 100644 --- a/libs/ardour/file_source.cc +++ b/libs/ardour/file_source.cc @@ -81,6 +81,10 @@ FileSource::FileSource (Session& session, const XMLNode& node, bool /*must_exist prevent_deletion (); } +FileSource::~FileSource() +{ +} + void FileSource::prevent_deletion () { @@ -88,6 +92,8 @@ FileSource::prevent_deletion () */ if (Glib::file_test (_path, Glib::FILE_TEST_EXISTS)) { + cerr << " ... " << _path << " already exists, marking immutable\n"; + if (!(_flags & Destructive)) { mark_immutable (); } else { @@ -527,10 +533,13 @@ FileSource::set_source_name (const string& newname, bool destructive) return -1; } - if (::rename (oldpath.c_str(), newpath.c_str()) != 0) { - error << string_compose (_("cannot rename file %1 to %2 (%3)"), oldpath, newpath, strerror(errno)) << endmsg; - return -1; - } + if (Glib::file_test (oldpath.c_str(), Glib::FILE_TEST_EXISTS)) { + /* rename only needed if file exists on disk */ + if (::rename (oldpath.c_str(), newpath.c_str()) != 0) { + error << string_compose (_("cannot rename file %1 to %2 (%3)"), oldpath, newpath, strerror(errno)) << endmsg; + return -1; + } + } _name = Glib::path_get_basename (newpath); _path = newpath; diff --git a/libs/ardour/midi_diskstream.cc b/libs/ardour/midi_diskstream.cc index a1a640cd1c..6fd40be57b 100644 --- a/libs/ardour/midi_diskstream.cc +++ b/libs/ardour/midi_diskstream.cc @@ -1213,25 +1213,24 @@ MidiDiskstream::use_new_write_source (uint32_t n) return 0; } - -list > -MidiDiskstream::steal_write_sources() +/** + * We want to use the name of the existing write source (the one that will be + * used by the next capture) for another purpose. So change the name of the + * current source, and return its current name. + * + * Return an empty string if the change cannot be accomplished. + */ +std::string +MidiDiskstream::steal_write_source_name () { - list > ret; - - /* put some data on the disk, even if its just a header for an empty file */ - boost::dynamic_pointer_cast (_write_source)->ensure_disk_file (); - - /* never let it go away */ - _write_source->mark_nonremovable (); - - ret.push_back (_write_source); - - /* get a new one */ - - use_new_write_source (0); + std::string our_new_name = _session.new_midi_source_name (_write_source->name()); + std::string our_old_name = _write_source->name(); + + if (_write_source->set_source_name (our_new_name, false)) { + return string(); + } - return ret; + return our_old_name; } void diff --git a/libs/ardour/midi_source.cc b/libs/ardour/midi_source.cc index 8034634ba7..d4452490fb 100644 --- a/libs/ardour/midi_source.cc +++ b/libs/ardour/midi_source.cc @@ -335,21 +335,19 @@ MidiSource::mark_streaming_write_completed () boost::shared_ptr MidiSource::clone (const string& path, Evoral::MusicalTime begin, Evoral::MusicalTime end) { - string newname = PBD::basename_nosuffix(_name.val()); string newpath; + /* get a new name for the MIDI file we're going to write to + */ + if (path.empty()) { - - /* get a new name for the MIDI file we're going to write to - */ - - do { - newname = bump_name_once (newname, '-'); - newpath = Glib::build_filename (_session.session_directory().midi_path(), newname + ".mid"); - - } while (Glib::file_test (newpath, Glib::FILE_TEST_EXISTS)); + string newname = PBD::basename_nosuffix(_name.val()); + newname = bump_name_once (newname, '-'); + newname += ".mid"; + newpath = _session.new_source_path_from_name (DataType::MIDI, newname); } else { /* caller must check for pre-existing file */ + assert (!Glib::file_test (path, Glib::FILE_TEST_EXISTS)); newpath = path; } diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc index be2d4c5441..93bce28fcd 100644 --- a/libs/ardour/session.cc +++ b/libs/ardour/session.cc @@ -88,6 +88,7 @@ #include "ardour/smf_source.h" #include "ardour/source_factory.h" #include "ardour/speakers.h" +#include "ardour/track.h" #include "ardour/utils.h" #include "midi++/port.h" @@ -3587,6 +3588,7 @@ Session::new_midi_source_name (const string& base) legalized = legalize_for_path (base); // Find a "version" of the file name that doesn't exist in any of the possible directories. + for (cnt = 1; cnt <= limit; ++cnt) { vector::iterator i; @@ -3626,23 +3628,37 @@ Session::new_midi_source_name (const string& base) boost::shared_ptr Session::create_midi_source_for_session (Track* track, string const & n) { - /* try to use the existing write source for the track, to keep numbering sane - */ + std::string name; if (track) { - /*MidiTrack* mt = dynamic_cast (track); - assert (mt); + /* the caller passes in the track the source will be used in, + so that we can keep the numbering sane. + + Rationale: a track with the name "Foo" that has had N + captures carried out so far will already have a write source + named "Foo-N+1.mid" waiting to be used for the next capture. + + If we call new_midi_source_name() we will get "Foo-N+2". But + there is no region corresponding to "Foo-N+1", so when + "Foo-N+2" appears in the track, the gap presents the user + with odd behaviour - why did it skip past Foo-N+1? + + We could explain this to the user in some odd way, but + instead we rename "Foo-N+1.mid" as "Foo-N+2.mid", and then + use "Foo-N+1" here. + + If that attempted rename fails, we get "Foo-N+2.mid" anyway. */ - list > l = track->steal_write_sources (); + MidiTrack* mt = dynamic_cast (track); + assert (mt); + name = track->steal_write_source_name (); + } - if (!l.empty()) { - assert (boost::dynamic_pointer_cast (l.front())); - return boost::dynamic_pointer_cast (l.front()); - } + if (name.empty()) { + name = new_midi_source_name (n); } - const string name = new_midi_source_name (n); const string path = new_source_path_from_name (DataType::MIDI, name); return boost::dynamic_pointer_cast ( diff --git a/libs/ardour/track.cc b/libs/ardour/track.cc index 57239cb841..5c95de0c41 100644 --- a/libs/ardour/track.cc +++ b/libs/ardour/track.cc @@ -579,10 +579,10 @@ Track::set_capture_offset () _diskstream->set_capture_offset (); } -list > -Track::steal_write_sources() +std::string +Track::steal_write_source_name() { - return _diskstream->steal_write_sources (); + return _diskstream->steal_write_source_name (); } void -- cgit v1.2.3