Strong guarantee, void return from all mutating methods of Sequence

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);
    }
 }
 

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);

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
@@ -70,9 +72,7 @@ Sequence::Sequence(const Sequence &orig, const std::shared_ptr<DirManager> &proj
    , mMinSamples(orig.mMinSamples)
    , mMaxSamples(orig.mMaxSamples)
 {
-   bool bResult = Paste(0, &orig);
+   Paste(0, &orig);
-   wxASSERT(bResult); // TO DO: Actually handle this.
-   (void)bResult;
 }
 
 Sequence::~Sequence()
@@ -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;
+   bool bSuccess = false;
-   // Use the ratio of old to NEW mMaxSamples to make a reasonable guess at allocation.
+   auto cleanup = finally( [&] {
-   newBlockArray.reserve(1 + mBlock.size() * ((float)oldMaxSamples / (float)mMaxSamples));
+      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();
-
+         Read(bufferOld.ptr(), oldFormat, oldSeqBlock, 0, len, true);
-         bSuccess = (oldBlockFile->ReadData(bufferOld.ptr(), oldFormat, 0, len) > 0);
-         if (!bSuccess)
-            break;
 
          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.
+   // Commit the changes to block file array
-      mBlock.swap(newBlockArray);
+   CommitChangesIfConsistent
-   }
+      (newBlockArray, mNumSamples, wxT("Sequence::ConvertToSampleFormat()"));
-   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.
 
-         PRL:  I don't understand why the comment above justifies leaving the sequence in an inconsistent state.
+   // Commit the other changes
-         If this function must fail, better to leave it as a no-op on this sequence.  I am uncommenting the
+   bSuccess = true;
-         lines below, and adding one to revert mMinSamples too.
-         */
 
-      // Conversion failed. Revert these member vars.
+   return true;
-      mSampleFormat = oldFormat;
-      mMaxSamples = oldMaxSamples;
-      mMinSamples = oldMinSamples;
-
-      *pbChanged = false;  // Revert overall change flag, in case we had some partial success in the loop.
-   }
-
-   bSuccess &= ConsistencyCheck(wxT("Sequence::ConvertToSampleFormat()"));
-
-   return bSuccess;
 }
 
 std::pair<float, float> Sequence::GetMinMax(
@@ -437,9 +419,7 @@ std::unique_ptr<Sequence> Sequence::Copy(sampleCount s0, sampleCount s1) const
          // Increase ref count or duplicate file
    }
 
-   if (! ConsistencyCheck(wxT("Sequence::Copy()")))
+   dest->ConsistencyCheck(wxT("Sequence::Copy()"));
-      //THROW_INCONSISTENCY_EXCEPTION
-      ;
 
    return dest;
 }
@@ -451,7 +431,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 +441,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);
+      //THROW_INCONSISTENCY_EXCEPTION
-      return false;
+      ;
    }
 
    // Quick check to make sure that it doesn't overflow
@@ -472,8 +453,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);
+      //THROW_INCONSISTENCY_EXCEPTION
-      return false;
+      ;
    }
 
    if (src->mSampleFormat != mSampleFormat)
@@ -481,8 +462,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);
+      //THROW_INCONSISTENCY_EXCEPTION
-      return false;
+      ;
    }
 
    const BlockArray &srcBlock = src->mBlock;
@@ -491,7 +472,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,11 +482,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
 
-      return ConsistencyCheck(wxT("Paste branch one"));
+      CommitChangesIfConsistent
+         (newBlock, samples, wxT("Paste branch one"));
+      return;
    }
 
    const int b = (s == mNumSamples) ? mBlock.size() - 1 : FindBlock(s);
@@ -540,6 +528,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++)
@@ -547,7 +539,10 @@ bool Sequence::Paste(sampleCount s, const Sequence *src)
 
       mNumSamples += addedLen;
 
-      return ConsistencyCheck(wxT("Paste branch two"));
+      // 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,
@@ -635,26 +630,26 @@ 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);
+   CommitChangesIfConsistent
-
+      (newBlock, mNumSamples + addedLen, wxT("Paste branch three"));
-   mNumSamples += addedLen;
-
-   return ConsistencyCheck(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);
 }
 
-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.
@@ -690,19 +685,19 @@ bool Sequence::InsertSilence(sampleCount s0, sampleCount len)
 
    sTrack.mNumSamples = pos;
 
