diff --git a/src/HistoryWindow.cpp b/src/HistoryWindow.cpp index 9633f91bc..ec37b3850 100644 --- a/src/HistoryWindow.cpp +++ b/src/HistoryWindow.cpp @@ -118,7 +118,7 @@ HistoryDialog::HistoryDialog(AudacityProject *parent, UndoManager *manager): wxDefaultSize, wxSP_ARROW_KEYS, 0, - mManager->GetCurrentState() - 1, + mManager->GetCurrentState(), 0); S.AddWindow(mLevels); /* i18n-hint: (verb)*/ @@ -204,11 +204,11 @@ void HistoryDialog::DoUpdate() mList->DeleteAllItems(); wxLongLong_t total = 0; - mSelected = mManager->GetCurrentState() - 1; + mSelected = mManager->GetCurrentState(); for (i = 0; i < (int)mManager->GetNumStates(); i++) { TranslatableString desc, size; - total += mManager->GetLongDescription(i + 1, &desc, &size); + total += mManager->GetLongDescription(i, &desc, &size); mList->InsertItem(i, desc.Translation(), i == mSelected ? 1 : 0); mList->SetItem(i, 1, size.Translation()); } @@ -261,7 +261,7 @@ void HistoryDialog::OnDiscard(wxCommandEvent & WXUNUSED(event)) mSelected -= i; mManager->RemoveStates(i); - ProjectHistory::Get( *mProject ).SetStateTo(mSelected + 1); + ProjectHistory::Get( *mProject ).SetStateTo(mSelected); while(--i >= 0) mList->DeleteItem(i); @@ -299,7 +299,7 @@ void HistoryDialog::OnItemSelected(wxListEvent &event) // entry. Doing so can cause unnecessary delays upon initial load or while // clicking the same entry over and over. if (selected != mSelected) { - ProjectHistory::Get( *mProject ).SetStateTo(selected + 1); + ProjectHistory::Get( *mProject ).SetStateTo(selected); } mSelected = selected; diff --git a/src/ProjectFileIO.cpp b/src/ProjectFileIO.cpp index 251bfafb3..ba4161488 100644 --- a/src/ProjectFileIO.cpp +++ b/src/ProjectFileIO.cpp @@ -661,7 +661,7 @@ void ProjectFileIO::InSet(sqlite3_context *context, int argc, sqlite3_value **ar sqlite3_result_int(context, blockids->find(blockid) != blockids->end()); } -bool ProjectFileIO::CheckForOrphans(BlockIDs &blockids) +bool ProjectFileIO::DeleteBlocks(const BlockIDs &blockids, bool complement) { auto db = DB(); int rc; @@ -673,15 +673,19 @@ bool ProjectFileIO::CheckForOrphans(BlockIDs &blockids) }); // Add the function used to verify each row's blockid against the set of active blockids - rc = sqlite3_create_function(db, "inset", 1, SQLITE_UTF8 | SQLITE_DETERMINISTIC, &blockids, InSet, nullptr, nullptr); + const void *p = &blockids; + rc = sqlite3_create_function(db, "inset", 1, SQLITE_UTF8 | SQLITE_DETERMINISTIC, const_cast(p), InSet, nullptr, nullptr); if (rc != SQLITE_OK) { wxLogDebug(wxT("Unable to add 'inset' function")); return false; } - // Delete all rows that are orphaned - rc = sqlite3_exec(db, "DELETE FROM sampleblocks WHERE NOT inset(blockid);", nullptr, nullptr, nullptr); + // Delete all rows in the set, or not in it + auto sql = wxString::Format( + "DELETE FROM sampleblocks WHERE %sinset(blockid);", + complement ? "NOT " : "" ); + rc = sqlite3_exec(db, sql, nullptr, nullptr, nullptr); if (rc != SQLITE_OK) { wxLogWarning(XO("Cleanup of orphan blocks failed").Translation()); @@ -708,24 +712,12 @@ Connection ProjectFileIO::CopyTo(const FilePath &destpath, auto pProject = &mProject; auto &tracklist = tracks ? *tracks : TrackList::Get(*pProject); - BlockIDs blockids; + SampleBlockIDSet blockids; // Collect all active blockids if (prune) { - for (auto wt : tracklist.Any()) - { - // Scan all clips within current track - for (const auto &clip : wt->GetAllClips()) - { - // Scan all sample blocks within current clip - auto blocks = clip->GetSequenceBlockArray(); - for (const auto &block : *blocks) - { - blockids.insert(block.sb->GetBlockID()); - } - } - } + InspectBlocks( tracklist, {}, &blockids ); } // Collect ALL blockids else @@ -929,33 +921,13 @@ Connection ProjectFileIO::CopyTo(const FilePath &destpath, bool ProjectFileIO::ShouldVacuum(const std::shared_ptr &tracks) { - std::set active; + SampleBlockIDSet active; unsigned long long current = 0; - // Scan all wave tracks - for (auto wt : tracks->Any()) - { - // Scan all clips within current track - for (const auto &clip : wt->GetAllClips()) - { - // Scan all sample blocks within current clip - auto blocks = clip->GetSequenceBlockArray(); - for (const auto &block : *blocks) - { - const auto &sb = block.sb; - auto blockid = sb->GetBlockID(); - - // Accumulate space used by the block if the blockid has not - // yet been seen - if (active.count(blockid) == 0) - { - current += sb->GetSpaceUsage(); - - active.insert(blockid); - } - } - } - } + InspectBlocks( *tracks, + BlockSpaceUsageAccumulator( current ), + &active // Visit unique blocks only + ); // Get the number of blocks and total length from the project file. unsigned long long blockcount = 0; @@ -1868,7 +1840,7 @@ bool ProjectFileIO::LoadProject(const FilePath &fileName) // Check for orphans blocks...sets mRecovered if any were deleted if (blockids.size() > 0) { - if (!CheckForOrphans(blockids)) + if (!DeleteBlocks(blockids, true)) { return false; } @@ -2176,36 +2148,30 @@ AutoCommitTransaction::AutoCommitTransaction(ProjectFileIO &projectFileIO, mName(name) { mInTrans = mIO.TransactionStart(mName); - // Must throw + if ( !mInTrans ) + // To do, improve the message + throw SimpleMessageBoxException( XO("Database error") ); } AutoCommitTransaction::~AutoCommitTransaction() { if (mInTrans) { - // Can't check return status...should probably throw an exception here - if (!Commit()) + if (!mIO.TransactionRollback(mName)) { - // must throw + // Do not throw from a destructor! + // This has to be a no-fail cleanup that does the best that it can. } } } bool AutoCommitTransaction::Commit() { - wxASSERT(mInTrans); + if ( !mInTrans ) + // Misuse of this class + THROW_INCONSISTENCY_EXCEPTION; mInTrans = !mIO.TransactionCommit(mName); return mInTrans; } - -bool AutoCommitTransaction::Rollback() -{ - wxASSERT(mInTrans); - - mInTrans = !mIO.TransactionCommit(mName); - - return mInTrans; -} - diff --git a/src/ProjectFileIO.h b/src/ProjectFileIO.h index f3a800192..3a0c8eed9 100644 --- a/src/ProjectFileIO.h +++ b/src/ProjectFileIO.h @@ -12,7 +12,7 @@ Paul Licameli split from AudacityProject.h #define __AUDACITY_PROJECT_FILE_IO__ #include -#include +#include #include "ClientData.h" // to inherit #include "Prefs.h" // to inherit @@ -166,12 +166,15 @@ private: bool WriteDoc(const char *table, const ProjectSerializer &autosave, const char *schema = "main"); // Application defined function to verify blockid exists is in set of blockids - using BlockIDs = std::set; + using BlockIDs = std::unordered_set; static void InSet(sqlite3_context *context, int argc, sqlite3_value **argv); - // Checks for orphan blocks. This will go away at a future date - bool CheckForOrphans(BlockIDs &blockids); +public: + // In one SQL command, delete sample blocks with ids in the given set, or + // (when complement is true), with ids not in the given set. + bool DeleteBlocks(const BlockIDs &blockids, bool complement); +private: // Return a database connection if successful, which caller must close Connection CopyTo(const FilePath &destpath, const TranslatableString &msg, @@ -213,10 +216,12 @@ private: TranslatableString mLastError; TranslatableString mLibraryError; - - friend AutoCommitTransaction; }; +// Make a savepoint (a transaction, possibly nested) with the given name; +// roll it back at destruction time, unless an explicit Commit() happened first. +// Commit() must not be called again after one successful call. +// An exception is thrown from the constructor if the transaction cannot open. class AutoCommitTransaction { public: @@ -224,7 +229,6 @@ public: ~AutoCommitTransaction(); bool Commit(); - bool Rollback(); private: ProjectFileIO &mIO; diff --git a/src/ProjectManager.cpp b/src/ProjectManager.cpp index 583a0f008..629b08bb2 100644 --- a/src/ProjectManager.cpp +++ b/src/ProjectManager.cpp @@ -748,6 +748,8 @@ void ProjectManager::OnCloseWindow(wxCloseEvent & event) // Delete all the tracks to free up memory tracks.Clear(); + + trans.Commit(); } // We're all done with the project file, so close it now diff --git a/src/ProjectSerializer.cpp b/src/ProjectSerializer.cpp index 6f1efb398..adfcec24c 100644 --- a/src/ProjectSerializer.cpp +++ b/src/ProjectSerializer.cpp @@ -382,7 +382,7 @@ bool ProjectSerializer::DictChanged() const return mDictChanged; } -// See ProjectFileIO::CheckForOrphans() for explanation of the blockids arg +// See ProjectFileIO::LoadProject() for explanation of the blockids arg wxString ProjectSerializer::Decode(const wxMemoryBuffer &buffer, BlockIDs &blockids) { wxMemoryInputStream in(buffer.GetData(), buffer.GetDataLen()); diff --git a/src/ProjectSerializer.h b/src/ProjectSerializer.h index c17089637..5e561216f 100644 --- a/src/ProjectSerializer.h +++ b/src/ProjectSerializer.h @@ -15,7 +15,7 @@ #include // member variables -#include +#include #include #include "audacity/Types.h" @@ -23,7 +23,7 @@ using SampleBlockID = long long; // From ProjectFileiIO.h -using BlockIDs = std::set; +using BlockIDs = std::unordered_set; /// /// ProjectSerializer diff --git a/src/SampleBlock.h b/src/SampleBlock.h index 5d22e50be..42b713a9b 100644 --- a/src/SampleBlock.h +++ b/src/SampleBlock.h @@ -47,7 +47,7 @@ public: virtual void CloseLock() = 0; - virtual SampleBlockID GetBlockID() = 0; + virtual SampleBlockID GetBlockID() const = 0; // If !mayThrow and there is an error, ignores it and returns zero. // That may be appropriate when only attempting to display samples, not edit. @@ -89,6 +89,15 @@ protected: virtual MinMaxRMS DoGetMinMaxRMS() const = 0; }; +// Makes a useful function object +inline std::function< void(const SampleBlock&) > +BlockSpaceUsageAccumulator (unsigned long long &total) +{ + return [&total]( const SampleBlock &block ){ + total += block.GetSpaceUsage(); + }; +}; + ///\brief abstract base class with methods to produce @ref SampleBlock objects class SampleBlockFactory { diff --git a/src/SqliteSampleBlock.cpp b/src/SqliteSampleBlock.cpp index 081f43b2e..19bb4164b 100644 --- a/src/SqliteSampleBlock.cpp +++ b/src/SqliteSampleBlock.cpp @@ -36,7 +36,7 @@ public: void Delete(); - SampleBlockID GetBlockID() override; + SampleBlockID GetBlockID() const override; size_t DoGetSamples(samplePtr dest, sampleFormat destformat, @@ -284,7 +284,7 @@ void SqliteSampleBlock::CloseLock() mLocked = true; } -SampleBlockID SqliteSampleBlock::GetBlockID() +SampleBlockID SqliteSampleBlock::GetBlockID() const { return mBlockID; } diff --git a/src/UndoManager.cpp b/src/UndoManager.cpp index 2db73aebf..1bb325418 100644 --- a/src/UndoManager.cpp +++ b/src/UndoManager.cpp @@ -45,7 +45,6 @@ wxDEFINE_EVENT(EVT_UNDO_OR_REDO, wxCommandEvent); wxDEFINE_EVENT(EVT_UNDO_RESET, wxCommandEvent); using SampleBlockID = long long; -using Set = std::unordered_set; struct UndoStackElem { @@ -94,35 +93,15 @@ UndoManager::~UndoManager() namespace { SpaceArray::value_type - CalculateUsage(const TrackList &tracks, Set &seen) + CalculateUsage(const TrackList &tracks, SampleBlockIDSet &seen) { SpaceArray::value_type result = 0; - //TIMER_START( "CalculateSpaceUsage", space_calc ); - for (auto wt : tracks.Any< const WaveTrack >()) - { - // Scan all clips within current track - for(const auto &clip : wt->GetAllClips()) - { - // Scan all sample blocks within current clip - auto blocks = clip->GetSequenceBlockArray(); - for (const auto &block : *blocks) - { - const auto &sb = block.sb; - - // Accumulate space used by the block if the block was not - // yet seen - if ( seen.count( sb->GetBlockID() ) == 0 ) - { - unsigned long long usage{ sb->GetSpaceUsage() }; - result += usage; - - seen.insert( sb->GetBlockID() ); - } - } - } - } - + InspectBlocks( + tracks, + BlockSpaceUsageAccumulator( result ), + &seen + ); return result; } } @@ -132,7 +111,7 @@ void UndoManager::CalculateSpaceUsage() space.clear(); space.resize(stack.size(), 0); - Set seen; + SampleBlockIDSet seen; // After copies and pastes, a block file may be used in more than // one place in one undo history state, and it may be used in more than @@ -157,9 +136,9 @@ void UndoManager::CalculateSpaceUsage() // Count the usage of the clipboard separately, using another set. Do not // multiple-count any block occurring multiple times within the clipboard. - Set seen2; + seen.clear(); mClipboardSpaceUsage = CalculateUsage( - Clipboard::Get().GetTracks(), seen2); + Clipboard::Get().GetTracks(), seen); //TIMER_STOP( space_calc ); } @@ -167,8 +146,6 @@ void UndoManager::CalculateSpaceUsage() wxLongLong_t UndoManager::GetLongDescription( unsigned int n, TranslatableString *desc, TranslatableString *size) { - n -= 1; // 1 based to zero based - wxASSERT(n < stack.size()); wxASSERT(space.size() == stack.size()); @@ -181,8 +158,6 @@ wxLongLong_t UndoManager::GetLongDescription( void UndoManager::GetShortDescription(unsigned int n, TranslatableString *desc) { - n -= 1; // 1 based to zero based - wxASSERT(n < stack.size()); *desc = stack[n]->shortDescription; @@ -228,7 +203,7 @@ unsigned int UndoManager::GetNumStates() unsigned int UndoManager::GetCurrentState() { - return current + 1; // the array is 0 based, the abstraction is 1 based + return current; } bool UndoManager::UndoAvailable() @@ -332,8 +307,6 @@ void UndoManager::PushState(const TrackList * l, void UndoManager::SetStateTo(unsigned int n, const Consumer &consumer) { - n -= 1; - wxASSERT(n < stack.size()); current = n; diff --git a/src/UndoManager.h b/src/UndoManager.h index e76c19756..8615ab76a 100644 --- a/src/UndoManager.h +++ b/src/UndoManager.h @@ -131,7 +131,6 @@ class AUDACITY_DLL_API UndoManager final const SelectedRegion &selectedRegion, const std::shared_ptr &tags); void ClearStates(); void RemoveStates(int num); // removes the 'num' oldest states - void RemoveStateAt(int n); // removes the n'th state (1 is oldest) unsigned int GetNumStates(); unsigned int GetCurrentState(); @@ -168,6 +167,8 @@ class AUDACITY_DLL_API UndoManager final // void Debug(); // currently unused private: + void RemoveStateAt(int n); + AudacityProject &mProject; int current; diff --git a/src/WaveTrack.cpp b/src/WaveTrack.cpp index fc1fdd46a..fd8c829fa 100644 --- a/src/WaveTrack.cpp +++ b/src/WaveTrack.cpp @@ -2683,3 +2683,32 @@ void WaveTrack::AllClipsIterator::push( WaveClipHolders &clips ) pClips = &(*first)->GetCutLines(); } } + +#include "SampleBlock.h" +void VisitBlocks(TrackList &tracks, BlockVisitor visitor, + SampleBlockIDSet *pIDs) +{ + for (auto wt : tracks.Any< const WaveTrack >()) { + // Scan all clips within current track + for(const auto &clip : wt->GetAllClips()) { + // Scan all sample blocks within current clip + auto blocks = clip->GetSequenceBlockArray(); + for (const auto &block : *blocks) { + auto &pBlock = block.sb; + if ( pBlock ) { + if ( pIDs && !pIDs->insert(pBlock->GetBlockID()).second ) + continue; + if ( visitor ) + visitor( *pBlock ); + } + } + } + } +} + +void InspectBlocks(const TrackList &tracks, BlockInspector inspector, + SampleBlockIDSet *pIDs) +{ + VisitBlocks( + const_cast(tracks), std::move( inspector ), pIDs ); +} diff --git a/src/WaveTrack.h b/src/WaveTrack.h index 64485f86e..6db777703 100644 --- a/src/WaveTrack.h +++ b/src/WaveTrack.h @@ -626,4 +626,23 @@ private: int mNValidBuffers; }; +#include +class SampleBlock; +using SampleBlockID = long long; +using SampleBlockIDSet = std::unordered_set; +class TrackList; +using BlockVisitor = std::function< void(SampleBlock&) >; +using BlockInspector = std::function< void(const SampleBlock&) >; + +// Function to visit all sample blocks from a list of tracks. +// If a set is supplied, then only visit once each unique block ID not already +// in that set, and accumulate those into the set as a side-effect. +// The visitor function may be null. +void VisitBlocks(TrackList &tracks, BlockVisitor visitor, + SampleBlockIDSet *pIDs = nullptr); + +// Non-mutating version of the above +void InspectBlocks(const TrackList &tracks, BlockInspector inspector, + SampleBlockIDSet *pIDs = nullptr); + #endif // __AUDACITY_WAVETRACK__ diff --git a/src/effects/Effect.cpp b/src/effects/Effect.cpp index 1c4b9796e..ded4443e4 100644 --- a/src/effects/Effect.cpp +++ b/src/effects/Effect.cpp @@ -1251,6 +1251,8 @@ bool Effect::DoEffect(double projectRate, // LastUsedDuration may have been modified by Preview. SetDuration(oldDuration); } + else + trans.Commit(); End(); ReplaceProcessedTracks( false );