From a00fe789b948c338943cee9963cca30d7bc2029e Mon Sep 17 00:00:00 2001
From: Paul Licameli <Paul-Licameli@users.noreply.github.com>
Date: Wed, 2 Sep 2020 01:36:48 -0400
Subject: [PATCH] More aup import review (#656)

* More format fixes on import: beware sequences with differing format...

... It could happen.  When collecting file information to be used in the
second pass, record the correct format for each.

Don't mistakenly use whatever was the format of the last read Sequence for all
files.

This only matters for the avoidance of unnecessary sample format conversions
when finally appending samples to tracks.  It won't affect the space ultimately
occupied in the database.

* Cautions against double throw in a finally block

* Fix some Doxygen in ImportAUP.cpp
---
 src/import/ImportAUP.cpp | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/import/ImportAUP.cpp b/src/import/ImportAUP.cpp
index b8ef84ab6..fe4e2d8c9 100644
--- a/src/import/ImportAUP.cpp
+++ b/src/import/ImportAUP.cpp
@@ -1,12 +1,13 @@
-/**********************************************************************
+/*!********************************************************************
 
   Audacity: A Digital Audio Editor
 
-  ImportAUP.cpp
+  @file ImportAUP.cpp
+  @brief Upgrading project file formats from before version 3
 
 *//****************************************************************//**
 
-\class AUPmportFileHandle
+\class AUPImportFileHandle
 \brief An ImportFileHandle for AUP files (pre-AUP3)
 
 *//****************************************************************//**
@@ -134,14 +135,18 @@ private:
    bool HandleSilentBlockFile(XMLTagHandler *&handle);
    bool HandlePCMAliasBlockFile(XMLTagHandler *&handle);
 
+   // Called in first pass to collect information about blocks
    void AddFile(sampleCount len,
+                sampleFormat format,
                 const FilePath &filename = wxEmptyString,
                 sampleCount origin = 0,
                 int channel = 0);
 
+   // These two use the collected file information in a second pass
    bool AddSilence(sampleCount len);
    bool AddSamples(const FilePath &filename,
                    sampleCount len,
+                   sampleFormat format,
                    sampleCount origin = 0,
                    int channel = 0);
 
@@ -181,6 +186,7 @@ private:
       WaveClip *clip;
       FilePath path;
       sampleCount len;
+      sampleFormat format;
       sampleCount origin;
       int channel;
    } fileinfo;
@@ -341,7 +347,7 @@ ProgressResult AUPImportFileHandle::Import(WaveTrackFactory *WXUNUSED(trackFacto
       }
       else
       {
-         AddSamples(fi.path, fi.len, fi.origin, fi.channel);
+         AddSamples(fi.path, fi.len, fi.format, fi.origin, fi.channel);
       }
 
       processed += fi.len;
@@ -1215,7 +1221,7 @@ bool AUPImportFileHandle::HandleSimpleBlockFile(XMLTagHandler *&handler)
 
    // Do not set the handler - already handled
 
-   AddFile(len, filename);
+   AddFile(len, mFormat, filename);
 
    return true;
 }
@@ -1251,7 +1257,7 @@ bool AUPImportFileHandle::HandleSilentBlockFile(XMLTagHandler *&handler)
 
    // Do not set the handler - already handled
 
-   AddFile(len);
+   AddFile(len, mFormat);
 
    return true;
 }
@@ -1329,14 +1335,15 @@ bool AUPImportFileHandle::HandlePCMAliasBlockFile(XMLTagHandler *&handler)
    // Do not set the handler - already handled
 
    if (filename.IsOk())
-      AddFile(len, filename.GetFullPath(), start, channel);
+      AddFile(len, mFormat, filename.GetFullPath(), start, channel);
    else
-      AddFile(len); // will add silence instead
+      AddFile(len, mFormat); // will add silence instead
 
    return true;
 }
 
 void AUPImportFileHandle::AddFile(sampleCount len,
+                                  sampleFormat format,
                                   const FilePath &filename /* = wxEmptyString */,
                                   sampleCount origin /* = 0 */,
                                   int channel /* = 0 */)
@@ -1346,6 +1353,7 @@ void AUPImportFileHandle::AddFile(sampleCount len,
    fi.clip = mClip;
    fi.path = filename;
    fi.len = len;
+   fi.format = format,
    fi.origin = origin,
    fi.channel = channel;
 
@@ -1374,6 +1382,7 @@ bool AUPImportFileHandle::AddSilence(sampleCount len)
 // import to continue.
 bool AUPImportFileHandle::AddSamples(const FilePath &filename,
                                      sampleCount len,
+                                     sampleFormat format,
                                      sampleCount origin /* = 0 */,
                                      int channel /* = 0 */)
 {
@@ -1403,7 +1412,11 @@ bool AUPImportFileHandle::AddSamples(const FilePath &filename,
       {
          SetWarning(XO("Error while processing %s\n\nInserting silence.").Format(filename));
 
-         AddSilence(len);
+         // If we are unwinding for an exception, don't do another
+         // potentially throwing operation
+         if (!std::uncaught_exception())
+            // If this does throw, let that propagate, don't guard the call
+            AddSilence(len);
       }
 
       if (sf)
@@ -1441,7 +1454,6 @@ bool AUPImportFileHandle::AddSamples(const FilePath &filename,
       }
    }
 
-   sampleFormat format = mFormat;
    sf_count_t cnt = len.as_size_t();
    int channels = info.channels;