-   bool bResult = Paste(s0, &sTrack);
+   // use STRONG-GUARANTEE
-   wxASSERT(bResult);
+   Paste(s0, &sTrack);
-
-   return bResult && ConsistencyCheck(wxT("InsertSilence"));
 }
 
-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?
@@ -712,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),
@@ -729,17 +724,17 @@ bool Sequence::AppendCoded(const wxString &fName, sampleCount start,
    );
    mBlock.push_back(newBlock);
    mNumSamples += len;
-
-   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
@@ -751,16 +746,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.
@@ -1180,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)
+       start + len > mNumSamples)
-      return false;
+      //THROW_INCONSISTENCY_EXCEPTION
+      ;
 
    SampleBuffer scratch(mMaxSamples, mSampleFormat);
 
@@ -1196,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();
@@ -1253,7 +1248,9 @@ bool Sequence::Set(samplePtr buffer, sampleFormat format,
       b++;
    }
 
-   return ConsistencyCheck(wxT("Set"));
+   std::copy( mBlock.begin() + b, mBlock.end(), std::back_inserter(newBlock) );
+
+   CommitChangesIfConsistent( newBlock, mNumSamples, wxT("SetSamples") );
 }
 
 namespace {
@@ -1521,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);
@@ -1547,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,
+         mDirManager->NewSimpleBlockFile(
-            blockFileLog != NULL),
+            buffer2.ptr(), newLastBlockLen, mSampleFormat,
+            blockFileLog != NULL
+         ),
          lastBlock.start
       );
 
@@ -1560,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,
+         pFile = mDirManager->NewSimpleBlockFile(
-                                                blockFileLog != NULL);
+            buffer, addedLen, mSampleFormat, blockFileLog != NULL);
       }
       else {
-         CopySamples(buffer, format, buffer2.ptr(), mSampleFormat, l);
+         CopySamples(buffer, format, buffer2.ptr(), mSampleFormat, addedLen);
-         pFile = mDirManager->NewSimpleBlockFile(buffer2.ptr(), l, mSampleFormat,
+         pFile = mDirManager->NewSimpleBlockFile(
-                                                blockFileLog != NULL);
+            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);
+      buffer += addedLen * SAMPLE_SIZE(format);
-      mNumSamples += l;
+      newNumSamples += addedLen;
-      len -= l;
+      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
@@ -1625,12 +1637,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
@@ -1644,9 +1659,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;
 
@@ -1655,7 +1667,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();
@@ -1673,17 +1689,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(
+      auto newFile =
-         mDirManager->NewSimpleBlockFile(scratch.ptr(), newLen, mSampleFormat),
+          mDirManager->NewSimpleBlockFile(scratch.ptr(), newLen, mSampleFormat);
-         b.start
+
-      );
+      // 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;
 
-      return ConsistencyCheck(wxT("Delete - branch one"));
+      // 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
@@ -1784,50 +1808,120 @@ 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
+   CommitChangesIfConsistent
-   mBlock.swap(newBlock);
+      (newBlock, mNumSamples - len, wxT("Delete - branch two"));
-
-   // Update total number of samples and do a consistency check.
-   mNumSamples -= len;
-
-   return ConsistencyCheck(wxT("Delete - branch two"));
 }
 
-bool Sequence::ConsistencyCheck(const wxChar *whereStr) const
+void Sequence::ConsistencyCheck(const wxChar *whereStr, bool mayThrow) const
 {
-   unsigned int i;
+   ConsistencyCheck(mBlock, 0, mNumSamples, whereStr, mayThrow);
-   sampleCount pos = 0;
+}
-   unsigned int numBlocks = mBlock.size();
-   bool bError = false;
 
-   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)
+   if ( !bError && pos != mNumSamples )
-      bError = true;
+      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;

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.
@@ -104,17 +104,17 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{
                        size_t len, const sampleCount *where) const;
 
    std::unique_ptr<Sequence> 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,
+   void 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,
+   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.
@@ -128,8 +128,8 @@ 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);
-   bool InsertSilence(sampleCount s0, sampleCount len);
+   void InsertSilence(sampleCount s0, sampleCount len);
 
    const std::shared_ptr<DirManager> &GetDirManager() { return mDirManager; }
 
@@ -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
@@ -245,7 +246,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);
 
@@ -263,19 +264,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__

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();
@@ -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;
+   auto bChanged = mSequence->ConvertToSampleFormat(format);
-   bool bResult = mSequence->ConvertToSampleFormat(format, &bChanged);
+   if (bChanged)
-   if (bResult && bChanged)
       MarkChanged();
