diff options
author | Paul Davis <paul@linuxaudiosystems.com> | 2016-06-28 15:05:48 -0400 |
---|---|---|
committer | Paul Davis <paul@linuxaudiosystems.com> | 2016-06-28 15:05:56 -0400 |
commit | 54d5f36311d041ce3d5fa3e6ed14fde30bcb70b7 (patch) | |
tree | 8bb084f1feea765dd33de4ace30c0c1030da40fc /libs/ardour/session_state.cc | |
parent | dbb26485df31bbf3bd0aeb83e3e01d42e25fbd07 (diff) |
changes in logic used by source cleanup to avoid endless recursion in sessions with deeply nested/recursive compound regions.
This also fixes some potentially dangerous cleanup logic related to two sources with the same name (but different paths)
Diffstat (limited to 'libs/ardour/session_state.cc')
-rw-r--r-- | libs/ardour/session_state.cc | 129 |
1 files changed, 83 insertions, 46 deletions
diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc index 0075799a16..752c8d03c9 100644 --- a/libs/ardour/session_state.cc +++ b/libs/ardour/session_state.cc @@ -2872,14 +2872,17 @@ Session::find_all_sources_across_snapshots (set<string>& result, bool exclude_th return 0; } - this_snapshot_path = _path; - this_snapshot_path += legalize_for_path (_current_snapshot_name); + this_snapshot_path = Glib::build_filename (_path, legalize_for_path (_current_snapshot_name)); this_snapshot_path += statefile_suffix; for (vector<string>::iterator i = state_files.begin(); i != state_files.end(); ++i) { + cerr << "Looking at snapshot " << (*i) << " ( with this = [" << this_snapshot_path << "])\n"; + if (exclude_this_snapshot && *i == this_snapshot_path) { + cerr << "\texcluded\n"; continue; + } if (find_all_sources (*i, result) < 0) { @@ -3010,6 +3013,12 @@ Session::cleanup_peakfiles () return 0; } +static void +merge_all_sources (boost::shared_ptr<const Playlist> pl, std::set<boost::shared_ptr<Source> >* all_sources) +{ + pl->deep_sources (*all_sources); +} + int Session::cleanup_sources (CleanupReport& rep) { @@ -3020,14 +3029,14 @@ Session::cleanup_sources (CleanupReport& rep) string midi_path; vector<string> candidates; vector<string> unused; - set<string> all_sources; - bool used; + set<string> sources_used_by_all_snapshots; string spath; int ret = -1; string tmppath1; string tmppath2; Searchpath asp; Searchpath msp; + set<boost::shared_ptr<Source> > sources_used_by_this_snapshot; _state_of_the_state = (StateOfTheState) (_state_of_the_state | InCleanup); @@ -3098,12 +3107,21 @@ Session::cleanup_sources (CleanupReport& rep) find_files_matching_filter (candidates, audio_path, accept_all_audio_files, (void *) 0, true, true); find_files_matching_filter (candidates, midi_path, accept_all_midi_files, (void *) 0, true, true); - /* find all sources, but don't use this snapshot because the - state file on disk still references sources we may have already - dropped. + /* add sources from all other snapshots as "used", but don't use this + snapshot because the state file on disk still references sources we + may have already dropped. */ - find_all_sources_across_snapshots (all_sources, true); + find_all_sources_across_snapshots (sources_used_by_all_snapshots, true); + + /* Although the region factory has a list of all regions ever created + * for this session, we're only interested in regions actually in + * playlists right now. So merge all playlist regions lists together. + * + * This will include the playlists used within compound regions. + */ + + playlists->foreach (boost::bind (merge_all_sources, _1, &sources_used_by_this_snapshot)); /* add our current source list */ @@ -3113,59 +3131,76 @@ Session::cleanup_sources (CleanupReport& rep) SourceMap::iterator tmp = i; ++tmp; - if ((fs = boost::dynamic_pointer_cast<FileSource> (i->second)) != 0) { + if ((fs = boost::dynamic_pointer_cast<FileSource> (i->second)) == 0) { + /* not a file */ + i = tmp; + continue; + } - /* this is mostly for windows which doesn't allow file - * renaming if the file is in use. But we don't special - * case it because we need to know if this causes - * problems, and the easiest way to notice that is to - * keep it in place for all platforms. - */ + /* this is mostly for windows which doesn't allow file + * renaming if the file is in use. But we do not special + * case it because we need to know if this causes + * problems, and the easiest way to notice that is to + * keep it in place for all platforms. + */ - fs->close (); + fs->close (); - if (!fs->is_stub()) { + if (!fs->is_stub()) { - if (playlists->source_use_count (fs) != 0) { - all_sources.insert (fs->path()); - } else { + /* Note that we're checking a list of all + * sources across all snapshots with the list + * of sources used by this snapshot. + */ - /* we might not remove this source from disk, because it may be used - by other snapshots, but its not being used in this version - so lets get rid of it now, along with any representative regions - in the region list. - */ + if (sources_used_by_this_snapshot.find (i->second) != sources_used_by_this_snapshot.end()) { + /* this source is in use by this snapshot */ + sources_used_by_all_snapshots.insert (fs->path()); + cerr << "Source from source list found in used_by_this_snapshot (" << fs->path() << ")\n"; + } else { + cerr << "Source from source list NOT found in used_by_this_snapshot (" << fs->path() << ")\n"; + /* this source is NOT in use by this snapshot + */ - RegionFactory::remove_regions_using_source (i->second); + /* remove all related regions from RegionFactory master list + */ - // also remove source from all_sources + RegionFactory::remove_regions_using_source (i->second); - for (set<string>::iterator j = all_sources.begin(); j != all_sources.end(); ++j) { - spath = Glib::path_get_basename (*j); - if (spath == i->second->name()) { - all_sources.erase (j); - break; - } - } + /* remove from our current source list + * also. We may not remove it from + * disk, because it may be used by + * other snapshots, but it isn't used inside this + * snapshot anymore, so we don't need a + * reference to it. + */ - sources.erase (i); - } + sources.erase (i); } } i = tmp; } + /* now check each candidate source to see if it exists in the list of + sources_used_by_all_snapshots. If it doesn't, put it into "unused". + */ + + cerr << "Candidates: " << candidates.size() << endl; + cerr << "Used by others: " << sources_used_by_all_snapshots.size() << endl; + for (vector<string>::iterator x = candidates.begin(); x != candidates.end(); ++x) { - used = false; + bool used = false; spath = *x; - for (set<string>::iterator i = all_sources.begin(); i != all_sources.end(); ++i) { + for (set<string>::iterator i = sources_used_by_all_snapshots.begin(); i != sources_used_by_all_snapshots.end(); ++i) { tmppath1 = canonical_path (spath); tmppath2 = canonical_path ((*i)); + cerr << "\t => " << tmppath2 << endl; + if (tmppath1 == tmppath2) { used = true; break; @@ -3177,6 +3212,14 @@ Session::cleanup_sources (CleanupReport& rep) } } + cerr << "Actually unused: " << unused.size() << endl; + + if (unused.empty()) { + /* Nothing to do */ + ret = 0; + goto out; + } + /* now try to move all unused files into the "dead" directory(ies) */ for (vector<string>::iterator x = unused.begin(); x != unused.end(); ++x) { @@ -3239,19 +3282,13 @@ Session::cleanup_sources (CleanupReport& rep) newpath = newpath_v; } - } else { - - /* it doesn't exist, or we can't read it or something */ - } g_stat ((*x).c_str(), &statbuf); if (::rename ((*x).c_str(), newpath.c_str()) != 0) { - error << string_compose (_("cannot rename unused file source from %1 to %2 (%3)"), - (*x), newpath, strerror (errno)) - << endmsg; - goto out; + error << string_compose (_("cannot rename unused file source from %1 to %2 (%3)"), (*x), newpath, strerror (errno)) << endmsg; + continue; } /* see if there an easy to find peakfile for this file, and remove it. |