On-demand tasks check for i/o errors, and if so, make no false progress

src/Project.cpp

@@ -4412,6 +4412,9 @@ void AudacityProject::PopState(const UndoState &state)
                computeTask = make_movable<ODComputeSummaryTask>();
                odUsed=true;
             }
+            // PRL:  Is it correct to add all tracks to one task, even if they
+            // are not partnered channels?  Rather than
+            // make one task for each?
             computeTask->AddWaveTrack((WaveTrack*)copyTrack);
          }
       }

src/blockfile/ODDecodeBlockFile.cpp

@@ -47,7 +47,6 @@ ODDecodeBlockFile::ODDecodeBlockFile(wxFileNameWrapper &&baseFileName, wxFileNam
    mAliasChannel(aliasChannel)
 {
    mDecoder = NULL;
-   mDataAvailable=false;
    mAudioFileName = std::move(audioFileName);
    mFormat = int16Sample;
 }
@@ -59,11 +58,11 @@ ODDecodeBlockFile::ODDecodeBlockFile(wxFileNameWrapper &&existingFile, wxFileNam
    SimpleBlockFile{ std::move(existingFile), aliasLen, min, max, rms },
 
    mType(decodeType),
+   mDataAvailable( dataAvailable ),
    mAliasStart(aliasStart),
    mAliasChannel(aliasChannel)
 {
    mDecoder = NULL;
-   mDataAvailable=dataAvailable;
    mAudioFileName = std::move(audioFileName);
    mFormat = int16Sample;
 }
@@ -304,11 +303,7 @@ bool ODDecodeBlockFile::IsSummaryAvailable() const
 
 bool ODDecodeBlockFile::IsDataAvailable() const
 {
-   bool retval;
+   return mDataAvailable;
-   mDataAvailableMutex.Lock();
-   retval= mDataAvailable;
-   mDataAvailableMutex.Unlock();
-   return retval;
 }
 
 /// Write the summary to disk, using the derived ReadData() to get the data
@@ -321,43 +316,38 @@ int ODDecodeBlockFile::WriteODDecodeBlockFile()
    // derived classes) to get the sample data
    SampleBuffer sampleData;// = NewSamples(mLen, floatSample);
    int ret;
-   //use the decoder here.
-   mDecoderMutex.Lock();
 
-   if(!mDecoder)
    {
-      mDecoderMutex.Unlock();
+      //use the decoder here.
-      return -1;
+      ODLocker locker{ &mDecoderMutex };
+
+      if(!mDecoder)
+         return -1;
+
+      //sampleData and mFormat are set by the decoder.
+      ret = mDecoder->Decode(sampleData, mFormat, mAliasStart, mLen, mAliasChannel);
+
+      if(ret < 0) {
+         printf("ODDecodeBlockFile Decode failure\n");
+         return ret; //failure
+      }
    }
 
-
+   {
-   //sampleData and mFormat are set by the decoder.
+      //the summary is also calculated here.
-   ret = mDecoder->Decode(sampleData, mFormat, mAliasStart, mLen, mAliasChannel);
+      ODLocker locker{ &mFileNameMutex };
-
+      //TODO: we may need to write a version of WriteSimpleBlockFile that uses threadsafe FILE vs wxFile
-   mDecoderMutex.Unlock();
+      bool bSuccess =
-   if(ret < 0) {
+         WriteSimpleBlockFile(
-      printf("ODDecodeBlockFile Decode failure\n");
+                              sampleData.ptr(),
-      return ret; //failure
+                              mLen,
+                              mFormat,
+                              NULL);
+      if ( !bSuccess )
+         return -1;
    }
 
-   //the summary is also calculated here.
+   wxAtomicInc( mDataAvailable );
-   mFileNameMutex.Lock();
-   //TODO: we may need to write a version of WriteSimpleBlockFile that uses threadsafe FILE vs wxFile
-   bool bSuccess =
-      WriteSimpleBlockFile(
-         sampleData.ptr(),
-         mLen,
-         mFormat,
-         NULL);
-   wxASSERT(bSuccess); // TODO: Handle failure here by alert to user and undo partial op.
-   wxUnusedVar(bSuccess);
-
-   mFileNameMutex.Unlock();
-
-
-   mDataAvailableMutex.Lock();
-   mDataAvailable=true;
-   mDataAvailableMutex.Unlock();
 
    return ret;
 }

src/blockfile/ODDecodeBlockFile.h

@@ -32,6 +32,7 @@ Also, see ODPCMAliasBlockFile for a similar file.
 #include "../ondemand/ODTaskThread.h"
 #include "../DirManager.h"
 #include "../ondemand/ODDecodeTask.h"
+#include <wx/atomic.h>
 #include <wx/thread.h>
 
 /// An AliasBlockFile that references uncompressed data in an existing file
@@ -166,8 +167,7 @@ class ODDecodeBlockFile final : public SimpleBlockFile
    ///The original file the audio came from.
    wxFileNameWrapper mAudioFileName;
 
-   mutable ODLock    mDataAvailableMutex;
+   wxAtomicInt mDataAvailable{ 0 };
-   bool mDataAvailable;
    bool mDataBeingComputed;
 
    ODFileDecoder* mDecoder;

src/ondemand/ODComputeSummaryTask.cpp

@@ -64,32 +64,42 @@ void ODComputeSummaryTask::DoSomeInternal()
       return;
    }
 
-   sampleCount blockStartSample = 0;
-   sampleCount blockEndSample = 0;
-   bool success =false;
-
    mBlockFilesMutex.Lock();
    for(size_t i=0; i < mWaveTracks.size() && mBlockFiles.size();i++)
    {
+      bool success = false;
       const auto bf = mBlockFiles[0].lock();
 
+      sampleCount blockStartSample = 0;
+      sampleCount blockEndSample = 0;
+
       if(bf)
       {
-         bf->DoWriteSummary();
+         // WriteSummary might throw, but this is a worker thread, so stop
+         // the exceptions here!
          success = true;
+         try { bf->DoWriteSummary(); }
+         catch(...) { success = false; }
          blockStartSample = bf->GetStart();
          blockEndSample = blockStartSample + bf->GetLength();
       }
       else
       {
+         success = true;
          // The block file disappeared.
          //the waveform in the wavetrack now is shorter, so we need to update mMaxBlockFiles
          //because now there is less work to do.
          mMaxBlockFiles--;
       }
 
-      //take it out of the array - we are done with it.
+      if (success)
-      mBlockFiles.erase(mBlockFiles.begin());
+      {
+         //take it out of the array - we are done with it.
+         mBlockFiles.erase(mBlockFiles.begin());
+      }
+      else
+         // The task does not make progress
+         ;
 
       //This is a bit of a convenience in case someone tries to terminate the task by closing the trackpanel or window.
       //ODComputeSummaryTask::Terminate() uses this lock to remove everything, and we don't want it to wait since the UI is being blocked.
@@ -97,16 +107,18 @@ void ODComputeSummaryTask::DoSomeInternal()
       wxThread::This()->Yield();
       mBlockFilesMutex.Lock();
 
-      //upddate the gui for all associated blocks.  It doesn't matter that we're hitting more wavetracks then we should
+      //update the gui for all associated blocks.  It doesn't matter that we're hitting more wavetracks then we should
       //because this loop runs a number of times equal to the number of tracks, they probably are getting processed in
       //the next iteration at the same sample window.
-      mWaveTrackMutex.Lock();
+      if (success && bf) {
-      for(size_t i=0;i<mWaveTracks.size();i++)
+         mWaveTrackMutex.Lock();
-      {
+         for(size_t i=0;i<mWaveTracks.size();i++)
-         if(success && mWaveTracks[i])
+         {
-            mWaveTracks[i]->AddInvalidRegion(blockStartSample,blockEndSample);
+            if(success && mWaveTracks[i])
+               mWaveTracks[i]->AddInvalidRegion(blockStartSample,blockEndSample);
+         }
+         mWaveTrackMutex.Unlock();
       }
-      mWaveTrackMutex.Unlock();
    }
 
    mBlockFilesMutex.Unlock();

src/ondemand/ODDecodeTask.cpp

@@ -42,13 +42,13 @@ void ODDecodeTask::DoSomeInternal()
    }
 
    ODFileDecoder* decoder;
-   sampleCount blockStartSample = 0;
-   sampleCount blockEndSample = 0;
-   bool success =false;
 
    for(size_t i=0; i < mWaveTracks.size() && mBlockFiles.size();i++)
    {
       const auto bf = mBlockFiles[0].lock();
+      sampleCount blockStartSample = 0;
+      sampleCount blockEndSample = 0;
+      bool success =false;
 
       int ret = 1;
 
@@ -63,6 +63,7 @@ void ODDecodeTask::DoSomeInternal()
          if(!decoder->IsInitialized())
             decoder->Init();
          bf->SetODFileDecoder(decoder);
+         // Does not throw:
          ret = bf->DoWriteBlockFile();
          bf->UnlockRead();
 
@@ -74,24 +75,31 @@ void ODDecodeTask::DoSomeInternal()
       }
       else
       {
+         success = true;
          // The block file disappeared.
          //the waveform in the wavetrack now is shorter, so we need to update mMaxBlockFiles
          //because now there is less work to do.
          mMaxBlockFiles--;
       }
 
-      //Release the refcount we placed on it if we are successful
+      if (success)
-      if(ret >= 0 ) {
+      {
          //take it out of the array - we are done with it.
          mBlockFiles.erase(mBlockFiles.begin());
+      }
+      else
+         // The task does not make progress
+         ;
 
+      //Release the refcount we placed on it if we are successful
+      if( bf && success ) {
          //upddate the gui for all associated blocks.  It doesn't matter that we're hitting more wavetracks then we should
          //because this loop runs a number of times equal to the number of tracks, they probably are getting processed in
          //the next iteration at the same sample window.
          mWaveTrackMutex.Lock();
          for(size_t i=0;i<mWaveTracks.size();i++)
          {
-            if(success && mWaveTracks[i])
+            if(mWaveTracks[i])
                mWaveTracks[i]->AddInvalidRegion(blockStartSample,blockEndSample);
          }
          mWaveTrackMutex.Unlock();