-   wxASSERT(bResult); // TODO: Throw an actual error.
 }
 
 void WaveClip::UpdateEnvelopeTrackLen()
@@ -1405,13 +1403,10 @@ void WaveClip::Append(samplePtr buffer, sampleFormat format,
 
    for(;;) {
       if (mAppendBufferLen >= blockSize) {
-         bool success =
+         // flush some previously appended contents
-            // flush some previously appended contents
+         // use STRONG-GUARANTEE
-            // use STRONG-GUARANTEE
+         mSequence->Append(mAppendBuffer.ptr(), seqFormat, blockSize,
-            mSequence->Append(mAppendBuffer.ptr(), seqFormat, blockSize,
+                           blockFileLog);
-                              blockFileLog);
-         if (!success)
-            return;
 
          // use NOFAIL-GUARANTEE for rest of this "if"
          memmove(mAppendBuffer.ptr(),
@@ -1446,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,
@@ -1461,17 +1453,14 @@ 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();
-   }
 }
 
-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
@@ -1481,7 +1470,6 @@ bool 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( [&] {
@@ -1494,12 +1482,11 @@ bool 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());
-
-   return success;
 }
 
 bool WaveClip::HandleXMLTag(const wxChar *tag, const wxChar **attrs)
@@ -1613,18 +1600,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()))
+   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());
 
-      for (auto &holder : newCutlines)
+   // Assume NOFAIL-GUARANTEE in the remaining
-         mCutLines.push_back(std::move(holder));
+   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)
@@ -1635,11 +1620,7 @@ void WaveClip::InsertSilence(double t, double len)
    auto slen = (sampleCount)floor(len * mRate + 0.5);
 
    // use STRONG-GUARANTEE
-   if (!GetSequence()->InsertSilence(s0, slen))
+   GetSequence()->InsertSilence(s0, slen);
-   {
-      wxASSERT(false);
-      return;
-   }
 
    // use NOFAIL-GUARANTEE
    OffsetCutLines(t, len);
@@ -1656,55 +1637,55 @@ void WaveClip::Clear(double t0, double t1)
    TimeToSamplesClip(t1, &s1);
 
    // use STRONG-GUARANTEE
-   if (GetSequence()->Delete(s0, s1-s0))
+   GetSequence()->Delete(s0, s1-s0);
-      // use NOFAIL-GUARANTEE in the remaining
+
+   // 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
+      WaveClip* clip = it->get();
-      //
+      double cutlinePosition = mOffset + clip->GetOffset();
-      // Delete all cutlines that are within the given area, if any.
+      if (cutlinePosition >= t0 && cutlinePosition <= t1)
-      //
-      // 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();
+         // This cutline is within the area, DELETE it
-         double cutlinePosition = mOffset + clip->GetOffset();
+         it = mCutLines.erase(it);
-         if (cutlinePosition >= t0 && cutlinePosition <= t1)
+      }
-         {
+      else
-            // This cutline is within the area, DELETE it
+      {
-            it = mCutLines.erase(it);
+         if (cutlinePosition >= t1)
-         }
+         {
-         else
+            clip->Offset(clip_t0 - clip_t1);
-         {
+         }
-            if (cutlinePosition >= t1)
+         ++it;
-            {
-               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)
@@ -1749,17 +1730,16 @@ void WaveClip::ClearAndAddCutLine(double t0, double t1)
    TimeToSamplesClip(t1, &s1);
 
    // use WEAK-GUARANTEE
-   if (GetSequence()->Delete(s0, s1-s0))
+   GetSequence()->Delete(s0, s1-s0);
-   {
-      // 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));
 
-      mCutLines.push_back(std::move(newClip));
+   MarkChanged();
-   }
+
+   mCutLines.push_back(std::move(newClip));
 }
 
 bool WaveClip::FindCutLine(double cutLinePosition,
@@ -1915,12 +1895,8 @@ void WaveClip::Resample(int rate, ProgressDialog *progress)
          break;
       }
 
-      if (!newSequence->Append((samplePtr)outBuffer.get(), floatSample,
+      newSequence->Append((samplePtr)outBuffer.get(), floatSample,
-                               outGenerated))
+                          outGenerated);
-      {
-         error = true;
-         break;
-      }
 
       if (progress)
       {

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);