diff --git a/src/AutoRecovery.cpp b/src/AutoRecovery.cpp index 5a55f6ee0..381bee079 100644 --- a/src/AutoRecovery.cpp +++ b/src/AutoRecovery.cpp @@ -280,7 +280,7 @@ wxString AutoSaveFile::Decode(const wxMemoryBuffer &buffer) mIds.clear(); - struct Error{}; + struct Error{}; // exception type for short-range try/catch auto Lookup = [&mIds]( short id ) -> const wxString & { auto iter = mIds.find( id ); @@ -318,7 +318,7 @@ wxString AutoSaveFile::Decode(const wxMemoryBuffer &buffer) return str; }; - while (!in.Eof()) + try { while (!in.Eof()) { short id; @@ -495,6 +495,10 @@ wxString AutoSaveFile::Decode(const wxMemoryBuffer &buffer) wxASSERT(true); break; } + } } catch( const Error& ) { + // Autosave was corrupt, or platform differences in size or endianness + // were not well canonicalized + return {}; } return out; diff --git a/src/AutoRecovery.h b/src/AutoRecovery.h index 03023d12d..a553dbef2 100644 --- a/src/AutoRecovery.h +++ b/src/AutoRecovery.h @@ -61,6 +61,7 @@ public: bool IsEmpty() const; bool DictChanged() const; + // Returns empty string if decoding fails static wxString Decode(const wxMemoryBuffer &buffer); private: diff --git a/src/SampleBlock.cpp b/src/SampleBlock.cpp index f83f5c4f9..1899631bd 100644 --- a/src/SampleBlock.cpp +++ b/src/SampleBlock.cpp @@ -8,6 +8,7 @@ SampleBlock.cpp #include "Audacity.h" #include "SampleBlock.h" +#include "SampleFormat.h" #include @@ -36,5 +37,78 @@ SampleBlockFactoryPtr SampleBlockFactory::New( AudacityProject &project ) SampleBlockFactory::~SampleBlockFactory() = default; +SampleBlockPtr SampleBlockFactory::Get(SampleBlockID sbid) +{ + auto result = DoGet(sbid); + if (!result) + THROW_INCONSISTENCY_EXCEPTION; + return result; +} + +SampleBlockPtr SampleBlockFactory::Create(samplePtr src, + size_t numsamples, + sampleFormat srcformat) +{ + auto result = DoCreate(src, numsamples, srcformat); + if (!result) + THROW_INCONSISTENCY_EXCEPTION; + return result; +} + +SampleBlockPtr SampleBlockFactory::CreateSilent( + size_t numsamples, + sampleFormat srcformat) +{ + auto result = DoCreateSilent(numsamples, srcformat); + if (!result) + THROW_INCONSISTENCY_EXCEPTION; + return result; +} + +SampleBlockPtr SampleBlockFactory::CreateFromXML( + sampleFormat srcformat, + const wxChar **attrs) +{ + auto result = DoCreateFromXML(srcformat, attrs); + if (!result) + THROW_INCONSISTENCY_EXCEPTION; + return result; +} + SampleBlock::~SampleBlock() = default; +size_t SampleBlock::GetSamples(samplePtr dest, + sampleFormat destformat, + size_t sampleoffset, + size_t numsamples, bool mayThrow) +{ + try{ return DoGetSamples(dest, destformat, sampleoffset, numsamples); } + catch( ... ) { + if( mayThrow ) + throw; + ClearSamples( dest, destformat, 0, numsamples ); + return 0; + } +} + + MinMaxRMS SampleBlock::GetMinMaxRMS( + size_t start, size_t len, bool mayThrow) +{ + try{ return DoGetMinMaxRMS(start, len); } + catch( ... ) { + if( mayThrow ) + throw; + return {}; + } +} + + MinMaxRMS SampleBlock::GetMinMaxRMS(bool mayThrow) const +{ + try{ return DoGetMinMaxRMS(); } + catch( ... ) { + if( mayThrow ) + throw; + return {}; + } +} + diff --git a/src/SampleBlock.h b/src/SampleBlock.h index 38b056aae..8cec62e24 100644 --- a/src/SampleBlock.h +++ b/src/SampleBlock.h @@ -31,9 +31,9 @@ using SampleBlockID = long long; class MinMaxRMS { public: - float min; - float max; - float RMS; + float min = 0; + float max = 0; + float RMS = 0; }; class SqliteSampleBlockFactory; @@ -52,10 +52,12 @@ public: virtual SampleBlockID GetBlockID() = 0; - virtual size_t GetSamples(samplePtr dest, + // If !mayThrow and there is an error, ignores it and returns zero. + // That may be appropriate when only attempting to display samples, not edit. + size_t GetSamples(samplePtr dest, sampleFormat destformat, size_t sampleoffset, - size_t numsamples) = 0; + size_t numsamples, bool mayThrow = true); virtual size_t GetSampleCount() const = 0; @@ -65,14 +67,29 @@ public: GetSummary64k(float *dest, size_t frameoffset, size_t numframes) = 0; /// Gets extreme values for the specified region - virtual MinMaxRMS GetMinMaxRMS(size_t start, size_t len) = 0; + // If !mayThrow and there is an error, ignores it and returns zeroes. + // That may be appropriate when only attempting to display samples, not edit. + MinMaxRMS GetMinMaxRMS( + size_t start, size_t len, bool mayThrow = true); /// Gets extreme values for the entire block - virtual MinMaxRMS GetMinMaxRMS() const = 0; + // If !mayThrow and there is an error, ignores it and returns zeroes. + // That may be appropriate when only attempting to display samples, not edit. + MinMaxRMS GetMinMaxRMS(bool mayThrow = true) const; virtual size_t GetSpaceUsage() const = 0; virtual void SaveXML(XMLWriter &xmlFile) = 0; + +protected: + virtual size_t DoGetSamples(samplePtr dest, + sampleFormat destformat, + size_t sampleoffset, + size_t numsamples) = 0; + + virtual MinMaxRMS DoGetMinMaxRMS(size_t start, size_t len) = 0; + + virtual MinMaxRMS DoGetMinMaxRMS() const = 0; }; ///\brief abstract base class with methods to produce @ref SampleBlock objects @@ -90,17 +107,44 @@ public: virtual ~SampleBlockFactory(); - virtual SampleBlockPtr Get(SampleBlockID sbid) = 0; + // Returns a non-null pointer or else throws an exception + SampleBlockPtr Get(SampleBlockID sbid); - virtual SampleBlockPtr Create(samplePtr src, + // Returns a non-null pointer or else throws an exception + SampleBlockPtr Create(samplePtr src, + size_t numsamples, + sampleFormat srcformat); + + // Returns a non-null pointer or else throws an exception + SampleBlockPtr CreateSilent( + size_t numsamples, + sampleFormat srcformat); + + // Returns a non-null pointer or else throws an exception + SampleBlockPtr CreateFromXML( + sampleFormat srcformat, + const wxChar **attrs); + +protected: + // The override should throw more informative exceptions on error than the + // default InconsistencyException thrown by Create + virtual SampleBlockPtr DoGet(SampleBlockID sbid) = 0; + + // The override should throw more informative exceptions on error than the + // default InconsistencyException thrown by Create + virtual SampleBlockPtr DoCreate(samplePtr src, size_t numsamples, sampleFormat srcformat) = 0; - virtual SampleBlockPtr CreateSilent( + // The override should throw more informative exceptions on error than the + // default InconsistencyException thrown by CreateSilent + virtual SampleBlockPtr DoCreateSilent( size_t numsamples, sampleFormat srcformat) = 0; - virtual SampleBlockPtr CreateFromXML( + // The override should throw more informative exceptions on error than the + // default InconsistencyException thrown by CreateFromXML + virtual SampleBlockPtr DoCreateFromXML( sampleFormat srcformat, const wxChar **attrs) = 0; }; diff --git a/src/Sequence.cpp b/src/Sequence.cpp index 1cb184b3c..e54e220d6 100644 --- a/src/Sequence.cpp +++ b/src/Sequence.cpp @@ -255,7 +255,7 @@ std::pair Sequence::GetMinMax( // already in memory. for (unsigned b = block0 + 1; b < block1; ++b) { - auto results = mBlock[b].sb->GetMinMaxRMS(); + auto results = mBlock[b].sb->GetMinMaxRMS(mayThrow); if (results.min < min) min = results.min; @@ -270,7 +270,7 @@ std::pair Sequence::GetMinMax( { const SeqBlock &theBlock = mBlock[block0]; const auto &theFile = theBlock.sb; - auto results = theFile->GetMinMaxRMS(); + auto results = theFile->GetMinMaxRMS(mayThrow); if (results.min < min || results.max > max) { // start lies within theBlock: @@ -282,7 +282,7 @@ std::pair Sequence::GetMinMax( wxASSERT(maxl0 <= mMaxSamples); // Vaughan, 2011-10-19 const auto l0 = limitSampleBufferSize ( maxl0, len ); - results = theFile->GetMinMaxRMS(s0, l0); + results = theFile->GetMinMaxRMS(s0, l0, mayThrow); if (results.min < min) min = results.min; if (results.max > max) @@ -294,7 +294,7 @@ std::pair Sequence::GetMinMax( { const SeqBlock &theBlock = mBlock[block1]; const auto &theFile = theBlock.sb; - auto results = theFile->GetMinMaxRMS(); + auto results = theFile->GetMinMaxRMS(mayThrow); if (results.min < min || results.max > max) { @@ -302,7 +302,7 @@ std::pair Sequence::GetMinMax( const auto l0 = ( start + len - theBlock.start ).as_size_t(); wxASSERT(l0 <= mMaxSamples); // Vaughan, 2011-10-19 - results = theFile->GetMinMaxRMS(0, l0); + results = theFile->GetMinMaxRMS(0, l0, mayThrow); if (results.min < min) min = results.min; if (results.max > max) @@ -332,7 +332,7 @@ float Sequence::GetRMS(sampleCount start, sampleCount len, bool mayThrow) const for (unsigned b = block0 + 1; b < block1; b++) { const SeqBlock &theBlock = mBlock[b]; const auto &sb = theBlock.sb; - auto results = sb->GetMinMaxRMS(); + auto results = sb->GetMinMaxRMS(mayThrow); const auto fileLen = sb->GetSampleCount(); const auto blockRMS = results.RMS; @@ -354,7 +354,7 @@ float Sequence::GetRMS(sampleCount start, sampleCount len, bool mayThrow) const wxASSERT(maxl0 <= mMaxSamples); // Vaughan, 2011-10-19 const auto l0 = limitSampleBufferSize( maxl0, len ); - auto results = sb->GetMinMaxRMS(s0, l0); + auto results = sb->GetMinMaxRMS(s0, l0, mayThrow); const auto partialRMS = results.RMS; sumsq += partialRMS * partialRMS * l0; length += l0; @@ -368,7 +368,7 @@ float Sequence::GetRMS(sampleCount start, sampleCount len, bool mayThrow) const const auto l0 = ( start + len - theBlock.start ).as_size_t(); wxASSERT(l0 <= mMaxSamples); // PRL: I think Vaughan missed this - auto results = sb->GetMinMaxRMS(0, l0); + auto results = sb->GetMinMaxRMS(0, l0, mayThrow); const auto partialRMS = results.RMS; sumsq += partialRMS * partialRMS * l0; length += l0; @@ -1036,7 +1036,7 @@ bool Sequence::Read(samplePtr buffer, sampleFormat format, wxASSERT(blockRelativeStart + len <= sb->GetSampleCount()); // Either throws, or of !mayThrow, tells how many were really read - auto result = sb->GetSamples(buffer, format, blockRelativeStart, len); + auto result = sb->GetSamples(buffer, format, blockRelativeStart, len, mayThrow); if (result != len) { diff --git a/src/SqliteSampleBlock.cpp b/src/SqliteSampleBlock.cpp index 79a7b9404..9e821ef89 100644 --- a/src/SqliteSampleBlock.cpp +++ b/src/SqliteSampleBlock.cpp @@ -29,17 +29,17 @@ public: void Unlock() override; void CloseLock() override; - bool SetSamples(samplePtr src, size_t numsamples, sampleFormat srcformat); + void SetSamples(samplePtr src, size_t numsamples, sampleFormat srcformat); - bool SetSilent(size_t numsamples, sampleFormat srcformat); + void SetSilent(size_t numsamples, sampleFormat srcformat); - bool Commit(); + void Commit(); void Delete(); SampleBlockID GetBlockID() override; - size_t GetSamples(samplePtr dest, + size_t DoGetSamples(samplePtr dest, sampleFormat destformat, size_t sampleoffset, size_t numsamples) override; @@ -53,10 +53,10 @@ public: double GetSumRms() const; /// Gets extreme values for the specified region - MinMaxRMS GetMinMaxRMS(size_t start, size_t len) override; + MinMaxRMS DoGetMinMaxRMS(size_t start, size_t len) override; /// Gets extreme values for the entire block - MinMaxRMS GetMinMaxRMS() const override; + MinMaxRMS DoGetMinMaxRMS() const override; size_t GetSpaceUsage() const override; void SaveXML(XMLWriter &xmlFile) override; @@ -116,17 +116,17 @@ public: ~SqliteSampleBlockFactory() override; - SampleBlockPtr Get(SampleBlockID sbid) override; + SampleBlockPtr DoGet(SampleBlockID sbid) override; - SampleBlockPtr Create(samplePtr src, + SampleBlockPtr DoCreate(samplePtr src, size_t numsamples, sampleFormat srcformat) override; - SampleBlockPtr CreateSilent( + SampleBlockPtr DoCreateSilent( size_t numsamples, sampleFormat srcformat) override; - SampleBlockPtr CreateFromXML( + SampleBlockPtr DoCreateFromXML( sampleFormat srcformat, const wxChar **attrs) override; @@ -144,40 +144,24 @@ SqliteSampleBlockFactory::SqliteSampleBlockFactory( AudacityProject &project ) SqliteSampleBlockFactory::~SqliteSampleBlockFactory() = default; -SampleBlockPtr SqliteSampleBlockFactory::Create( +SampleBlockPtr SqliteSampleBlockFactory::DoCreate( samplePtr src, size_t numsamples, sampleFormat srcformat ) { auto sb = std::make_shared(&mProject); - - if (sb) - { - if (sb->SetSamples(src, numsamples, srcformat)) - { - return sb; - } - } - - return nullptr; + sb->SetSamples(src, numsamples, srcformat); + return sb; } -SampleBlockPtr SqliteSampleBlockFactory::CreateSilent( +SampleBlockPtr SqliteSampleBlockFactory::DoCreateSilent( size_t numsamples, sampleFormat srcformat ) { auto sb = std::make_shared(&mProject); - - if (sb) - { - if (sb->SetSilent(numsamples, srcformat)) - { - return sb; - } - } - - return nullptr; + sb->SetSilent(numsamples, srcformat); + return sb; } -SampleBlockPtr SqliteSampleBlockFactory::CreateFromXML( +SampleBlockPtr SqliteSampleBlockFactory::DoCreateFromXML( sampleFormat srcformat, const wxChar **attrs ) { auto sb = std::make_shared(&mProject); @@ -204,10 +188,8 @@ SampleBlockPtr SqliteSampleBlockFactory::CreateFromXML( { if (wxStrcmp(attr, wxT("blockid")) == 0) { - if (!sb->Load((SampleBlockID) nValue)) - { - return nullptr; - } + // This may throw + sb->Load((SampleBlockID) nValue); found++; } else if (wxStrcmp(attr, wxT("samplecount")) == 0) @@ -246,18 +228,10 @@ SampleBlockPtr SqliteSampleBlockFactory::CreateFromXML( return sb; } -SampleBlockPtr SqliteSampleBlockFactory::Get( SampleBlockID sbid ) +SampleBlockPtr SqliteSampleBlockFactory::DoGet( SampleBlockID sbid ) { auto sb = std::make_shared(&mProject); - - if (sb) - { - if (!sb->Load(sbid)) - { - return nullptr; - } - } - + sb->Load(sbid); return sb; } @@ -286,7 +260,12 @@ SqliteSampleBlock::~SqliteSampleBlock() // See ProjectFileIO::Bypass() for a description of mIO.mBypass if (mRefCnt == 0 && !mIO.ShouldBypass()) { - Delete(); + // In case Delete throws, don't let an exception escape a destructor, + // but we can still enqueue the delayed handler so that an error message + // is presented to the user. + // The failure in this case may be a less harmful waste of space in the + // database, which should not cause aborting of the attempted edit. + GuardedCall( [this]{ Delete(); } ); } } @@ -320,7 +299,7 @@ size_t SqliteSampleBlock::GetSampleCount() const return mSampleCount; } -size_t SqliteSampleBlock::GetSamples(samplePtr dest, +size_t SqliteSampleBlock::DoGetSamples(samplePtr dest, sampleFormat destformat, size_t sampleoffset, size_t numsamples) @@ -333,7 +312,7 @@ size_t SqliteSampleBlock::GetSamples(samplePtr dest, numsamples * SAMPLE_SIZE(mSampleFormat)) / SAMPLE_SIZE(mSampleFormat); } -bool SqliteSampleBlock::SetSamples(samplePtr src, +void SqliteSampleBlock::SetSamples(samplePtr src, size_t numsamples, sampleFormat srcformat) { @@ -346,10 +325,10 @@ bool SqliteSampleBlock::SetSamples(samplePtr src, CalcSummary(); - return Commit(); + Commit(); } -bool SqliteSampleBlock::SetSilent(size_t numsamples, sampleFormat srcformat) +void SqliteSampleBlock::SetSilent(size_t numsamples, sampleFormat srcformat) { mSampleFormat = srcformat; @@ -362,7 +341,7 @@ bool SqliteSampleBlock::SetSilent(size_t numsamples, sampleFormat srcformat) mSilent = true; - return Commit(); + Commit(); } bool SqliteSampleBlock::GetSummary256(float *dest, @@ -413,7 +392,7 @@ double SqliteSampleBlock::GetSumRms() const /// /// @param start The offset in this block where the region should begin /// @param len The number of samples to include in the region -MinMaxRMS SqliteSampleBlock::GetMinMaxRMS(size_t start, size_t len) +MinMaxRMS SqliteSampleBlock::DoGetMinMaxRMS(size_t start, size_t len) { float min = FLT_MAX; float max = -FLT_MAX; @@ -462,7 +441,7 @@ MinMaxRMS SqliteSampleBlock::GetMinMaxRMS(size_t start, size_t len) /// Retrieves the minimum, maximum, and maximum RMS of this entire /// block. This is faster than the other GetMinMax function since /// these values are already computed. -MinMaxRMS SqliteSampleBlock::GetMinMaxRMS() const +MinMaxRMS SqliteSampleBlock::DoGetMinMaxRMS() const { return { (float) mSumMin, (float) mSumMax, (float) mSumRms }; } @@ -541,6 +520,11 @@ size_t SqliteSampleBlock::GetBlob(void *dest, } } + if ( rc != SQLITE_ROW ) + // Just showing the user a simple message, not the library error too + // which isn't internationalized + throw SimpleMessageBoxException{ XO("Failed to retrieve samples") }; + if (srcbytes - minbytes) { memset(dest, 0, srcbytes - minbytes); @@ -587,17 +571,19 @@ bool SqliteSampleBlock::Load(SampleBlockID sbid) if (rc != SQLITE_OK) { wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db)); - // handle error - return false; + } + else { + rc = sqlite3_step(stmt); + if (rc != SQLITE_ROW) + { + wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db)); + } } - rc = sqlite3_step(stmt); - if (rc != SQLITE_ROW) - { - wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db)); - // handle error - return false; - } + if ( rc != SQLITE_ROW ) + // Just showing the user a simple message, not the library error too + // which isn't internationalized + throw SimpleMessageBoxException{ XO("Failed to retrieve samples") }; mBlockID = sbid; mSampleFormat = (sampleFormat) sqlite3_column_int(stmt, 0); @@ -610,11 +596,9 @@ bool SqliteSampleBlock::Load(SampleBlockID sbid) mSampleCount = mSampleBytes / SAMPLE_SIZE(mSampleFormat); mValid = true; - - return true; } -bool SqliteSampleBlock::Commit() +void SqliteSampleBlock::Commit() { auto db = mIO.DB(); int rc; @@ -638,25 +622,32 @@ bool SqliteSampleBlock::Commit() if (rc != SQLITE_OK) { wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db)); - // handle error - return false; + // Just showing the user a simple message, not the library error too + // which isn't internationalized + throw SimpleMessageBoxException{ mIO.GetLastError() }; } // BIND SQL sampleblocks - sqlite3_bind_int(stmt, 1, mSampleFormat); - sqlite3_bind_double(stmt, 2, mSumMin); - sqlite3_bind_double(stmt, 3, mSumMax); - sqlite3_bind_double(stmt, 4, mSumRms); - sqlite3_bind_blob(stmt, 5, mSummary256.get(), mSummary256Bytes, SQLITE_STATIC); - sqlite3_bind_blob(stmt, 6, mSummary64k.get(), mSummary64kBytes, SQLITE_STATIC); - sqlite3_bind_blob(stmt, 7, mSamples.get(), mSampleBytes, SQLITE_STATIC); + // Might return SQL_MISUSE which means it's our mistake that we violated + // preconditions; should return SQL_OK which is 0 + if ( + sqlite3_bind_int(stmt, 1, mSampleFormat) || + sqlite3_bind_double(stmt, 2, mSumMin) || + sqlite3_bind_double(stmt, 3, mSumMax) || + sqlite3_bind_double(stmt, 4, mSumRms) || + sqlite3_bind_blob(stmt, 5, mSummary256.get(), mSummary256Bytes, SQLITE_STATIC) || + sqlite3_bind_blob(stmt, 6, mSummary64k.get(), mSummary64kBytes, SQLITE_STATIC) || + sqlite3_bind_blob(stmt, 7, mSamples.get(), mSampleBytes, SQLITE_STATIC) + ) + THROW_INCONSISTENCY_EXCEPTION; rc = sqlite3_step(stmt); if (rc != SQLITE_DONE) { wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db)); - // handle error - return false; + // Just showing the user a simple message, not the library error too + // which isn't internationalized + throw SimpleMessageBoxException{ mIO.GetLastError() }; } mBlockID = sqlite3_last_insert_rowid(db); @@ -666,8 +657,6 @@ bool SqliteSampleBlock::Commit() mSummary64k.reset(); mValid = true; - - return true; } void SqliteSampleBlock::Delete() @@ -688,8 +677,9 @@ void SqliteSampleBlock::Delete() if (rc != SQLITE_OK) { wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db)); - // handle error - return; + // Just showing the user a simple message, not the library error too + // which isn't internationalized + throw SimpleMessageBoxException{ XO("Failed to purge unused samples") }; } } }