Exception safety in: subclasses of ExportPlugin; and more error checking

src/FileIO.h

@@ -26,6 +26,8 @@ class FileIO
 
  public:
    FileIO(const wxString & name, FileIOMode mode);
+
+   // Calls Close()
    ~FileIO();
 
    bool IsOpened();

src/export/ExportCL.cpp

@@ -354,14 +354,19 @@ ProgressResult ExportCL::Export(AudacityProject *project,
 
    // Kick off the command
    ExportCLProcess process(&output);
-   rc = wxExecute(cmd, wxEXEC_ASYNC, &process);
 
+   {
 #if defined(__WXMSW__)
-   if (!opath.IsEmpty()) {
+      auto cleanup = finally( [&] {
-      wxSetEnv(wxT("PATH"),opath.c_str());
+         if (!opath.IsEmpty()) {
-   }
+            wxSetEnv(wxT("PATH"),opath.c_str());
+         }
+      } );
 #endif
 
+      rc = wxExecute(cmd, wxEXEC_ASYNC, &process);
+   }
+
    if (!rc) {
       wxMessageBox(wxString::Format(_("Cannot export audio to %s"),
                                     fName.c_str()));
@@ -435,6 +440,11 @@ ProgressResult ExportCL::Export(AudacityProject *project,
    auto updateResult = ProgressResult::Success;
 
    {
+      auto closeIt = finally ( [&] {
+         // Should make the process die, before propagating any exception
+         process.CloseOutput();
+      } );
+
       // Prepare the progress display
       ProgressDialog progress(_("Export"),
          selectionOnly ?
@@ -475,6 +485,7 @@ ProgressResult ExportCL::Export(AudacityProject *project,
          while (bytes > 0) {
             os->Write(mixed, bytes);
             if (!os->IsOk()) {
+               updateResult = ProgressResult::Cancelled;
                break;
             }
             bytes -= os->LastWrite();
@@ -487,9 +498,6 @@ ProgressResult ExportCL::Export(AudacityProject *project,
       // Done with the progress display
    }
 
-   // Should make the process die
-   process.CloseOutput();
-
    // Wait for process to terminate
    while (process.IsActive()) {
       wxMilliSleep(10);

src/export/ExportFFmpeg.cpp

@@ -165,7 +165,7 @@ private:
    unsigned          mChannels{};
    bool              mSupportsUTF8{};
 
-   // Smart pointer fields, their order is the reverse in which they are reset in Finalize():
+   // Smart pointer fields, their order is the reverse in which they are reset in FreeResources():
    AVFifoBufferHolder   mEncAudioFifo;          // FIFO to write incoming audio samples into
    AVMallocHolder<int16_t> mEncAudioFifoOutBuf;  // buffer to read _out_ of the FIFO into
    AVFormatContextHolder mEncFormatCtx;        // libavformat's context for our output file
@@ -357,7 +357,7 @@ bool ExportFFmpeg::Init(const char *shortname, AudacityProject *project, const T
       return false;
    }
 
-   // Only now, we can keep all the resources until Finalize().
+   // Only now, we can keep all the resources until after Finalize().
    // Cancel the local cleanup.
    cleanup.release();
 
@@ -737,7 +737,6 @@ bool ExportFFmpeg::Finalize()
    // Write any file trailers.
    av_write_trailer(mEncFormatCtx.get());
 
-   FreeResources();
    return true;
 }
 
@@ -774,7 +773,8 @@ bool ExportFFmpeg::EncodeAudioFrame(int16_t *pFrame, size_t frameSize)
    // Put the raw audio samples into the FIFO.
    ret = av_fifo_generic_write(mEncAudioFifo.get(), pRawSamples, nBytesToWrite,NULL);
 
-   wxASSERT(ret == nBytesToWrite);
+   if(ret != nBytesToWrite)
+      return false;
 
    if (nAudioFrameSizeOut > mEncAudioFifoOutBufSiz) {
       wxMessageBox(wxString::Format(_("FFmpeg : ERROR - nAudioFrameSizeOut too large.")),
@@ -853,11 +853,13 @@ ProgressResult ExportFFmpeg::Export(AudacityProject *project,
    if (mSubFormat == FMT_OTHER)
       shortname = gPrefs->Read(wxT("/FileFormats/FFmpegFormat"),wxT("matroska"));
    ret = Init(shortname.mb_str(),project, metadata, subformat);
+   auto cleanup = finally ( [&] { FreeResources(); } );
 
    if (!ret)
       return ProgressResult::Cancelled;
 
    size_t pcmBufferSize = 1024;
+
    const WaveTrackConstArray waveTracks =
       tracks->GetWaveTrackConstArray(selectionOnly, false);
    auto mixer = CreateMixer(waveTracks,
@@ -881,13 +883,19 @@ ProgressResult ExportFFmpeg::Export(AudacityProject *project,
 
          short *pcmBuffer = (short *)mixer->GetBuffer();
 
-         EncodeAudioFrame(pcmBuffer, (pcmNumSamples)*sizeof(int16_t)*mChannels);
+         if (!EncodeAudioFrame(
+            pcmBuffer, (pcmNumSamples)*sizeof(int16_t)*mChannels)) {
+            updateResult = ProgressResult::Cancelled;
+            break;
+         }
 
          updateResult = progress.Update(mixer->MixGetCurrentTime() - t0, t1 - t0);
       }
    }
 
-   Finalize();
+   if ( updateResult != ProgressResult::Cancelled )
+      if ( !Finalize() )
+         return ProgressResult::Cancelled;
 
    return updateResult;
 }

src/export/ExportFLAC.cpp

@@ -204,6 +204,7 @@ private:
 
    bool GetMetadata(AudacityProject *project, const Tags *tags);
 
+   // Should this be a stack variable instead in Export?
    FLAC__StreamMetadataHandle mMetadata;
 };
 
@@ -262,6 +263,10 @@ ProgressResult ExportFLAC::Export(AudacityProject *project,
       encoder.set_metadata(&p, 1);
    }
 
+   auto cleanup1 = finally( [&] {
+      mMetadata.reset(); // need this?
+   } );
+
    sampleFormat format;
    if (bitDepthPref == wxT("24")) {
       format = int24Sample;
@@ -312,6 +317,13 @@ ProgressResult ExportFLAC::Export(AudacityProject *project,
 
    mMetadata.reset();
 
+   auto cleanup2 = finally( [&] {
+#ifndef LEGACY_FLAC
+      f.Detach(); // libflac closes the file
+#endif
+      encoder.finish();
+   } );
+
    const WaveTrackConstArray waveTracks =
       tracks->GetWaveTrackConstArray(selectionOnly, false);
    auto mixer = CreateMixer(waveTracks,
@@ -322,38 +334,40 @@ ProgressResult ExportFLAC::Export(AudacityProject *project,
 
    ArraysOf<FLAC__int32> tmpsmplbuf{ numChannels, SAMPLES_PER_RUN, true };
 
-   {
+   ProgressDialog progress(wxFileName(fName).GetName(),
-      ProgressDialog progress(wxFileName(fName).GetName(),
+                           selectionOnly ?
-         selectionOnly ?
+                           _("Exporting the selected audio as FLAC") :
-         _("Exporting the selected audio as FLAC") :
+                           _("Exporting the entire project as FLAC"));
-         _("Exporting the entire project as FLAC"));
 
-      while (updateResult == ProgressResult::Success) {
+   while (updateResult == ProgressResult::Success) {
-         auto samplesThisRun = mixer->Process(SAMPLES_PER_RUN);
+      auto samplesThisRun = mixer->Process(SAMPLES_PER_RUN);
-         if (samplesThisRun == 0) { //stop encoding
+      if (samplesThisRun == 0) { //stop encoding
-            break;
+         break;
-         }
+      }
-         else {
+      else {
-            for (size_t i = 0; i < numChannels; i++) {
+         for (size_t i = 0; i < numChannels; i++) {
-               samplePtr mixed = mixer->GetBuffer(i);
+            samplePtr mixed = mixer->GetBuffer(i);
-               if (format == int24Sample) {
+            if (format == int24Sample) {
-                  for (decltype(samplesThisRun) j = 0; j < samplesThisRun; j++) {
+               for (decltype(samplesThisRun) j = 0; j < samplesThisRun; j++) {
-                     tmpsmplbuf[i][j] = ((int *)mixed)[j];
+                  tmpsmplbuf[i][j] = ((int *)mixed)[j];
-                  }
+               }
-               }
+            }
-               else {
+            else {
-                  for (decltype(samplesThisRun) j = 0; j < samplesThisRun; j++) {
+               for (decltype(samplesThisRun) j = 0; j < samplesThisRun; j++) {
-                     tmpsmplbuf[i][j] = ((short *)mixed)[j];
+                  tmpsmplbuf[i][j] = ((short *)mixed)[j];
-                  }
                }
             }
-            encoder.process(reinterpret_cast<const FLAC__int32**>(tmpsmplbuf.get()), samplesThisRun);
          }
-         updateResult = progress.Update(mixer->MixGetCurrentTime() - t0, t1 - t0);
+         if (! encoder.process(
+               reinterpret_cast<FLAC__int32**>( tmpsmplbuf.get() ),
+               samplesThisRun) ) {
+            updateResult = ProgressResult::Cancelled;
+            break;
+         }
       }
-      f.Detach(); // libflac closes the file
-      encoder.finish();
    }
+   if (updateResult == ProgressResult::Success)
+      updateResult = progress.Update(mixer->MixGetCurrentTime() - t0, t1 - t0);
 
    return updateResult;
 }

src/export/ExportMP2.cpp

@@ -217,6 +217,7 @@ ProgressResult ExportMP2::Export(AudacityProject *project,
 
    twolame_options *encodeOptions;
    encodeOptions = twolame_init();
+   auto cleanup = finally( [&] { twolame_close(&encodeOptions); } );
 
    twolame_set_in_samplerate(encodeOptions, (int)(rate + 0.5));
    twolame_set_out_samplerate(encodeOptions, (int)(rate + 0.5));
@@ -227,7 +228,6 @@ ProgressResult ExportMP2::Export(AudacityProject *project,
    {
       wxMessageBox(_("Cannot export MP2 with this sample rate and bit rate"),
          _("Error"), wxICON_STOP);
-      twolame_close(&encodeOptions);
       return ProgressResult::Cancelled;
    }
 
@@ -238,7 +238,6 @@ ProgressResult ExportMP2::Export(AudacityProject *project,
    FileIO outFile(fName, FileIO::Output);
    if (!outFile.IsOpened()) {
       wxMessageBox(_("Unable to open target file for writing"));
-      twolame_close(&encodeOptions);
       return ProgressResult::Cancelled;
    }
 
@@ -288,6 +287,11 @@ ProgressResult ExportMP2::Export(AudacityProject *project,
             mp2Buffer.get(),
             mp2BufferSize);
 
+         if (mp2BufferNumBytes < 0) {
+            updateResult = ProgressResult::Cancelled;
+            break;
+         }
+
          outFile.Write(mp2Buffer.get(), mp2BufferNumBytes);
 
          updateResult = progress.Update(mixer->MixGetCurrentTime() - t0, t1 - t0);
@@ -302,15 +306,11 @@ ProgressResult ExportMP2::Export(AudacityProject *project,
    if (mp2BufferNumBytes > 0)
       outFile.Write(mp2Buffer.get(), mp2BufferNumBytes);
 
-   twolame_close(&encodeOptions);
-
    /* Write ID3 tag if it was supposed to be at the end of the file */
 
    if (id3len && endOfFile)
       outFile.Write(id3buffer.get(), id3len);
 
-   /* Close file */
-
    outFile.Close();
 
    return updateResult;
@@ -328,7 +328,9 @@ struct id3_tag_deleter {
 using id3_tag_holder = std::unique_ptr<id3_tag, id3_tag_deleter>;
 
 // returns buffer len; caller frees
-int ExportMP2::AddTags(AudacityProject * WXUNUSED(project), ArrayOf<char> &buffer, bool *endOfFile, const Tags *tags)
+int ExportMP2::AddTags(
+   AudacityProject * WXUNUSED(project), ArrayOf< char > &buffer,
+   bool *endOfFile, const Tags *tags)
 {
 #ifdef USE_LIBID3TAG
    id3_tag_holder tp { id3_tag_new() };

src/export/ExportMP3.cpp

@@ -1809,6 +1809,9 @@ ProgressResult ExportMP3::Export(AudacityProject *project,
    long bytes;
 
    size_t bufferSize = std::max(0, exporter.GetOutBufferSize());
+   if (bufferSize == 0)
+      return ProgressResult::Cancelled;
+
    ArrayOf<unsigned char> buffer{ bufferSize };
    wxASSERT(buffer);
 
@@ -1873,6 +1876,7 @@ ProgressResult ExportMP3::Export(AudacityProject *project,
             wxString msg;
             msg.Printf(_("Error %ld returned from MP3 encoder"), bytes);
             wxMessageBox(msg);
+            updateResult = ProgressResult::Cancelled;
             break;
          }
 
@@ -1882,28 +1886,29 @@ ProgressResult ExportMP3::Export(AudacityProject *project,
       }
    }
 
-   bytes = exporter.FinishStream(buffer.get());
+   if ( updateResult != ProgressResult::Cancelled ) {
+      bytes = exporter.FinishStream(buffer.get());
 
-   if (bytes) {
+      if (bytes > 0) {
-      outFile.Write(buffer.get(), bytes);
+         outFile.Write(buffer.get(), bytes);
+      }
+
+      // Write ID3 tag if it was supposed to be at the end of the file
+      if (id3len > 0 && endOfFile) {
+         outFile.Write(id3buffer.get(), id3len);
+      }
+
+      // Always write the info (Xing/Lame) tag.  Until we stop supporting Lame
+      // versions before 3.98, we must do this after the MP3 file has been
+      // closed.
+      //
+      // Also, if beWriteInfoTag() is used, mGF will no longer be valid after
+      // this call, so do not use it.
+      exporter.PutInfoTag(outFile, pos);
+
+      outFile.Close();
    }
 
-   // Write ID3 tag if it was supposed to be at the end of the file
-   if (id3len && endOfFile) {
-      outFile.Write(id3buffer.get(), id3len);
-   }
-
-   // Always write the info (Xing/Lame) tag.  Until we stop supporting Lame
-   // versions before 3.98, we must do this after the MP3 file has been
-   // closed.
-   //
-   // Also, if beWriteInfoTag() is used, mGF will no longer be valid after
-   // this call, so do not use it.
-   exporter.PutInfoTag(outFile, pos);
-
-   // Close the file
-   outFile.Close();
-
    return updateResult;
 }
 

src/export/ExportOGG.cpp

@@ -193,6 +193,15 @@ ProgressResult ExportOGG::Export(AudacityProject *project,
    vorbis_dsp_state dsp;
    vorbis_block     block;
 
+   auto cleanup = finally( [&] {
+      ogg_stream_clear(&stream);
+
+      vorbis_block_clear(&block);
+      vorbis_dsp_clear(&dsp);
+      vorbis_info_clear(&info);
+      vorbis_comment_clear(&comment);
+   } );
+
    // Encoding setup
    vorbis_info_init(&info);
    vorbis_encode_init_vbr(&info, numChannels, (int)(rate + 0.5), quality);
@@ -257,9 +266,10 @@ ProgressResult ExportOGG::Export(AudacityProject *project,
          float **vorbis_buffer = vorbis_analysis_buffer(&dsp, SAMPLES_PER_RUN);
          auto samplesThisRun = mixer->Process(SAMPLES_PER_RUN);
 
+         int err;
          if (samplesThisRun == 0) {
             // Tell the library that we wrote 0 bytes - signalling the end.
-            vorbis_analysis_wrote(&dsp, 0);
+            err = vorbis_analysis_wrote(&dsp, 0);
          }
          else {
 
@@ -269,7 +279,7 @@ ProgressResult ExportOGG::Export(AudacityProject *project,
             }
 
             // tell the encoder how many samples we have
-            vorbis_analysis_wrote(&dsp, samplesThisRun);
+            err = vorbis_analysis_wrote(&dsp, samplesThisRun);
          }
 
          // I don't understand what this call does, so here is the comment
@@ -278,22 +288,23 @@ ProgressResult ExportOGG::Export(AudacityProject *project,
          //    vorbis does some data preanalysis, then divvies up blocks
          //    for more involved (potentially parallel) processing. Get
          //    a single block for encoding now
-         while (vorbis_analysis_blockout(&dsp, &block) == 1) {
+         while (!err && vorbis_analysis_blockout(&dsp, &block) == 1) {
 
             // analysis, assume we want to use bitrate management
-            vorbis_analysis(&block, NULL);
+            err = vorbis_analysis(&block, NULL);
-            vorbis_bitrate_addblock(&block);
+            if (!err)
+               err = vorbis_bitrate_addblock(&block);
 
-            while (vorbis_bitrate_flushpacket(&dsp, &packet)) {
+            while (!err && vorbis_bitrate_flushpacket(&dsp, &packet)) {
 
                // add the packet to the bitstream
-               ogg_stream_packetin(&stream, &packet);
+               err = ogg_stream_packetin(&stream, &packet);
 
                // From vorbis-tools-1.0/oggenc/encode.c:
                //   If we've gone over a page boundary, we can do actual output,
                //   so do so (for however many pages are available).
 
-               while (!eos) {
+               while (!err && !eos) {
                   int result = ogg_stream_pageout(&stream, &page);
                   if (!result) {
                      break;
@@ -309,17 +320,15 @@ ProgressResult ExportOGG::Export(AudacityProject *project,
             }
          }
 
+         if (err) {
+            updateResult = ProgressResult::Cancelled;
+            break;
+         }
+
          updateResult = progress.Update(mixer->MixGetCurrentTime() - t0, t1 - t0);
       }
    }
 
-   ogg_stream_clear(&stream);
-
-   vorbis_block_clear(&block);
-   vorbis_dsp_clear(&dsp);
-   vorbis_info_clear(&info);
-   vorbis_comment_clear(&comment);
-
    outFile.Close();
 
    return updateResult;

src/export/ExportPCM.cpp

@@ -331,7 +331,7 @@ private:
 
    ArrayOf<char> AdjustString(const wxString & wxStr, int sf_format);
    bool AddStrings(AudacityProject *project, SNDFILE *sf, const Tags *tags, int sf_format);
-   void AddID3Chunk(wxString fName, const Tags *tags, int sf_format);
+   bool AddID3Chunk(wxString fName, const Tags *tags, int sf_format);
 
 };
 
@@ -516,6 +516,7 @@ ProgressResult ExportPCM::Export(AudacityProject *project,
                                              _("Error while writing %s file (disk full?).\nLibsndfile says \"%s\""),
                                              formatStr.c_str(),
                                              wxString::FromAscii(buffer2).c_str()));
+               updateResult = ProgressResult::Cancelled;
                break;
             }
             
@@ -534,7 +535,8 @@ ProgressResult ExportPCM::Export(AudacityProject *project,
 
    if (((sf_format & SF_FORMAT_TYPEMASK) == SF_FORMAT_AIFF) ||
        ((sf_format & SF_FORMAT_TYPEMASK) == SF_FORMAT_WAV))
-      AddID3Chunk(fName, metadata, sf_format);
+      if (!AddID3Chunk(fName, metadata, sf_format) )
+         return ProgressResult::Cancelled;
 
    return updateResult;
 }
@@ -699,7 +701,7 @@ struct id3_tag_deleter {
 };
 using id3_tag_holder = std::unique_ptr<id3_tag, id3_tag_deleter>;
 
-void ExportPCM::AddID3Chunk(wxString fName, const Tags *tags, int sf_format)
+bool ExportPCM::AddID3Chunk(wxString fName, const Tags *tags, int sf_format)
 {
 #ifdef USE_LIBID3TAG
    id3_tag_holder tp { id3_tag_new() };
@@ -784,12 +786,12 @@ void ExportPCM::AddID3Chunk(wxString fName, const Tags *tags, int sf_format)
 
    len = id3_tag_render(tp.get(), 0);
    if (len == 0)
-      return;
+      return true;
 
    if ((len % 2) != 0) len++;   // Length must be even.
    ArrayOf<id3_byte_t> buffer { len, true };
    if (buffer == NULL)
-      return;
+      return false;
 
    // Zero all locations, for ending odd UTF16 content
    // correctly, i.e., two '\0's at the end.
@@ -797,33 +799,44 @@ void ExportPCM::AddID3Chunk(wxString fName, const Tags *tags, int sf_format)
    id3_tag_render(tp.get(), buffer.get());
 
    wxFFile f(fName, wxT("r+b"));
-   // FIXME: TRAP_ERR wxFFILE ops in Export PCM ID3 could fail.
    if (f.IsOpened()) {
       wxUint32 sz;
 
       sz = (wxUint32) len;
-      f.SeekEnd(0);
+      if (!f.SeekEnd(0))
+         return false;
       if ((sf_format & SF_FORMAT_TYPEMASK) == SF_FORMAT_WAV)
-         f.Write("id3 ", 4);	// Must be lower case for foobar2000.
+         {
+            if (4 != f.Write("id3 ", 4))// Must be lower case for foobar2000.
+               return false ;
+         }
       else {
-         f.Write("ID3 ", 4);
+         if (4 != f.Write("ID3 ", 4))
+            return false;
          sz = wxUINT32_SWAP_ON_LE(sz);
       }
-      f.Write(&sz, 4);
+      if (4 != f.Write(&sz, 4))
+         return false;
 
-      f.Write(buffer.get(), len);
+      if (len != f.Write(buffer.get(), len))
+         return false;
 
       sz = (wxUint32) f.Tell() - 8;
       if ((sf_format & SF_FORMAT_TYPEMASK) == SF_FORMAT_AIFF)
          sz = wxUINT32_SWAP_ON_LE(sz);
 
-      f.Seek(4);
+      if (!f.Seek(4))
-      f.Write(&sz, 4);
+         return false;
+      if (4 != f.Write(&sz, 4))
+         return false;
 
-      f.Close();
+      if (!f.Close())
+         return false;
    }
+   else
+      return false;
 #endif
-   return;
+   return true;
 }
 
 wxWindow *ExportPCM::OptionsCreate(wxWindow *parent, int format)