From 39f054cac3d9c0903d3b9e28c09f784214f09106 Mon Sep 17 00:00:00 2001 From: James Crook Date: Mon, 29 Jul 2019 19:04:30 +0100 Subject: [PATCH] Bug 536 - OD computation stalls if applying effect before aliased waveform computation completes - and subsequent crash Fixed by ensuring new imports now always copy data in. The 'Projects' preferences page is no longer needed. There is one fewer warnings pref. The relevant import pref is gone. Importing of wave data no longer offers the option of working by reference. I have kept the menu item 'Check Dependencies' for now, as it gives a way for a user to convert an old by-reference audacity project to a self-contained one. The message for self-contained projects has been updated. --- src/Dependencies.cpp | 12 ++++++++++++ src/Experimental.h | 5 +++++ src/ProjectFileManager.cpp | 3 +++ src/import/ImportPCM.cpp | 12 +++++++++++- src/prefs/ImportExportPrefs.cpp | 2 ++ src/prefs/PrefsDialog.cpp | 3 +++ src/prefs/WarningsPrefs.cpp | 2 ++ 7 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/Dependencies.cpp b/src/Dependencies.cpp index f3816706d..22bd57b75 100644 --- a/src/Dependencies.cpp +++ b/src/Dependencies.cpp @@ -609,10 +609,17 @@ bool ShowDependencyDialogIfNeeded(AudacityProject *project, if (!isSaving) { wxString msg = +#ifdef EXPERIMENTAL_OD_DATA _("Your project is currently self-contained; it does not depend on any external audio files. \ \n\nIf you change the project to a state that has external dependencies on imported \ files, it will no longer be self-contained. If you then Save without copying those files in, \ you may lose data."); +#else +_("Your project is self-contained; it does not depend on any external audio files. \ +\n\nSome older Audacity projects may not be self-contained, and care \n\ +is needed to keep their external dependencies in the right place.\n\ +New projects will be self-contained and are less risky."); +#endif AudacityMessageBox(msg, _("Dependency Check"), wxOK | wxICON_INFORMATION, @@ -623,6 +630,7 @@ you may lose data."); if (isSaving) { +#ifdef EXPERIMENTAL_OD_DATA wxString action = gPrefs->Read( wxT("/FileFormats/SaveProjectWithDependencies"), @@ -636,6 +644,10 @@ you may lose data."); if (action == wxT("never")) // User never wants to remove dependencies return true; +#else + RemoveDependencies(project, aliasedFiles); + return true; +#endif } DependencyDialog dlog(pWindow, -1, project, aliasedFiles, isSaving); diff --git a/src/Experimental.h b/src/Experimental.h index cae1a04f9..2f555939d 100644 --- a/src/Experimental.h +++ b/src/Experimental.h @@ -254,4 +254,9 @@ // mmm-1 22 Aug 2018 //#define EXPERIMENTAL_R128_NORM +// JKC 29 July 2019 +// OD_DATA made experimental. It is on the way out because +// it is dangerous and has too many bugs. See bug 536 for example. +//#do not define EXPERIMENTAL_OD_DATA + #endif diff --git a/src/ProjectFileManager.cpp b/src/ProjectFileManager.cpp index 8846c161d..bb44ad14f 100644 --- a/src/ProjectFileManager.cpp +++ b/src/ProjectFileManager.cpp @@ -120,12 +120,14 @@ auto ProjectFileManager::ReadProjectFile( const FilePath &fileName ) XMLFileReader xmlFile; +#ifdef EXPERIMENTAL_OD_DATA // 'Lossless copy' projects have dependencies. We need to always copy-in // these dependencies when converting to a normal project. wxString oldAction = gPrefs->Read(wxT("/FileFormats/CopyOrEditUncompressedData"), wxT("copy")); bool oldAsk = gPrefs->ReadBool(wxT("/Warnings/CopyOrEditUncompressedDataAsk"), true); + if (oldAction != wxT("copy")) gPrefs->Write(wxT("/FileFormats/CopyOrEditUncompressedData"), wxT("copy")); if (oldAsk) @@ -140,6 +142,7 @@ auto ProjectFileManager::ReadProjectFile( const FilePath &fileName ) gPrefs->Write(wxT("/Warnings/CopyOrEditUncompressedDataAsk"), (long) true); gPrefs->Flush(); } ); +#endif bool bParseSuccess = xmlFile.Parse(&projectFileIO, fileName); diff --git a/src/import/ImportPCM.cpp b/src/import/ImportPCM.cpp index 6d2a3192a..bd46b3814 100644 --- a/src/import/ImportPCM.cpp +++ b/src/import/ImportPCM.cpp @@ -230,11 +230,14 @@ auto PCMImportFileHandle::GetFileUncompressedBytes() -> ByteCount return mInfo.frames * mInfo.channels * SAMPLE_SIZE(mFormat); } +#ifdef EXPERIMENTAL_OD_DATA // returns "copy" or "edit" (aliased) as the user selects. // if the cancel button is hit then "cancel" is returned. static wxString AskCopyOrEdit() { + wxString oldCopyPref = gPrefs->Read(wxT("/FileFormats/CopyOrEditUncompressedData"), wxT("copy")); + bool firstTimeAsk = gPrefs->Read(wxT("/Warnings/CopyOrEditUncompressedDataFirstAsk"), true)?true:false; bool oldAskPref = gPrefs->Read(wxT("/Warnings/CopyOrEditUncompressedDataAsk"), true)?true:false; @@ -342,6 +345,7 @@ static wxString AskCopyOrEdit() } return oldCopyPref; } +#endif #ifdef USE_LIBID3TAG struct id3_tag_deleter { @@ -358,6 +362,7 @@ ProgressResult PCMImportFileHandle::Import(TrackFactory *trackFactory, wxASSERT(mFile.get()); +#ifdef EXPERIMENTAL_OD_DATA // Get the preference / warn the user about aliased files. wxString copyEdit = AskCopyOrEdit(); @@ -368,6 +373,7 @@ ProgressResult PCMImportFileHandle::Import(TrackFactory *trackFactory, bool doEdit = false; if (copyEdit.IsSameAs(wxT("edit"), false)) doEdit = true; +#endif CreateProgress(); @@ -386,6 +392,7 @@ ProgressResult PCMImportFileHandle::Import(TrackFactory *trackFactory, auto maxBlockSize = channels.begin()->get()->GetMaxBlockSize(); auto updateResult = ProgressResult::Cancelled; +#ifdef EXPERIMENTAL_OD_DATA // If the format is not seekable, we must use 'copy' mode, // because 'edit' mode depends on the ability to seek to an // arbitrary location in the file. @@ -458,7 +465,10 @@ ProgressResult PCMImportFileHandle::Import(TrackFactory *trackFactory, ODManager::Instance()->AddNewTask(std::move(computeTask)); } } - else { + else +#endif + + { // Otherwise, we're in the "copy" mode, where we read in the actual // samples from the file and store our own local copy of the // samples in the tracks. diff --git a/src/prefs/ImportExportPrefs.cpp b/src/prefs/ImportExportPrefs.cpp index e188cea07..c493bcbc0 100644 --- a/src/prefs/ImportExportPrefs.cpp +++ b/src/prefs/ImportExportPrefs.cpp @@ -67,6 +67,7 @@ void ImportExportPrefs::PopulateOrExchange(ShuttleGui & S) S.StartStatic(_("When importing audio files")); { +#ifdef EXPERIMENTAL_OD_DATA S.StartRadioButtonGroup(wxT("/FileFormats/CopyOrEditUncompressedData"), wxT("copy")); { S.TieRadioButton(_("&Copy uncompressed files into the project (safer)"), @@ -75,6 +76,7 @@ void ImportExportPrefs::PopulateOrExchange(ShuttleGui & S) wxT("edit")); } S.EndRadioButtonGroup(); +#endif S.TieCheckBox(_("&Normalize all tracks in project"), wxT("/AudioFiles/NormalizeOnLoad"), diff --git a/src/prefs/PrefsDialog.cpp b/src/prefs/PrefsDialog.cpp index 8ca71711c..f0c7c1634 100644 --- a/src/prefs/PrefsDialog.cpp +++ b/src/prefs/PrefsDialog.cpp @@ -505,7 +505,10 @@ PrefsDialog::Factories PrefsNode(ImportExportPrefsFactory, 1), ExtImportPrefsFactory, +#ifdef EXPERIMENTAL_OD_DATA ProjectsPrefsFactory, +#endif + #if !defined(DISABLE_DYNAMIC_LOADING_FFMPEG) || !defined(DISABLE_DYNAMIC_LOADING_LAME) LibraryPrefsFactory, #endif diff --git a/src/prefs/WarningsPrefs.cpp b/src/prefs/WarningsPrefs.cpp index c561f407e..a0e8f9b50 100644 --- a/src/prefs/WarningsPrefs.cpp +++ b/src/prefs/WarningsPrefs.cpp @@ -87,9 +87,11 @@ void WarningsPrefs::PopulateOrExchange(ShuttleGui & S) S.TieCheckBox(_("Mixing down on export (&Custom FFmpeg or external program)"), wxT("/Warnings/MixUnknownChannels"), true); +#ifdef EXPERIMENTAL_OD_DATA S.TieCheckBox(_("&Importing uncompressed audio files"), wxT("/Warnings/CopyOrEditUncompressedDataAsk"), true); +#endif } S.EndStatic(); S.EndScroller();