From 74bc0c84686c4a85941b98d17179d3209bf9a2a8 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Mon, 2 Jun 2014 11:20:37 -0400 Subject: substantive changes to the logic and safety for naming of (audio/MIDI) sources, especially when created via import --- libs/ardour/file_source.cc | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) (limited to 'libs/ardour/file_source.cc') diff --git a/libs/ardour/file_source.cc b/libs/ardour/file_source.cc index de2783a1ac..d579d11965 100644 --- a/libs/ardour/file_source.cc +++ b/libs/ardour/file_source.cc @@ -214,7 +214,7 @@ FileSource::move_to_trash (const string& trash_dir_name) if (move_dependents_to_trash() != 0) { /* try to back out */ - rename (newpath.c_str(), _path.c_str()); + ::rename (newpath.c_str(), _path.c_str()); return -1; } @@ -581,3 +581,28 @@ FileSource::is_stub () const return true; } +int +FileSource::rename (const string& newpath) +{ + Glib::Threads::Mutex::Lock lm (_lock); + string oldpath = _path; + + // Test whether newpath exists, if yes notify the user but continue. + if (Glib::file_test (newpath, Glib::FILE_TEST_EXISTS)) { + error << string_compose (_("Programming error! %1 tried to rename a file over another file! It's safe to continue working, but please report this to the developers."), PROGRAM_NAME) << 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; + + return 0; +} -- cgit v1.2.3 From b660bc8ae92d19aedf0165815432b77a0c6170c4 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Tue, 10 Jun 2014 10:07:04 -0400 Subject: fix crash recovery: add new constructors to SndFileSource, AudioFileSource, add a new SourceFactory method and finally tweak AudioDiskstream::use_pending_capture_data() to create both the required whole-file and the in-playlist regions --- libs/ardour/ardour/audiofilesource.h | 6 ++++++ libs/ardour/ardour/sndfilesource.h | 11 ++++++++++- libs/ardour/ardour/source_factory.h | 3 +++ libs/ardour/audio_diskstream.cc | 38 ++++++++++++++++++++++++------------ libs/ardour/audiofilesource.cc | 16 +++++++++++++++ libs/ardour/file_source.cc | 2 +- libs/ardour/sndfilesource.cc | 36 ++++++++++++++++++++++++++++++++++ libs/ardour/source_factory.cc | 33 +++++++++++++++++++++++++++++++ 8 files changed, 131 insertions(+), 14 deletions(-) (limited to 'libs/ardour/file_source.cc') diff --git a/libs/ardour/ardour/audiofilesource.h b/libs/ardour/ardour/audiofilesource.h index 9be8d6ce45..7f4b18e404 100644 --- a/libs/ardour/ardour/audiofilesource.h +++ b/libs/ardour/ardour/audiofilesource.h @@ -93,6 +93,12 @@ protected: /** Constructor to be called for existing in-session files */ AudioFileSource (Session&, const XMLNode&, bool must_exist = true); + /** Constructor to be called for crash recovery. Final argument is not + * used but exists to differentiate from the external-to-session + * constructor above. + */ + AudioFileSource (Session&, const std::string& path, Source::Flag flags, bool); + int init (const std::string& idstr, bool must_exist); virtual void set_header_timeline_position () = 0; diff --git a/libs/ardour/ardour/sndfilesource.h b/libs/ardour/ardour/sndfilesource.h index 3f63f1c598..9604d3f232 100644 --- a/libs/ardour/ardour/sndfilesource.h +++ b/libs/ardour/ardour/sndfilesource.h @@ -38,7 +38,16 @@ class SndFileSource : public AudioFileSource { SampleFormat samp_format, HeaderFormat hdr_format, framecnt_t rate, Flag flags = SndFileSource::default_writable_flags); - /** Constructor to be called for existing in-session files */ + /* Constructor to be called for recovering files being used for + * capture. They are in-session, they already exist, they should not + * be writable. They are an odd hybrid (from a constructor point of + * view) of the previous two constructors. + */ + SndFileSource (Session&, const std::string& path, int chn); + + /** Constructor to be called for existing in-session files during + * session loading + */ SndFileSource (Session&, const XMLNode&); ~SndFileSource (); diff --git a/libs/ardour/ardour/source_factory.h b/libs/ardour/ardour/source_factory.h index c94f783b44..ce0f86bb6b 100644 --- a/libs/ardour/ardour/source_factory.h +++ b/libs/ardour/ardour/source_factory.h @@ -57,6 +57,9 @@ class SourceFactory { bool destructive, framecnt_t rate, bool announce = true, bool async = false); + static boost::shared_ptr createForRecovery + (DataType type, Session&, const std::string& path, int chn); + static boost::shared_ptr createFromPlaylist (DataType type, Session& s, boost::shared_ptr p, const PBD::ID& orig, const std::string& name, uint32_t chn, frameoffset_t start, framecnt_t len, bool copy, bool defer_peaks); diff --git a/libs/ardour/audio_diskstream.cc b/libs/ardour/audio_diskstream.cc index 010e1da21f..7785284dac 100644 --- a/libs/ardour/audio_diskstream.cc +++ b/libs/ardour/audio_diskstream.cc @@ -2181,11 +2181,16 @@ AudioDiskstream::use_pending_capture_data (XMLNode& node) continue; } + /* XXX as of June 2014, we always record to mono + files. Since this Source is being created as part of + crash recovery, we know that we need the first + channel (the final argument to the SourceFactory + call below). If we ever support non-mono files for + capture, this will need rethinking. + */ + try { - fs = boost::dynamic_pointer_cast ( - SourceFactory::createWritable ( - DataType::AUDIO, _session, - prop->value(), false, _session.frame_rate())); + fs = boost::dynamic_pointer_cast (SourceFactory::createForRecovery (DataType::AUDIO, _session, prop->value(), 0)); } catch (failed_constructor& err) { @@ -2216,21 +2221,31 @@ AudioDiskstream::use_pending_capture_data (XMLNode& node) return -1; } - boost::shared_ptr region; - try { - PropertyList plist; + boost::shared_ptr wf_region; + boost::shared_ptr region; + + /* First create the whole file region */ + PropertyList plist; + plist.add (Properties::start, 0); plist.add (Properties::length, first_fs->length (first_fs->timeline_position())); plist.add (Properties::name, region_name_from_path (first_fs->name(), true)); - region = boost::dynamic_pointer_cast (RegionFactory::create (pending_sources, plist)); + wf_region = boost::dynamic_pointer_cast (RegionFactory::create (pending_sources, plist)); + + wf_region->set_automatic (true); + wf_region->set_whole_file (true); + wf_region->special_set_position (position); - region->set_automatic (true); - region->set_whole_file (true); - region->special_set_position (0); + /* Now create a region that isn't the whole file for adding to + * the playlist */ + + region = boost::dynamic_pointer_cast (RegionFactory::create (pending_sources, plist)); + + _playlist->add_region (region, position); } catch (failed_constructor& err) { @@ -2241,7 +2256,6 @@ AudioDiskstream::use_pending_capture_data (XMLNode& node) return -1; } - _playlist->add_region (region, position); return 0; } diff --git a/libs/ardour/audiofilesource.cc b/libs/ardour/audiofilesource.cc index 37bf502177..8c3bf00176 100644 --- a/libs/ardour/audiofilesource.cc +++ b/libs/ardour/audiofilesource.cc @@ -115,6 +115,22 @@ AudioFileSource::AudioFileSource (Session& s, const string& path, const string& } } +/** Constructor used for existing internal-to-session files during crash + * recovery. File must exist + */ +AudioFileSource::AudioFileSource (Session& s, const string& path, Source::Flag flags, bool /* ignored-exists-for-prototype differentiation */) + : Source (s, DataType::AUDIO, path, flags) + , AudioSource (s, path) + , FileSource (s, DataType::AUDIO, path, string(), flags) +{ + /* note that origin remains empty */ + + if (init (_path, true)) { + throw failed_constructor (); + } +} + + /** Constructor used for existing internal-to-session files via XML. File must exist. */ AudioFileSource::AudioFileSource (Session& s, const XMLNode& node, bool must_exist) : Source (s, node) diff --git a/libs/ardour/file_source.cc b/libs/ardour/file_source.cc index d579d11965..30ae2178fe 100644 --- a/libs/ardour/file_source.cc +++ b/libs/ardour/file_source.cc @@ -56,7 +56,7 @@ PBD::Signal3 > FileSource:: FileSource::FileSource (Session& session, DataType type, const string& path, const string& origin, Source::Flag flag) : Source(session, type, path, flag) , _path (path) - , _file_is_new (!origin.empty()) // origin empty => new file VS. origin !empty => new file + , _file_is_new (!origin.empty()) // if origin is left unspecified (empty string) then file must exist , _channel (0) , _origin (origin) , _open (false) diff --git a/libs/ardour/sndfilesource.cc b/libs/ardour/sndfilesource.cc index 6b019f6fd0..5465c5e4a4 100644 --- a/libs/ardour/sndfilesource.cc +++ b/libs/ardour/sndfilesource.cc @@ -183,6 +183,34 @@ SndFileSource::SndFileSource (Session& s, const string& path, const string& orig } } +/** Constructor to be called for recovering files being used for + * capture. They are in-session, they already exist, they should not + * be writable. They are an odd hybrid (from a constructor point of + * view) of the previous two constructors. + */ +SndFileSource::SndFileSource (Session& s, const string& path, int chn) + : Source (s, DataType::AUDIO, path, Flag (0)) + /* the final boolean argument is not used, its value is irrelevant. see audiofilesource.h for explanation */ + , AudioFileSource (s, path, Flag (0)) + , _descriptor (0) + , _broadcast_info (0) + , _capture_start (false) + , _capture_end (false) + , file_pos (0) + , xfade_buf (0) +{ + _channel = chn; + + init_sndfile (); + + assert (Glib::file_test (_path, Glib::FILE_TEST_EXISTS)); + existence_check (); + + if (open()) { + throw failed_constructor (); + } +} + void SndFileSource::init_sndfile () { @@ -256,6 +284,14 @@ SndFileSource::open () delete _broadcast_info; _broadcast_info = 0; _flags = Flag (_flags & ~Broadcast); + } + + /* Set the broadcast flag if the BWF info is already there. We need + * this when recovering or using existing files. + */ + + if (bwf_info_exists) { + _flags = Flag (_flags | Broadcast); } if (writable()) { diff --git a/libs/ardour/source_factory.cc b/libs/ardour/source_factory.cc index 391b205a94..6d2bb80b30 100644 --- a/libs/ardour/source_factory.cc +++ b/libs/ardour/source_factory.cc @@ -341,6 +341,39 @@ SourceFactory::createWritable (DataType type, Session& s, const std::string& pat return boost::shared_ptr (); } +boost::shared_ptr +SourceFactory::createForRecovery (DataType type, Session& s, const std::string& path, int chn) +{ + /* this might throw failed_constructor(), which is OK */ + + if (type == DataType::AUDIO) { + Source* src = new SndFileSource (s, path, chn); + +#ifdef BOOST_SP_ENABLE_DEBUG_HOOKS + // boost_debug_shared_ptr_mark_interesting (src, "Source"); +#endif + boost::shared_ptr ret (src); + + if (setup_peakfile (ret, false)) { + return boost::shared_ptr(); + } + + // no analysis data - this is still basically a new file (we + // crashed while recording. + + // always announce these files + + SourceCreated (ret); + + return ret; + + } else if (type == DataType::MIDI) { + error << _("Recovery attempted on a MIDI file - not implemented") << endmsg; + } + + return boost::shared_ptr (); +} + boost::shared_ptr SourceFactory::createFromPlaylist (DataType type, Session& s, boost::shared_ptr p, const PBD::ID& orig, const std::string& name, uint32_t chn, frameoffset_t start, framecnt_t len, bool copy, bool defer_peaks) -- cgit v1.2.3