From 6b0c5c096b01bc280435d711d5459b24df29582a Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 2 Apr 2017 21:19:12 -0400 Subject: [PATCH 1/8] WaveClip::Flush returns void, throws on error --- src/WaveClip.cpp | 4 +--- src/WaveClip.h | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index ac78d0511..d4a6855ff 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -1471,7 +1471,7 @@ if (result) } } -bool WaveClip::Flush() +void WaveClip::Flush() // NOFAIL-GUARANTEE that the clip will be in a flushed state. // PARTIAL-GUARANTEE in case of exceptions: // Some initial portion (maybe none) of the append buffer of the @@ -1498,8 +1498,6 @@ bool WaveClip::Flush() } //wxLogDebug(wxT("now sample count %lli"), (long long) mSequence->GetNumSamples()); - - return success; } bool WaveClip::HandleXMLTag(const wxChar *tag, const wxChar **attrs) diff --git a/src/WaveClip.h b/src/WaveClip.h index 3d1fef9fe..1c45da356 100644 --- a/src/WaveClip.h +++ b/src/WaveClip.h @@ -301,7 +301,7 @@ public: size_t len, unsigned int stride=1, XMLWriter* blockFileLog = NULL); /// Flush must be called after last Append - bool Flush(); + void Flush(); void AppendAlias(const wxString &fName, sampleCount start, size_t len, int channel,bool useOD); From 2ba6065961729a6ce9f5a7a666cbda09408559de Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Mon, 28 Nov 2016 09:50:09 -0500 Subject: [PATCH 2/8] Sequence consistency check throws; new unused commit functions use it --- src/AutoRecovery.cpp | 3 +- src/Benchmark.cpp | 3 +- src/Sequence.cpp | 136 ++++++++++++++++++++++++++++++++++--------- src/Sequence.h | 29 +++++++-- 4 files changed, 137 insertions(+), 34 deletions(-) diff --git a/src/AutoRecovery.cpp b/src/AutoRecovery.cpp index 4d7471fa1..9bb836546 100644 --- a/src/AutoRecovery.cpp +++ b/src/AutoRecovery.cpp @@ -411,7 +411,8 @@ void RecordingRecoveryHandler::HandleXMLEndTag(const wxChar *tag) WaveClip* clip = track->NewestOrNewClip(); Sequence* seq = clip->GetSequence(); - seq->ConsistencyCheck(wxT("RecordingRecoveryHandler::HandleXMLEndTag")); + seq->ConsistencyCheck + (wxT("RecordingRecoveryHandler::HandleXMLEndTag"), false); } } diff --git a/src/Benchmark.cpp b/src/Benchmark.cpp index e83fe3d64..33d3174d9 100644 --- a/src/Benchmark.cpp +++ b/src/Benchmark.cpp @@ -474,7 +474,8 @@ void BenchmarkDialog::OnRun( wxCommandEvent & WXUNUSED(event)) elapsed = timer.Time(); if (mBlockDetail) { - t->GetClipByIndex(0)->GetSequence()->DebugPrintf(&tempStr); + auto seq = t->GetClipByIndex(0)->GetSequence(); + seq->DebugPrintf(seq->GetBlockArray(), seq->GetNumSamples(), &tempStr); mToPrint += tempStr; } Printf(wxT("Time to perform %d edits: %ld ms\n"), trials, elapsed); diff --git a/src/Sequence.cpp b/src/Sequence.cpp index 5058573ed..91b5acc0a 100644 --- a/src/Sequence.cpp +++ b/src/Sequence.cpp @@ -50,6 +50,8 @@ #include "blockfile/SimpleBlockFile.h" #include "blockfile/SilentBlockFile.h" +#include "InconsistencyException.h" + size_t Sequence::sMaxDiskBlockSize = 1048576; // Sequence methods @@ -222,7 +224,7 @@ bool Sequence::ConvertToSampleFormat(sampleFormat format, bool* pbChanged) *pbChanged = false; // Revert overall change flag, in case we had some partial success in the loop. } - bSuccess &= ConsistencyCheck(wxT("Sequence::ConvertToSampleFormat()")); + ConsistencyCheck(wxT("Sequence::ConvertToSampleFormat()")); return bSuccess; } @@ -437,9 +439,7 @@ std::unique_ptr Sequence::Copy(sampleCount s0, sampleCount s1) const // Increase ref count or duplicate file } - if (! ConsistencyCheck(wxT("Sequence::Copy()"))) - //THROW_INCONSISTENCY_EXCEPTION - ; + dest->ConsistencyCheck(wxT("Sequence::Copy()")); return dest; } @@ -505,7 +505,8 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) AppendBlock(*mDirManager, mBlock, mNumSamples, srcBlock[i]); // Increase ref count or duplicate file - return ConsistencyCheck(wxT("Paste branch one")); + ConsistencyCheck(wxT("Paste branch one")); + return true; } const int b = (s == mNumSamples) ? mBlock.size() - 1 : FindBlock(s); @@ -547,7 +548,8 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) mNumSamples += addedLen; - return ConsistencyCheck(wxT("Paste branch two")); + ConsistencyCheck(wxT("Paste branch two")); + return true; } // Case three: if we are inserting four or fewer blocks, @@ -639,7 +641,8 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) mNumSamples += addedLen; - return ConsistencyCheck(wxT("Paste branch three")); + ConsistencyCheck(wxT("Paste branch three")); + return true; } bool Sequence::SetSilence(sampleCount s0, sampleCount len) @@ -693,7 +696,8 @@ bool Sequence::InsertSilence(sampleCount s0, sampleCount len) bool bResult = Paste(s0, &sTrack); wxASSERT(bResult); - return bResult && ConsistencyCheck(wxT("InsertSilence")); + ConsistencyCheck(wxT("InsertSilence")); + return bResult; } bool Sequence::AppendAlias(const wxString &fullPath, @@ -1253,7 +1257,8 @@ bool Sequence::Set(samplePtr buffer, sampleFormat format, b++; } - return ConsistencyCheck(wxT("Set")); + ConsistencyCheck(wxT("Set")); + return true; } namespace { @@ -1683,7 +1688,8 @@ bool Sequence::Delete(sampleCount start, sampleCount len) mNumSamples -= len; - return ConsistencyCheck(wxT("Delete - branch one")); + ConsistencyCheck(wxT("Delete - branch one")); + return true; } // Create a NEW array of blocks @@ -1790,44 +1796,120 @@ bool Sequence::Delete(sampleCount start, sampleCount len) // Update total number of samples and do a consistency check. mNumSamples -= len; - return ConsistencyCheck(wxT("Delete - branch two")); + ConsistencyCheck(wxT("Delete - branch two")); + return true; } -bool Sequence::ConsistencyCheck(const wxChar *whereStr) const +void Sequence::ConsistencyCheck(const wxChar *whereStr, bool mayThrow) const { - unsigned int i; - sampleCount pos = 0; - unsigned int numBlocks = mBlock.size(); - bool bError = false; + ConsistencyCheck(mBlock, 0, mNumSamples, whereStr, mayThrow); +} - for (i = 0; !bError && i < numBlocks; i++) { +void Sequence::ConsistencyCheck + (const BlockArray &mBlock, size_t from, + sampleCount mNumSamples, const wxChar *whereStr, + bool mayThrow) +{ + bool bError = false; + // Construction of the exception at the appropriate line of the function + // gives a little more discrimination + InconsistencyException ex; + + unsigned int i; + sampleCount pos = mBlock[from].start; + if ( from == 0 && pos != 0 ) + ex = CONSTRUCT_INCONSISTENCY_EXCEPTION, bError = true; + + unsigned int numBlocks = mBlock.size(); + + for (i = from; !bError && i < numBlocks; i++) { const SeqBlock &seqBlock = mBlock[i]; if (pos != seqBlock.start) - bError = true; + ex = CONSTRUCT_INCONSISTENCY_EXCEPTION, bError = true; - if (seqBlock.f) + if ( seqBlock.f ) pos += seqBlock.f->GetLength(); else - bError = true; + ex = CONSTRUCT_INCONSISTENCY_EXCEPTION, bError = true; } - if (pos != mNumSamples) - bError = true; + if ( !bError && pos != mNumSamples ) + ex = CONSTRUCT_INCONSISTENCY_EXCEPTION, bError = true; - if (bError) + if ( bError ) { wxLogError(wxT("*** Consistency check failed after %s. ***"), whereStr); wxString str; - DebugPrintf(&str); + DebugPrintf(mBlock, mNumSamples, &str); wxLogError(wxT("%s"), str.c_str()); wxLogError(wxT("*** Please report this error to feedback@audacityteam.org. ***\n\n") wxT("Recommended course of action:\n") wxT("Undo the failed operation(s), then export or save your work and quit.")); - } - return !bError; + if (mayThrow) + //throw ex + ; + else + wxASSERT(false); + } } -void Sequence::DebugPrintf(wxString *dest) const +void Sequence::CommitChangesIfConsistent + (BlockArray &newBlock, sampleCount numSamples, const wxChar *whereStr) +{ + ConsistencyCheck( newBlock, 0, numSamples, whereStr ); // may throw + + // now commit + // use NOFAIL-GUARANTEE + + mBlock.swap(newBlock); + mNumSamples = numSamples; +} + +void Sequence::AppendBlocksIfConsistent +(BlockArray &additionalBlocks, bool replaceLast, + sampleCount numSamples, const wxChar *whereStr) +{ + // Any additional blocks are meant to be appended, + // replacing the final block if there was one. + + if (additionalBlocks.empty()) + return; + + bool tmpValid = false; + SeqBlock tmp; + + if ( replaceLast && ! mBlock.empty() ) { + tmp = mBlock.back(), tmpValid = true; + mBlock.pop_back(); + } + + auto prevSize = mBlock.size(); + + bool consistent = false; + auto cleanup = finally( [&] { + if ( !consistent ) { + mBlock.resize( prevSize ); + if ( tmpValid ) + mBlock.push_back( tmp ); + } + } ); + + std::copy( additionalBlocks.begin(), additionalBlocks.end(), + std::back_inserter( mBlock ) ); + + // Check consistency only of the blocks that were added, + // avoiding quadratic time for repeated checking of repeating appends + ConsistencyCheck( mBlock, prevSize, numSamples, whereStr ); // may throw + + // now commit + // use NOFAIL-GUARANTEE + + mNumSamples = numSamples; + consistent = true; +} + +void Sequence::DebugPrintf + (const BlockArray &mBlock, sampleCount mNumSamples, wxString *dest) { unsigned int i; decltype(mNumSamples) pos = 0; diff --git a/src/Sequence.h b/src/Sequence.h index ede815556..29b1da119 100644 --- a/src/Sequence.h +++ b/src/Sequence.h @@ -263,19 +263,38 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{ bool Get(int b, samplePtr buffer, sampleFormat format, sampleCount start, size_t len, bool mayThrow) const; - public: +public: // - // Public methods intended for debugging only + // Public methods // - // This function makes sure that the track isn't messed up + // This function throws if the track is messed up // because of inconsistent block starts & lengths - bool ConsistencyCheck(const wxChar *whereStr) const; + void ConsistencyCheck (const wxChar *whereStr, bool mayThrow = true) const; // This function prints information to stdout about the blocks in the // tracks and indicates if there are inconsistencies. - void DebugPrintf(wxString *dest) const; + static void DebugPrintf + (const BlockArray &block, sampleCount numSamples, wxString *dest); + +private: + static void ConsistencyCheck + (const BlockArray &block, size_t from, + sampleCount numSamples, const wxChar *whereStr, + bool mayThrow = true); + + // The next two are used in methods that give a strong guarantee. + // They either throw because final consistency check fails, or swap the + // changed contents into place. + + void CommitChangesIfConsistent + (BlockArray &newBlock, sampleCount numSamples, const wxChar *whereStr); + + void AppendBlocksIfConsistent + (BlockArray &additionalBlocks, bool replaceLast, + sampleCount numSamples, const wxChar *whereStr); + }; #endif // __AUDACITY_SEQUENCE__ From 160d846643d9e9518a393fc9006ed4d092bde7a5 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 27 Nov 2016 11:00:23 -0500 Subject: [PATCH 3/8] Sequence::Paste gives strong guarantee --- src/Sequence.cpp | 68 +++++++++++++++++++++++++----------------------- src/Sequence.h | 4 +-- src/WaveClip.cpp | 18 ++++++------- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/Sequence.cpp b/src/Sequence.cpp index 91b5acc0a..bb0df2917 100644 --- a/src/Sequence.cpp +++ b/src/Sequence.cpp @@ -72,9 +72,7 @@ Sequence::Sequence(const Sequence &orig, const std::shared_ptr &proj , mMinSamples(orig.mMinSamples) , mMaxSamples(orig.mMaxSamples) { - bool bResult = Paste(0, &orig); - wxASSERT(bResult); // TO DO: Actually handle this. - (void)bResult; + Paste(0, &orig); } Sequence::~Sequence() @@ -451,7 +449,8 @@ namespace { } } -bool Sequence::Paste(sampleCount s, const Sequence *src) +void Sequence::Paste(sampleCount s, const Sequence *src) +// STRONG-GUARANTEE { if ((s < 0) || (s > mNumSamples)) { @@ -460,8 +459,8 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) // PRL: Why bother with Internat when the above is just wxT? Internat::ToString(s.as_double(), 0).c_str(), Internat::ToString(mNumSamples.as_double(), 0).c_str()); - wxASSERT(false); - return false; + //THROW_INCONSISTENCY_EXCEPTION + ; } // Quick check to make sure that it doesn't overflow @@ -472,8 +471,8 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) // PRL: Why bother with Internat when the above is just wxT? Internat::ToString(mNumSamples.as_double(), 0).c_str(), Internat::ToString(src->mNumSamples.as_double(), 0).c_str()); - wxASSERT(false); - return false; + //THROW_INCONSISTENCY_EXCEPTION + ; } if (src->mSampleFormat != mSampleFormat) @@ -481,8 +480,8 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) wxLogError( wxT("Sequence::Paste: Sample format to be pasted, %s, does not match destination format, %s."), GetSampleFormatStr(src->mSampleFormat), GetSampleFormatStr(src->mSampleFormat)); - wxASSERT(false); - return false; + //THROW_INCONSISTENCY_EXCEPTION + ; } const BlockArray &srcBlock = src->mBlock; @@ -491,7 +490,7 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) auto sampleSize = SAMPLE_SIZE(mSampleFormat); if (addedLen == 0 || srcNumBlocks == 0) - return true; + return; const size_t numBlocks = mBlock.size(); @@ -501,12 +500,18 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) // onto the end because the current last block is longer than the // minimum size + // Build and swap a copy so there is a strong exception safety guarantee + BlockArray newBlock{ mBlock }; + sampleCount samples = mNumSamples; for (unsigned int i = 0; i < srcNumBlocks; i++) - AppendBlock(*mDirManager, mBlock, mNumSamples, srcBlock[i]); + // AppendBlock may throw for limited disk space, if pasting from + // one project into another. + AppendBlock(*mDirManager, newBlock, samples, srcBlock[i]); // Increase ref count or duplicate file - ConsistencyCheck(wxT("Paste branch one")); - return true; + CommitChangesIfConsistent + (newBlock, samples, wxT("Paste branch one")); + return; } const int b = (s == mNumSamples) ? mBlock.size() - 1 : FindBlock(s); @@ -541,6 +546,10 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) // largerBlockLen is not more than mMaxSamples... buffer.ptr(), largerBlockLen.as_size_t(), mSampleFormat); + // Don't make a duplicate array. We can still give STRONG-GUARANTEE + // if we modify only one block in place. + + // use NOFAIL-GUARANTEE in remaining steps block.f = file; for (unsigned int i = b + 1; i < numBlocks; i++) @@ -548,8 +557,10 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) mNumSamples += addedLen; - ConsistencyCheck(wxT("Paste branch two")); - return true; + // This consistency check won't throw, it asserts. + // Proof that we kept consistency is not hard. + ConsistencyCheck(wxT("Paste branch two"), false); + return; } // Case three: if we are inserting four or fewer blocks, @@ -637,12 +648,8 @@ bool Sequence::Paste(sampleCount s, const Sequence *src) for (i = b + 1; i < numBlocks; i++) newBlock.push_back(mBlock[i].Plus(addedLen)); - mBlock.swap(newBlock); - - mNumSamples += addedLen; - - ConsistencyCheck(wxT("Paste branch three")); - return true; + CommitChangesIfConsistent + (newBlock, mNumSamples + addedLen, wxT("Paste branch three")); } bool Sequence::SetSilence(sampleCount s0, sampleCount len) @@ -693,11 +700,9 @@ bool Sequence::InsertSilence(sampleCount s0, sampleCount len) sTrack.mNumSamples = pos; - bool bResult = Paste(s0, &sTrack); - wxASSERT(bResult); + Paste(s0, &sTrack); - ConsistencyCheck(wxT("InsertSilence")); - return bResult; + return true; } bool Sequence::AppendAlias(const wxString &fullPath, @@ -737,13 +742,15 @@ bool Sequence::AppendCoded(const wxString &fName, sampleCount start, return true; } -bool Sequence::AppendBlock +void Sequence::AppendBlock (DirManager &mDirManager, BlockArray &mBlock, sampleCount &mNumSamples, const SeqBlock &b) { // Quick check to make sure that it doesn't overflow if (Overflows((mNumSamples.as_double()) + ((double)b.f->GetLength()))) - return false; + // THROW_INCONSISTENCY_EXCEPTION + return + ; SeqBlock newBlock( mDirManager.CopyBlockFile(b.f), // Bump ref count if not locked, else copy @@ -755,16 +762,11 @@ bool Sequence::AppendBlock return false; } - //Don't need to Ref because it was done by CopyBlockFile, above... - //mDirManager->Ref(newBlock.f); - mBlock.push_back(newBlock); mNumSamples += newBlock.f->GetLength(); // Don't do a consistency check here because this // function gets called in an inner loop. - - return true; } ///gets an int with OD flags so that we can determine which ODTasks should be run on this track after save/open, etc. diff --git a/src/Sequence.h b/src/Sequence.h index 29b1da119..192dc4f68 100644 --- a/src/Sequence.h +++ b/src/Sequence.h @@ -104,7 +104,7 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{ size_t len, const sampleCount *where) const; std::unique_ptr Copy(sampleCount s0, sampleCount s1) const; - bool Paste(sampleCount s0, const Sequence *src); + void Paste(sampleCount s0, const Sequence *src); size_t GetIdealAppendLen() const; bool Append(samplePtr buffer, sampleFormat format, size_t len, @@ -245,7 +245,7 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{ int FindBlock(sampleCount pos) const; - static bool AppendBlock + static void AppendBlock (DirManager &dirManager, BlockArray &blocks, sampleCount &numSamples, const SeqBlock &b); diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index d4a6855ff..8d0f1f435 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -1611,18 +1611,16 @@ void WaveClip::Paste(double t0, const WaveClip* other) TimeToSamplesClip(t0, &s0); // Assume STRONG-GUARANTEE from Sequence::Paste - if (mSequence->Paste(s0, pastedClip->mSequence.get())) - { - // Assume NOFAIL-GUARANTEE in the remaining - MarkChanged(); - mEnvelope->Paste(s0.as_double()/mRate + mOffset, pastedClip->mEnvelope.get()); - mEnvelope->RemoveUnneededPoints(); - OffsetCutLines(t0, pastedClip->GetEndTime() - pastedClip->GetStartTime()); + mSequence->Paste(s0, pastedClip->mSequence.get()); - for (auto &holder : newCutlines) - mCutLines.push_back(std::move(holder)); + // Assume NOFAIL-GUARANTEE in the remaining + MarkChanged(); + mEnvelope->Paste(s0.as_double()/mRate + mOffset, pastedClip->mEnvelope.get()); + mEnvelope->RemoveUnneededPoints(); + OffsetCutLines(t0, pastedClip->GetEndTime() - pastedClip->GetStartTime()); - } + for (auto &holder : newCutlines) + mCutLines.push_back(std::move(holder)); } void WaveClip::InsertSilence(double t, double len) From 3c65731f38ead2d8c7045c8665daa7cb67a692a0 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 27 Nov 2016 10:39:46 -0500 Subject: [PATCH 4/8] Sequence::InsertSilence gives strong guarantee --- src/Sequence.cpp | 11 ++++++----- src/Sequence.h | 2 +- src/WaveClip.cpp | 6 +----- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Sequence.cpp b/src/Sequence.cpp index bb0df2917..8e1e9dc0f 100644 --- a/src/Sequence.cpp +++ b/src/Sequence.cpp @@ -657,14 +657,16 @@ bool Sequence::SetSilence(sampleCount s0, sampleCount len) return Set(NULL, mSampleFormat, s0, len); } -bool Sequence::InsertSilence(sampleCount s0, sampleCount len) +void Sequence::InsertSilence(sampleCount s0, sampleCount len) +// STRONG-GUARANTEE { // Quick check to make sure that it doesn't overflow if (Overflows((mNumSamples.as_double()) + (len.as_double()))) - return false; + //THROW_INCONSISTENCY_EXCEPTION + ; if (len <= 0) - return true; + return; // Create a NEW track containing as much silence as we // need to insert, and then call Paste to do the insertion. @@ -700,9 +702,8 @@ bool Sequence::InsertSilence(sampleCount s0, sampleCount len) sTrack.mNumSamples = pos; + // use STRONG-GUARANTEE Paste(s0, &sTrack); - - return true; } bool Sequence::AppendAlias(const wxString &fullPath, diff --git a/src/Sequence.h b/src/Sequence.h index 192dc4f68..c416a4d48 100644 --- a/src/Sequence.h +++ b/src/Sequence.h @@ -129,7 +129,7 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{ void AppendBlockFile(const BlockFilePtr &blockFile); bool SetSilence(sampleCount s0, sampleCount len); - bool InsertSilence(sampleCount s0, sampleCount len); + void InsertSilence(sampleCount s0, sampleCount len); const std::shared_ptr &GetDirManager() { return mDirManager; } diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index 8d0f1f435..a224f2461 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -1631,11 +1631,7 @@ void WaveClip::InsertSilence(double t, double len) auto slen = (sampleCount)floor(len * mRate + 0.5); // use STRONG-GUARANTEE - if (!GetSequence()->InsertSilence(s0, slen)) - { - wxASSERT(false); - return; - } + GetSequence()->InsertSilence(s0, slen); // use NOFAIL-GUARANTEE OffsetCutLines(t, len); From 73e61592aa5b8e9535f66d47f394b4248aa52fc1 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Mon, 28 Nov 2016 10:15:34 -0500 Subject: [PATCH 5/8] Sequence::ConvertToSampleFormat gives strong guarantee --- src/Sequence.cpp | 76 ++++++++++++++++++------------------------------ src/Sequence.h | 5 ++-- src/WaveClip.cpp | 6 ++-- 3 files changed, 34 insertions(+), 53 deletions(-) diff --git a/src/Sequence.cpp b/src/Sequence.cpp index 8e1e9dc0f..8d2d1179b 100644 --- a/src/Sequence.cpp +++ b/src/Sequence.cpp @@ -129,19 +129,16 @@ bool Sequence::SetSampleFormat(sampleFormat format) } */ -bool Sequence::ConvertToSampleFormat(sampleFormat format, bool* pbChanged) +bool Sequence::ConvertToSampleFormat(sampleFormat format) +// STRONG-GUARANTEE { - wxASSERT(pbChanged); - *pbChanged = false; - - // Caller should check this no-change case before calling; we ignore it here. if (format == mSampleFormat) - return true; + // no change + return false; if (mBlock.size() == 0) { mSampleFormat = format; - *pbChanged = true; return true; } @@ -153,25 +150,32 @@ bool Sequence::ConvertToSampleFormat(sampleFormat format, bool* pbChanged) mMinSamples = sMaxDiskBlockSize / SAMPLE_SIZE(mSampleFormat) / 2; mMaxSamples = mMinSamples * 2; - BlockArray newBlockArray; - // Use the ratio of old to NEW mMaxSamples to make a reasonable guess at allocation. - newBlockArray.reserve(1 + mBlock.size() * ((float)oldMaxSamples / (float)mMaxSamples)); + bool bSuccess = false; + auto cleanup = finally( [&] { + if (!bSuccess) { + // Conversion failed. Revert these member vars. + mSampleFormat = oldFormat; + mMaxSamples = oldMaxSamples; + mMinSamples = oldMinSamples; + } + } ); + + BlockArray newBlockArray; + // Use the ratio of old to NEW mMaxSamples to make a reasonable guess + // at allocation. + newBlockArray.reserve + (1 + mBlock.size() * ((float)oldMaxSamples / (float)mMaxSamples)); - bool bSuccess = true; { SampleBuffer bufferOld(oldMaxSamples, oldFormat); SampleBuffer bufferNew(oldMaxSamples, format); - for (size_t i = 0, nn = mBlock.size(); i < nn && bSuccess; i++) + for (size_t i = 0, nn = mBlock.size(); i < nn; i++) { SeqBlock &oldSeqBlock = mBlock[i]; const auto &oldBlockFile = oldSeqBlock.f; - const auto len = oldBlockFile->GetLength(); - - bSuccess = (oldBlockFile->ReadData(bufferOld.ptr(), oldFormat, 0, len) > 0); - if (!bSuccess) - break; + Read(bufferOld.ptr(), oldFormat, oldSeqBlock, 0, len, true); CopySamples(bufferOld.ptr(), oldFormat, bufferNew.ptr(), format, len); @@ -189,42 +193,20 @@ bool Sequence::ConvertToSampleFormat(sampleFormat format, bool* pbChanged) const unsigned prevSize = newBlockArray.size(); Blockify(*mDirManager, mMaxSamples, mSampleFormat, newBlockArray, blockstart, bufferNew.ptr(), len); - bSuccess = (newBlockArray.size() > prevSize); - if (bSuccess) - *pbChanged = true; } } - if (bSuccess) - { - // Invalidate all the old, non-aliased block files. - // Aliased files will be converted at save, per comment above. + // Invalidate all the old, non-aliased block files. + // Aliased files will be converted at save, per comment above. - // Replace with NEW blocks. - mBlock.swap(newBlockArray); - } - else - { - /* vvvvv We *should do the following, but TrackPanel::OnFormatChange() doesn't actually check the conversion results, - it just assumes the conversion was successful. - TODO: Uncomment this section when TrackPanel::OnFormatChange() is upgraded to check the results. + // Commit the changes to block file array + CommitChangesIfConsistent + (newBlockArray, mNumSamples, wxT("Sequence::ConvertToSampleFormat()")); - PRL: I don't understand why the comment above justifies leaving the sequence in an inconsistent state. - If this function must fail, better to leave it as a no-op on this sequence. I am uncommenting the - lines below, and adding one to revert mMinSamples too. - */ + // Commit the other changes + bSuccess = true; - // Conversion failed. Revert these member vars. - mSampleFormat = oldFormat; - mMaxSamples = oldMaxSamples; - mMinSamples = oldMinSamples; - - *pbChanged = false; // Revert overall change flag, in case we had some partial success in the loop. - } - - ConsistencyCheck(wxT("Sequence::ConvertToSampleFormat()")); - - return bSuccess; + return true; } std::pair Sequence::GetMinMax( diff --git a/src/Sequence.h b/src/Sequence.h index c416a4d48..11aa8f7dc 100644 --- a/src/Sequence.h +++ b/src/Sequence.h @@ -162,8 +162,9 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{ // sampleFormat GetSampleFormat() const; - // bool SetSampleFormat(sampleFormat format); - bool ConvertToSampleFormat(sampleFormat format, bool* pbChanged); + + // Return true iff there is a change + bool ConvertToSampleFormat(sampleFormat format); // // Retrieving summary info diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index a224f2461..19ef37708 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -1342,11 +1342,9 @@ void WaveClip::ConvertToSampleFormat(sampleFormat format) // Note: it is not necessary to do this recursively to cutlines. // They get converted as needed when they are expanded. - bool bChanged; - bool bResult = mSequence->ConvertToSampleFormat(format, &bChanged); - if (bResult && bChanged) + auto bChanged = mSequence->ConvertToSampleFormat(format); + if (bChanged) MarkChanged(); - wxASSERT(bResult); // TODO: Throw an actual error. } void WaveClip::UpdateEnvelopeTrackLen() From 06f6953c910d019a89f014ffba48a5704c890623 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Tue, 29 Nov 2016 09:42:32 -0500 Subject: [PATCH 6/8] Sequence::SetSamples, ::SetSilence give strong guarantee --- src/Sequence.cpp | 23 +++++++++++++++-------- src/Sequence.h | 4 ++-- src/WaveClip.cpp | 2 +- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/Sequence.cpp b/src/Sequence.cpp index 8d2d1179b..07b6d08dc 100644 --- a/src/Sequence.cpp +++ b/src/Sequence.cpp @@ -634,9 +634,10 @@ void Sequence::Paste(sampleCount s, const Sequence *src) (newBlock, mNumSamples + addedLen, wxT("Paste branch three")); } -bool Sequence::SetSilence(sampleCount s0, sampleCount len) +void Sequence::SetSilence(sampleCount s0, sampleCount len) +// STRONG-GUARANTEE { - return Set(NULL, mSampleFormat, s0, len); + SetSamples(NULL, mSampleFormat, s0, len); } void Sequence::InsertSilence(sampleCount s0, sampleCount len) @@ -1169,12 +1170,14 @@ bool Sequence::Get(int b, samplePtr buffer, sampleFormat format, } // Pass NULL to set silence -bool Sequence::Set(samplePtr buffer, sampleFormat format, +void Sequence::SetSamples(samplePtr buffer, sampleFormat format, sampleCount start, sampleCount len) +// STRONG-GUARANTEE { if (start < 0 || start >= mNumSamples || - start+len > mNumSamples) - return false; + start + len > mNumSamples) + //THROW_INCONSISTENCY_EXCEPTION + ; SampleBuffer scratch(mMaxSamples, mSampleFormat); @@ -1185,9 +1188,12 @@ bool Sequence::Set(samplePtr buffer, sampleFormat format, } int b = FindBlock(start); + BlockArray newBlock; + std::copy( mBlock.begin(), mBlock.begin() + b, std::back_inserter(newBlock) ); while (len != 0) { - SeqBlock &block = mBlock[b]; + newBlock.push_back( mBlock[b] ); + SeqBlock &block = newBlock.back(); // start is within block const auto bstart = ( start - block.start ).as_size_t(); const auto fileLength = block.f->GetLength(); @@ -1242,8 +1248,9 @@ bool Sequence::Set(samplePtr buffer, sampleFormat format, b++; } - ConsistencyCheck(wxT("Set")); - return true; + std::copy( mBlock.begin() + b, mBlock.end(), std::back_inserter(newBlock) ); + + CommitChangesIfConsistent( newBlock, mNumSamples, wxT("SetSamples") ); } namespace { diff --git a/src/Sequence.h b/src/Sequence.h index 11aa8f7dc..fdbf77cc1 100644 --- a/src/Sequence.h +++ b/src/Sequence.h @@ -90,7 +90,7 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{ // Note that len is not size_t, because nullptr may be passed for buffer, in // which case, silence is inserted, possibly a large amount. - bool Set(samplePtr buffer, sampleFormat format, + void SetSamples(samplePtr buffer, sampleFormat format, sampleCount start, sampleCount len); // where is input, assumed to be nondecreasing, and its size is len + 1. @@ -128,7 +128,7 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{ // loaded from an XML file via DirManager::HandleXMLTag void AppendBlockFile(const BlockFilePtr &blockFile); - bool SetSilence(sampleCount s0, sampleCount len); + void SetSilence(sampleCount s0, sampleCount len); void InsertSilence(sampleCount s0, sampleCount len); const std::shared_ptr &GetDirManager() { return mDirManager; } diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index 19ef37708..524e425b1 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -413,7 +413,7 @@ void WaveClip::SetSamples(samplePtr buffer, sampleFormat format, // STRONG-GUARANTEE { // use STRONG-GUARANTEE - mSequence->Set(buffer, format, start, len); + mSequence->SetSamples(buffer, format, start, len); // use NOFAIL-GUARANTEE MarkChanged(); From 63cf80d24432b1ac50cd4518616837dc6170c122 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Mon, 28 Nov 2016 21:49:00 -0500 Subject: [PATCH 7/8] Sequence::Delete gives strong guarantee --- src/Sequence.cpp | 47 +++++++++++---------- src/Sequence.h | 2 +- src/WaveClip.cpp | 107 +++++++++++++++++++++++------------------------ 3 files changed, 80 insertions(+), 76 deletions(-) diff --git a/src/Sequence.cpp b/src/Sequence.cpp index 07b6d08dc..43653e869 100644 --- a/src/Sequence.cpp +++ b/src/Sequence.cpp @@ -1622,12 +1622,15 @@ void Sequence::Blockify } } -bool Sequence::Delete(sampleCount start, sampleCount len) +void Sequence::Delete(sampleCount start, sampleCount len) +// STRONG-GUARANTEE { if (len == 0) - return true; + return; + if (len < 0 || start < 0 || start >= mNumSamples) - return false; + //THROW_INCONSISTENCY_EXCEPTION + ; //TODO: add a ref-deref mechanism to SeqBlock/BlockArray so we don't have to make this a critical section. //On-demand threads iterate over the mBlocks and the GUI thread deletes them, so for now put a mutex here over @@ -1641,9 +1644,6 @@ bool Sequence::Delete(sampleCount start, sampleCount len) auto sampleSize = SAMPLE_SIZE(mSampleFormat); - // Special case: if the samples to DELETE are all within a single - // block and the resulting length is not too small, perform the - // deletion within this block: SeqBlock *pBlock; decltype(pBlock->f->GetLength()) length; @@ -1652,7 +1652,11 @@ bool Sequence::Delete(sampleCount start, sampleCount len) // The maximum size that will ever be needed const auto scratchSize = mMaxSamples + mMinSamples; - if (b0 == b1 && (length = (pBlock = &mBlock[b0])->f->GetLength()) - len >= mMinSamples) { + // Special case: if the samples to DELETE are all within a single + // block and the resulting length is not too small, perform the + // deletion within this block: + if (b0 == b1 && + (length = (pBlock = &mBlock[b0])->f->GetLength()) - len >= mMinSamples) { SeqBlock &b = *pBlock; // start is within block auto pos = ( start - b.start ).as_size_t(); @@ -1670,18 +1674,25 @@ bool Sequence::Delete(sampleCount start, sampleCount len) // is not more than the length of the block ( pos + len ).as_size_t(), newLen - pos, true); - b = SeqBlock( - mDirManager->NewSimpleBlockFile(scratch.ptr(), newLen, mSampleFormat), - b.start - ); + auto newFile = + mDirManager->NewSimpleBlockFile(scratch.ptr(), newLen, mSampleFormat); + + // Don't make a duplicate array. We can still give STRONG-GUARANTEE + // if we modify only one block in place. + + // use NOFAIL-GUARANTEE in remaining steps + + b.f = newFile; for (unsigned int j = b0 + 1; j < numBlocks; j++) mBlock[j].start -= len; mNumSamples -= len; - ConsistencyCheck(wxT("Delete - branch one")); - return true; + // This consistency check won't throw, it asserts. + // Proof that we kept consistency is not hard. + ConsistencyCheck(wxT("Delete - branch one"), false); + return; } // Create a NEW array of blocks @@ -1782,14 +1793,8 @@ bool Sequence::Delete(sampleCount start, sampleCount len) for (i = b1 + 1; i < numBlocks; i++) newBlock.push_back(mBlock[i].Plus(-len)); - // Substitute our NEW array for the old one - mBlock.swap(newBlock); - - // Update total number of samples and do a consistency check. - mNumSamples -= len; - - ConsistencyCheck(wxT("Delete - branch two")); - return true; + CommitChangesIfConsistent + (newBlock, mNumSamples - len, wxT("Delete - branch two")); } void Sequence::ConsistencyCheck(const wxChar *whereStr, bool mayThrow) const diff --git a/src/Sequence.h b/src/Sequence.h index fdbf77cc1..1eaab1fcb 100644 --- a/src/Sequence.h +++ b/src/Sequence.h @@ -109,7 +109,7 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{ size_t GetIdealAppendLen() const; bool Append(samplePtr buffer, sampleFormat format, size_t len, XMLWriter* blockFileLog=NULL); - bool Delete(sampleCount start, sampleCount len); + void Delete(sampleCount start, sampleCount len); bool AppendAlias(const wxString &fullPath, sampleCount start, size_t len, int channel, bool useOD); diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index 524e425b1..8925a9a30 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -1646,55 +1646,55 @@ void WaveClip::Clear(double t0, double t1) TimeToSamplesClip(t1, &s1); // use STRONG-GUARANTEE - if (GetSequence()->Delete(s0, s1-s0)) - // use NOFAIL-GUARANTEE in the remaining + GetSequence()->Delete(s0, s1-s0); + + // use NOFAIL-GUARANTEE in the remaining + + // msmeyer + // + // Delete all cutlines that are within the given area, if any. + // + // Note that when cutlines are active, two functions are used: + // Clear() and ClearAndAddCutLine(). ClearAndAddCutLine() is called + // whenever the user directly calls a command that removes some audio, e.g. + // "Cut" or "Clear" from the menu. This command takes care about recursive + // preserving of cutlines within clips. Clear() is called when internal + // operations want to remove audio. In the latter case, it is the right + // thing to just remove all cutlines within the area. + // + double clip_t0 = t0; + double clip_t1 = t1; + if (clip_t0 < GetStartTime()) + clip_t0 = GetStartTime(); + if (clip_t1 > GetEndTime()) + clip_t1 = GetEndTime(); + + // May DELETE as we iterate, so don't use range-for + for (auto it = mCutLines.begin(); it != mCutLines.end();) { - // msmeyer - // - // Delete all cutlines that are within the given area, if any. - // - // Note that when cutlines are active, two functions are used: - // Clear() and ClearAndAddCutLine(). ClearAndAddCutLine() is called - // whenever the user directly calls a command that removes some audio, e.g. - // "Cut" or "Clear" from the menu. This command takes care about recursive - // preserving of cutlines within clips. Clear() is called when internal - // operations want to remove audio. In the latter case, it is the right - // thing to just remove all cutlines within the area. - // - double clip_t0 = t0; - double clip_t1 = t1; - if (clip_t0 < GetStartTime()) - clip_t0 = GetStartTime(); - if (clip_t1 > GetEndTime()) - clip_t1 = GetEndTime(); - - // May DELETE as we iterate, so don't use range-for - for (auto it = mCutLines.begin(); it != mCutLines.end();) + WaveClip* clip = it->get(); + double cutlinePosition = mOffset + clip->GetOffset(); + if (cutlinePosition >= t0 && cutlinePosition <= t1) { - WaveClip* clip = it->get(); - double cutlinePosition = mOffset + clip->GetOffset(); - if (cutlinePosition >= t0 && cutlinePosition <= t1) - { - // This cutline is within the area, DELETE it - it = mCutLines.erase(it); - } - else - { - if (cutlinePosition >= t1) - { - clip->Offset(clip_t0 - clip_t1); - } - ++it; - } + // This cutline is within the area, DELETE it + it = mCutLines.erase(it); + } + else + { + if (cutlinePosition >= t1) + { + clip->Offset(clip_t0 - clip_t1); + } + ++it; } - - // Collapse envelope - GetEnvelope()->CollapseRegion(t0, t1); - if (t0 < GetStartTime()) - Offset(-(GetStartTime() - t0)); - - MarkChanged(); } + + // Collapse envelope + GetEnvelope()->CollapseRegion(t0, t1); + if (t0 < GetStartTime()) + Offset(-(GetStartTime() - t0)); + + MarkChanged(); } void WaveClip::ClearAndAddCutLine(double t0, double t1) @@ -1739,17 +1739,16 @@ void WaveClip::ClearAndAddCutLine(double t0, double t1) TimeToSamplesClip(t1, &s1); // use WEAK-GUARANTEE - if (GetSequence()->Delete(s0, s1-s0)) - { - // Collapse envelope - GetEnvelope()->CollapseRegion(t0, t1); - if (t0 < GetStartTime()) - Offset(-(GetStartTime() - t0)); + GetSequence()->Delete(s0, s1-s0); - MarkChanged(); + // Collapse envelope + GetEnvelope()->CollapseRegion(t0, t1); + if (t0 < GetStartTime()) + Offset(-(GetStartTime() - t0)); - mCutLines.push_back(std::move(newClip)); - } + MarkChanged(); + + mCutLines.push_back(std::move(newClip)); } bool WaveClip::FindCutLine(double cutLinePosition, From fc0f093db76a472e31c1fbd8ce0b5cf613253e2d Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Thu, 1 Dec 2016 20:01:40 -0500 Subject: [PATCH 8/8] Sequence::Append* give strong guarantee --- src/Sequence.cpp | 71 +++++++++++++++++++++++++++++------------------- src/Sequence.h | 6 ++-- src/WaveClip.cpp | 41 ++++++++++------------------ 3 files changed, 60 insertions(+), 58 deletions(-) diff --git a/src/Sequence.cpp b/src/Sequence.cpp index 43653e869..f079f81da 100644 --- a/src/Sequence.cpp +++ b/src/Sequence.cpp @@ -689,13 +689,15 @@ void Sequence::InsertSilence(sampleCount s0, sampleCount len) Paste(s0, &sTrack); } -bool Sequence::AppendAlias(const wxString &fullPath, +void Sequence::AppendAlias(const wxString &fullPath, sampleCount start, size_t len, int channel, bool useOD) +// STRONG-GUARANTEE { // Quick check to make sure that it doesn't overflow if (Overflows((mNumSamples.as_double()) + ((double)len))) - return false; + //THROW_INCONSISTENCY_EXCEPTION + ; SeqBlock newBlock( useOD? @@ -705,16 +707,16 @@ bool Sequence::AppendAlias(const wxString &fullPath, ); mBlock.push_back(newBlock); mNumSamples += len; - - return true; } -bool Sequence::AppendCoded(const wxString &fName, sampleCount start, +void Sequence::AppendCoded(const wxString &fName, sampleCount start, size_t len, int channel, int decodeType) +// STRONG-GUARANTEE { // Quick check to make sure that it doesn't overflow if (Overflows((mNumSamples.as_double()) + ((double)len))) - return false; + //THROW_INCONSISTENCY_EXCEPTION + ; SeqBlock newBlock( mDirManager->NewODDecodeBlockFile(fName, start, len, channel, decodeType), @@ -722,8 +724,6 @@ bool Sequence::AppendCoded(const wxString &fName, sampleCount start, ); mBlock.push_back(newBlock); mNumSamples += len; - - return true; } void Sequence::AppendBlock @@ -1518,22 +1518,32 @@ size_t Sequence::GetIdealAppendLen() const return max - lastBlockLen; } -bool Sequence::Append(samplePtr buffer, sampleFormat format, +void Sequence::Append(samplePtr buffer, sampleFormat format, size_t len, XMLWriter* blockFileLog /*=NULL*/) +// STRONG-GUARANTEE { + if (len == 0) + return; + // Quick check to make sure that it doesn't overflow if (Overflows(mNumSamples.as_double() + ((double)len))) - return false; + //THROW_INCONSISTENCY_EXCEPTION + ; + + BlockArray newBlock; + sampleCount newNumSamples = mNumSamples; // If the last block is not full, we need to add samples to it int numBlocks = mBlock.size(); SeqBlock *pLastBlock; decltype(pLastBlock->f->GetLength()) length; SampleBuffer buffer2(mMaxSamples, mSampleFormat); + bool replaceLast = false; if (numBlocks > 0 && (length = (pLastBlock = &mBlock.back())->f->GetLength()) < mMinSamples) { - SeqBlock &lastBlock = *pLastBlock; + // Enlarge a sub-minimum block at the end + const SeqBlock &lastBlock = *pLastBlock; const auto addLen = std::min(mMaxSamples - length, len); Read(buffer2.ptr(), mSampleFormat, lastBlock, 0, length, true); @@ -1544,11 +1554,13 @@ bool Sequence::Append(samplePtr buffer, sampleFormat format, mSampleFormat, addLen); - const int newLastBlockLen = length + addLen; + const auto newLastBlockLen = length + addLen; SeqBlock newLastBlock( - mDirManager->NewSimpleBlockFile(buffer2.ptr(), newLastBlockLen, mSampleFormat, - blockFileLog != NULL), + mDirManager->NewSimpleBlockFile( + buffer2.ptr(), newLastBlockLen, mSampleFormat, + blockFileLog != NULL + ), lastBlock.start ); @@ -1557,46 +1569,49 @@ bool Sequence::Append(samplePtr buffer, sampleFormat format, static_cast< SimpleBlockFile * >( &*newLastBlock.f ) ->SaveXML( *blockFileLog ); - lastBlock = newLastBlock; + newBlock.push_back( newLastBlock ); len -= addLen; - mNumSamples += addLen; + newNumSamples += addLen; buffer += addLen * SAMPLE_SIZE(format); + + replaceLast = true; } // Append the rest as NEW blocks while (len) { const auto idealSamples = GetIdealBlockSize(); - const auto l = std::min(idealSamples, len); + const auto addedLen = std::min(idealSamples, len); BlockFilePtr pFile; if (format == mSampleFormat) { - pFile = mDirManager->NewSimpleBlockFile(buffer, l, mSampleFormat, - blockFileLog != NULL); + pFile = mDirManager->NewSimpleBlockFile( + buffer, addedLen, mSampleFormat, blockFileLog != NULL); } else { - CopySamples(buffer, format, buffer2.ptr(), mSampleFormat, l); - pFile = mDirManager->NewSimpleBlockFile(buffer2.ptr(), l, mSampleFormat, - blockFileLog != NULL); + CopySamples(buffer, format, buffer2.ptr(), mSampleFormat, addedLen); + pFile = mDirManager->NewSimpleBlockFile( + buffer2.ptr(), addedLen, mSampleFormat, blockFileLog != NULL); } if (blockFileLog) // shouldn't throw, because XMLWriter is not XMLFileWriter static_cast< SimpleBlockFile * >( &*pFile )->SaveXML( *blockFileLog ); - mBlock.push_back(SeqBlock(pFile, mNumSamples)); + newBlock.push_back(SeqBlock(pFile, mNumSamples)); - buffer += l * SAMPLE_SIZE(format); - mNumSamples += l; - len -= l; + buffer += addedLen * SAMPLE_SIZE(format); + newNumSamples += addedLen; + len -= addedLen; } + AppendBlocksIfConsistent(newBlock, replaceLast, + newNumSamples, wxT("Append")); + // JKC: During generate we use Append again and again. // If generating a long sequence this test would give O(n^2) // performance - not good! #ifdef VERY_SLOW_CHECKING ConsistencyCheck(wxT("Append")); #endif - - return true; } void Sequence::Blockify diff --git a/src/Sequence.h b/src/Sequence.h index 1eaab1fcb..7309d0f8d 100644 --- a/src/Sequence.h +++ b/src/Sequence.h @@ -107,14 +107,14 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{ void Paste(sampleCount s0, const Sequence *src); size_t GetIdealAppendLen() const; - bool Append(samplePtr buffer, sampleFormat format, size_t len, + void Append(samplePtr buffer, sampleFormat format, size_t len, XMLWriter* blockFileLog=NULL); void Delete(sampleCount start, sampleCount len); - bool AppendAlias(const wxString &fullPath, + void AppendAlias(const wxString &fullPath, sampleCount start, size_t len, int channel, bool useOD); - bool AppendCoded(const wxString &fName, sampleCount start, + void AppendCoded(const wxString &fName, sampleCount start, size_t len, int channel, int decodeType); ///gets an int with OD flags so that we can determine which ODTasks should be run on this track after save/open, etc. diff --git a/src/WaveClip.cpp b/src/WaveClip.cpp index 8925a9a30..a214090db 100644 --- a/src/WaveClip.cpp +++ b/src/WaveClip.cpp @@ -1403,13 +1403,10 @@ void WaveClip::Append(samplePtr buffer, sampleFormat format, for(;;) { if (mAppendBufferLen >= blockSize) { - bool success = - // flush some previously appended contents - // use STRONG-GUARANTEE - mSequence->Append(mAppendBuffer.ptr(), seqFormat, blockSize, - blockFileLog); - if (!success) - return; + // flush some previously appended contents + // use STRONG-GUARANTEE + mSequence->Append(mAppendBuffer.ptr(), seqFormat, blockSize, + blockFileLog); // use NOFAIL-GUARANTEE for rest of this "if" memmove(mAppendBuffer.ptr(), @@ -1444,14 +1441,11 @@ void WaveClip::AppendAlias(const wxString &fName, sampleCount start, // STRONG-GUARANTEE { // use STRONG-GUARANTEE - bool result = mSequence->AppendAlias(fName, start, len, channel,useOD); + mSequence->AppendAlias(fName, start, len, channel,useOD); // use NOFAIL-GUARANTEE - if (result) - { - UpdateEnvelopeTrackLen(); - MarkChanged(); - } + UpdateEnvelopeTrackLen(); + MarkChanged(); } void WaveClip::AppendCoded(const wxString &fName, sampleCount start, @@ -1459,14 +1453,11 @@ void WaveClip::AppendCoded(const wxString &fName, sampleCount start, // STRONG-GUARANTEE { // use STRONG-GUARANTEE - bool result = mSequence->AppendCoded(fName, start, len, channel, decodeType); + mSequence->AppendCoded(fName, start, len, channel, decodeType); // use NOFAIL-GUARANTEE -if (result) - { - UpdateEnvelopeTrackLen(); - MarkChanged(); - } + UpdateEnvelopeTrackLen(); + MarkChanged(); } void WaveClip::Flush() @@ -1479,7 +1470,6 @@ void WaveClip::Flush() //wxLogDebug(wxT(" mAppendBufferLen=%lli"), (long long) mAppendBufferLen); //wxLogDebug(wxT(" previous sample count %lli"), (long long) mSequence->GetNumSamples()); - bool success = true; if (mAppendBufferLen > 0) { auto cleanup = finally( [&] { @@ -1492,7 +1482,8 @@ void WaveClip::Flush() MarkChanged(); } ); - success = mSequence->Append(mAppendBuffer.ptr(), mSequence->GetSampleFormat(), mAppendBufferLen); + mSequence->Append(mAppendBuffer.ptr(), mSequence->GetSampleFormat(), + mAppendBufferLen); } //wxLogDebug(wxT("now sample count %lli"), (long long) mSequence->GetNumSamples()); @@ -1904,12 +1895,8 @@ void WaveClip::Resample(int rate, ProgressDialog *progress) break; } - if (!newSequence->Append((samplePtr)outBuffer.get(), floatSample, - outGenerated)) - { - error = true; - break; - } + newSequence->Append((samplePtr)outBuffer.get(), floatSample, + outGenerated); if (progress) {