changeset 1313:ff9697592bef 3.0-integration

Add gapless preference to prefs dialog; much work on audio read tests
author Chris Cannam
date Thu, 01 Dec 2016 17:45:40 +0000
parents 079e553dc16e
children 00cae2d5ee7e
files base/Preferences.cpp base/Preferences.h data/fileio/AudioFileReader.h data/fileio/AudioFileReaderFactory.cpp data/fileio/AudioFileReaderFactory.h data/fileio/MP3FileReader.cpp data/fileio/MP3FileReader.h data/fileio/test/AudioFileReaderTest.h data/model/ReadOnlyWaveFileModel.cpp
diffstat 9 files changed, 551 insertions(+), 282 deletions(-) [+]
line wrap: on
line diff
--- a/base/Preferences.cpp	Tue Nov 29 17:09:07 2016 +0000
+++ b/base/Preferences.cpp	Thu Dec 01 17:45:40 2016 +0000
@@ -46,6 +46,7 @@
     m_tempDirRoot(""),
     m_fixedSampleRate(0),
     m_resampleOnLoad(false),
+    m_gapless(true),
     m_normaliseAudio(false),
     m_viewFontSize(10),
     m_backgroundMode(BackgroundFromTheme),
@@ -69,6 +70,7 @@
     m_runPluginsInProcess = settings.value("run-vamp-plugins-in-process", true).toBool();
     m_fixedSampleRate = settings.value("fixed-sample-rate", 0).toDouble();
     m_resampleOnLoad = settings.value("resample-on-load", false).toBool();
+    m_gapless = settings.value("gapless", true).toBool();
     m_normaliseAudio = settings.value("normalise-audio", false).toBool();
     m_backgroundMode = BackgroundMode
         (settings.value("background-mode", int(BackgroundFromTheme)).toInt());
@@ -101,6 +103,7 @@
     props.push_back("Resample Quality");
     props.push_back("Omit Temporaries from Recent Files");
     props.push_back("Resample On Load");
+    props.push_back("Use Gapless Mode");
     props.push_back("Normalise Audio");
     props.push_back("Fixed Sample Rate");
     props.push_back("Temporary Directory Root");
@@ -143,6 +146,9 @@
     if (name == "Resample On Load") {
         return tr("Resample mismatching files on import");
     }
+    if (name == "Use Gapless Mode") {
+        return tr("Load mp3 files in gapless mode");
+    }
     if (name == "Fixed Sample Rate") {
         return tr("Single fixed sample rate to resample all files to");
     }
@@ -200,6 +206,9 @@
     if (name == "Resample On Load") {
         return ToggleProperty;
     }
+    if (name == "Use Gapless Mode") {
+        return ToggleProperty;
+    }
     if (name == "Fixed Sample Rate") {
         return ValueProperty;
     }
@@ -582,6 +591,19 @@
 }
 
 void
+Preferences::setUseGaplessMode(bool gapless)
+{
+    if (m_gapless != gapless) {
+        m_gapless = gapless;
+        QSettings settings;
+        settings.beginGroup("Preferences");
+        settings.setValue("gapless", gapless);
+        settings.endGroup();
+        emit propertyChanged("Use Gapless Mode");
+    }
+}
+
+void
 Preferences::setFixedSampleRate(sv_samplerate_t rate)
 {
     if (m_fixedSampleRate != rate) {
--- a/base/Preferences.h	Tue Nov 29 17:09:07 2016 +0000
+++ b/base/Preferences.h	Thu Dec 01 17:45:40 2016 +0000
@@ -72,7 +72,10 @@
     sv_samplerate_t getFixedSampleRate() const { return m_fixedSampleRate; }
 
     /// True if we should resample second or subsequent audio file to match first audio file's rate
-    bool getResampleOnLoad() const { return m_resampleOnLoad; }    
+    bool getResampleOnLoad() const { return m_resampleOnLoad; }
+
+    /// True if mp3 files should be loaded "gaplessly", i.e. compensating for encoder/decoder delay and padding
+    bool getUseGaplessMode() const { return m_gapless; }
     
     /// True if audio files should be loaded with normalisation (max == 1)
     bool getNormaliseAudio() const { return m_normaliseAudio; }
@@ -121,6 +124,7 @@
     void setTemporaryDirectoryRoot(QString tempDirRoot);
     void setFixedSampleRate(sv_samplerate_t);
     void setResampleOnLoad(bool);
+    void setUseGaplessMode(bool);
     void setNormaliseAudio(bool);
     void setBackgroundMode(BackgroundMode mode);
     void setTimeToTextMode(TimeToTextMode mode);
@@ -159,6 +163,7 @@
     QString m_tempDirRoot;
     sv_samplerate_t m_fixedSampleRate;
     bool m_resampleOnLoad;
+    bool m_gapless;
     bool m_normaliseAudio;
     int m_viewFontSize;
     BackgroundMode m_backgroundMode;
--- a/data/fileio/AudioFileReader.h	Tue Nov 29 17:09:07 2016 +0000
+++ b/data/fileio/AudioFileReader.h	Thu Dec 01 17:45:40 2016 +0000
@@ -31,15 +31,42 @@
 public:
     virtual ~AudioFileReader() { }
 
+    /**
+     * Return true if the file was opened successfully and no error
+     * has subsequently occurred.
+     */
     bool isOK() const { return (m_channelCount > 0); }
 
+    /**
+     * If isOK() is false, return an error string.
+     */
     virtual QString getError() const { return ""; }
 
+    /**
+     * Return the number of audio sample frames (i.e. samples per
+     * channel) in the file.
+     */
     sv_frame_t getFrameCount() const { return m_frameCount; }
+
+    /**
+     * Return the number of channels in the file.
+     */
     int getChannelCount() const { return m_channelCount; }
+
+    /**
+     * Return the samplerate at which the file is being read. This is
+     * the rate requested when the file was opened, which may differ
+     * from the native rate of the file (in which case the file will
+     * be resampled as it is read).
+     */
     sv_samplerate_t getSampleRate() const { return m_sampleRate; }
 
-    virtual sv_samplerate_t getNativeRate() const { return m_sampleRate; } // if resampled
+    /**
+     * Return the native samplerate of the file. This will differ from
+     * getSampleRate() if the file is being resampled because it was
+     * requested to open at a different rate from native.
+     */
+    virtual sv_samplerate_t getNativeRate() const { return m_sampleRate; }
 
     /**
      * Return the location of the audio data in the reader (as passed
@@ -90,7 +117,8 @@
      * thread-safe -- that is, safe to call from multiple threads with
      * different arguments on the same object at the same time.
      */
-    virtual std::vector<float> getInterleavedFrames(sv_frame_t start, sv_frame_t count) const = 0;
+    virtual std::vector<float> getInterleavedFrames(sv_frame_t start,
+                                                    sv_frame_t count) const = 0;
 
     /**
      * Return de-interleaved samples for count frames from index
@@ -99,7 +127,8 @@
      * will contain getChannelCount() sample blocks of count samples
      * each (or fewer if end of file is reached).
      */
-    virtual std::vector<std::vector<float> > getDeInterleavedFrames(sv_frame_t start, sv_frame_t count) const;
+    virtual std::vector<std::vector<float> > getDeInterleavedFrames(sv_frame_t start,
+                                                                    sv_frame_t count) const;
 
     // only subclasses that do not know exactly how long the audio
     // file is until it's been completely decoded should implement this
--- a/data/fileio/AudioFileReaderFactory.cpp	Tue Nov 29 17:09:07 2016 +0000
+++ b/data/fileio/AudioFileReaderFactory.cpp	Thu Dec 01 17:45:40 2016 +0000
@@ -61,33 +61,13 @@
 }
 
 AudioFileReader *
-AudioFileReaderFactory::createReader(FileSource source, 
-                                     sv_samplerate_t targetRate,
-                                     bool normalised,
+AudioFileReaderFactory::createReader(FileSource source,
+                                     Parameters params,
                                      ProgressReporter *reporter)
 {
-    return create(source, targetRate, normalised, false, reporter);
-}
-
-AudioFileReader *
-AudioFileReaderFactory::createThreadingReader(FileSource source, 
-                                              sv_samplerate_t targetRate,
-                                              bool normalised,
-                                              ProgressReporter *reporter)
-{
-    return create(source, targetRate, normalised, true, reporter);
-}
-
-AudioFileReader *
-AudioFileReaderFactory::create(FileSource source, 
-                               sv_samplerate_t targetRate, 
-                               bool normalised,
-                               bool threading,
-                               ProgressReporter *reporter)
-{
     QString err;
 
-    SVDEBUG << "AudioFileReaderFactory::createReader(\"" << source.getLocation() << "\"): Requested rate: " << targetRate << (targetRate == 0 ? " (use source rate)" : "") << endl;
+    SVDEBUG << "AudioFileReaderFactory::createReader(\"" << source.getLocation() << "\"): Requested rate: " << params.targetRate << (params.targetRate == 0 ? " (use source rate)" : "") << endl;
 
     if (!source.isOK()) {
         SVDEBUG << "AudioFileReaderFactory::createReader(\"" << source.getLocation() << "\": Failed to retrieve source (transmission error?): " << source.getErrorString() << endl;
@@ -101,6 +81,9 @@
 
     AudioFileReader *reader = 0;
 
+    sv_samplerate_t targetRate = params.targetRate;
+    bool normalised = (params.normalisation == Normalisation::Peak);
+  
     sv_frame_t estimatedSamples = 
         AudioFileSizeEstimator::estimate(source, targetRate);
     
@@ -118,153 +101,123 @@
     }
     
     CodedAudioFileReader::DecodeMode decodeMode =
-        (threading ?
+        (params.threadingMode == ThreadingMode::Threaded ?
          CodedAudioFileReader::DecodeThreaded :
          CodedAudioFileReader::DecodeAtOnce);
-    
-    // Try to construct a preferred reader based on the extension or
-    // MIME type.
 
-#define CHECK(reader) if (!reader->isOK()) { delete reader; reader = 0; }
+    // We go through the set of supported readers at most twice: once
+    // picking out only the readers that claim to support the given
+    // file's extension or MIME type, and (if that fails) again
+    // providing the file to every reader in turn regardless of
+    // extension or type. (If none of the readers claim to support a
+    // file, that may just mean its extension is missing or
+    // misleading. We have to be confident that the reader won't open
+    // just any old text file or whatever and pretend it's succeeded.)
 
-    if (WavFileReader::supports(source)) {
+    for (int any = 0; any <= 1; ++any) {
 
-        reader = new WavFileReader(source);
+        bool anyReader = (any > 0);
 
-        sv_samplerate_t fileRate = reader->getSampleRate();
+        if (anyReader || WavFileReader::supports(source)) {
 
-        if (reader->isOK() &&
-            (!reader->isQuicklySeekable() ||
-             normalised ||
-             (cacheMode == CodedAudioFileReader::CacheInMemory) ||
-             (targetRate != 0 && fileRate != targetRate))) {
+            reader = new WavFileReader(source);
 
-            SVDEBUG << "AudioFileReaderFactory::createReader: WAV file rate: " << reader->getSampleRate() << ", normalised " << normalised << ", seekable " << reader->isQuicklySeekable() << ", in memory " << (cacheMode == CodedAudioFileReader::CacheInMemory) << ", creating decoding reader" << endl;
+            sv_samplerate_t fileRate = reader->getSampleRate();
+
+            if (reader->isOK() &&
+                (!reader->isQuicklySeekable() ||
+                 normalised ||
+                 (cacheMode == CodedAudioFileReader::CacheInMemory) ||
+                 (targetRate != 0 && fileRate != targetRate))) {
+
+                SVDEBUG << "AudioFileReaderFactory::createReader: WAV file rate: " << reader->getSampleRate() << ", normalised " << normalised << ", seekable " << reader->isQuicklySeekable() << ", in memory " << (cacheMode == CodedAudioFileReader::CacheInMemory) << ", creating decoding reader" << endl;
             
-            delete reader;
-            reader = new DecodingWavFileReader
-                (source,
-                 decodeMode, cacheMode,
-                 targetRate ? targetRate : fileRate,
-                 normalised,
-                 reporter);
-            CHECK(reader);
+                delete reader;
+                reader = new DecodingWavFileReader
+                    (source,
+                     decodeMode, cacheMode,
+                     targetRate ? targetRate : fileRate,
+                     normalised,
+                     reporter);
+            }
+
+            if (reader->isOK()) {
+                return reader;
+            } else {
+                delete reader;
+            }
         }
-    }
     
 #ifdef HAVE_OGGZ
 #ifdef HAVE_FISHSOUND
-    if (!reader && OggVorbisFileReader::supports(source)) {
-        reader = new OggVorbisFileReader
-            (source, decodeMode, cacheMode, targetRate, normalised, reporter);
-        CHECK(reader);
-    }
+        if (anyReader || OggVorbisFileReader::supports(source)) {
+
+            reader = new OggVorbisFileReader
+                (source, decodeMode, cacheMode, targetRate, normalised, reporter);
+
+            if (reader->isOK()) {
+                return reader;
+            } else {
+                delete reader;
+            }
+        }
 #endif
 #endif
 
 #ifdef HAVE_MAD
-    if (!reader && MP3FileReader::supports(source)) {
-        reader = new MP3FileReader
-            (source, decodeMode, cacheMode, MP3FileReader::Gapless,
-             targetRate, normalised, reporter);
-        CHECK(reader);
-    }
+        if (anyReader || MP3FileReader::supports(source)) {
+
+            MP3FileReader::GaplessMode gapless =
+                params.gaplessMode == GaplessMode::Gapless ?
+                MP3FileReader::GaplessMode::Gapless :
+                MP3FileReader::GaplessMode::Gappy;
+            
+            reader = new MP3FileReader
+                (source, decodeMode, cacheMode, gapless,
+                 targetRate, normalised, reporter);
+
+            if (reader->isOK()) {
+                return reader;
+            } else {
+                delete reader;
+            }
+        }
 #endif
 
 #ifdef HAVE_QUICKTIME
-    if (!reader && QuickTimeFileReader::supports(source)) {
-        reader = new QuickTimeFileReader
-            (source, decodeMode, cacheMode, targetRate, normalised, reporter);
-        CHECK(reader);
-    }
+        if (anyReader || QuickTimeFileReader::supports(source)) {
+
+            reader = new QuickTimeFileReader
+                (source, decodeMode, cacheMode, targetRate, normalised, reporter);
+
+            if (reader->isOK()) {
+                return reader;
+            } else {
+                delete reader;
+            }
+        }
 #endif
 
 #ifdef HAVE_COREAUDIO
-    if (!reader && CoreAudioFileReader::supports(source)) {
-        reader = new CoreAudioFileReader
-            (source, decodeMode, cacheMode, targetRate, normalised, reporter);
-        CHECK(reader);
-    }
+        if (anyReader || CoreAudioFileReader::supports(source)) {
+
+            reader = new CoreAudioFileReader
+                (source, decodeMode, cacheMode, targetRate, normalised, reporter);
+
+            if (reader->isOK()) {
+                return reader;
+            } else {
+                delete reader;
+            }
+        }
 #endif
 
-    if (reader) {
-        // The happy case: a reader recognised the file extension &
-        // succeeded in opening the file
-        return reader;
     }
     
-    // If none of the readers claimed to support this file extension,
-    // perhaps the extension is missing or misleading.  Try again,
-    // ignoring it.  We have to be confident that the reader won't
-    // open just any old text file or whatever and pretend it's
-    // succeeded
-
-    reader = new WavFileReader(source);
-
-    sv_samplerate_t fileRate = reader->getSampleRate();
-
-    if (reader->isOK() &&
-        (!reader->isQuicklySeekable() ||
-         normalised ||
-         (cacheMode == CodedAudioFileReader::CacheInMemory) ||
-         (targetRate != 0 && fileRate != targetRate))) {
-
-        SVDEBUG << "AudioFileReaderFactory::createReader: WAV file rate: " << reader->getSampleRate() << ", normalised " << normalised << ", seekable " << reader->isQuicklySeekable() << ", in memory " << (cacheMode == CodedAudioFileReader::CacheInMemory) << ", creating decoding reader" << endl;
-
-        delete reader;
-        reader = new DecodingWavFileReader
-            (source,
-             decodeMode, cacheMode,
-             targetRate ? targetRate : fileRate,
-             normalised,
-             reporter);
-    }
-
-    CHECK(reader);
-    
-#ifdef HAVE_OGGZ
-#ifdef HAVE_FISHSOUND
-    if (!reader) {
-        reader = new OggVorbisFileReader
-            (source, decodeMode, cacheMode, targetRate, normalised, reporter);
-        CHECK(reader);
-    }
-#endif
-#endif
-
-#ifdef HAVE_MAD
-    if (!reader) {
-        reader = new MP3FileReader
-            (source, decodeMode, cacheMode, MP3FileReader::Gapless,
-             targetRate, normalised, reporter);
-        CHECK(reader);
-    }
-#endif
-
-#ifdef HAVE_QUICKTIME
-    if (!reader) {
-        reader = new QuickTimeFileReader
-            (source, decodeMode, cacheMode, targetRate, normalised, reporter);
-        CHECK(reader);
-    }
-#endif
-
-#ifdef HAVE_COREAUDIO
-    if (!reader) {
-        reader = new CoreAudioFileReader
-            (source, decodeMode, cacheMode, targetRate, normalised, reporter);
-        CHECK(reader);
-    }
-#endif
-
-    if (!reader) {
-        SVDEBUG << "AudioFileReaderFactory::Failed to create a reader for "
-                << "url \"" << source.getLocation()
-                << "\" (content type \""
-                << source.getContentType() << "\")" << endl;
-        return nullptr;
-    }
-    
-    return reader;
+    SVDEBUG << "AudioFileReaderFactory::Failed to create a reader for "
+            << "url \"" << source.getLocation()
+            << "\" (content type \""
+            << source.getContentType() << "\")" << endl;
+    return nullptr;
 }
 
--- a/data/fileio/AudioFileReaderFactory.h	Tue Nov 29 17:09:07 2016 +0000
+++ b/data/fileio/AudioFileReaderFactory.h	Thu Dec 01 17:45:40 2016 +0000
@@ -34,65 +34,113 @@
      */
     static QString getKnownExtensions();
 
+    enum class Normalisation {
+
+        /**
+         * Do not normalise file data.
+         */
+        None,
+
+        /**
+         * Normalise file data to abs(max) == 1.0.
+         */
+        Peak
+    };
+
+    enum class GaplessMode {
+
+        /** 
+         * Any encoder delay and padding found in file metadata will
+         * be compensated for, giving gapless decoding (assuming the
+         * metadata are correct). This is currently only applicable to
+         * mp3 files: all other supported files are always gapless
+         * where the file metadata provides for it. See documentation
+         * for MP3FileReader::GaplessMode for details of the specific
+         * implementation.
+         */
+        Gapless,
+
+        /**
+         * No delay compensation will happen and the results will be
+         * equivalent to the behaviour of audio readers before the
+         * compensation logic was implemented. This is currently only
+         * applicable to mp3 files: all other supported files are
+         * always gapless where the file metadata provides for it. See
+         * documentation for MP3FileReader::GaplessMode for details of
+         * the specific implementation.
+         */
+        Gappy
+    };
+
+    enum class ThreadingMode {
+        
+        /** 
+         * Any necessary decoding will happen synchronously when the
+         * reader is created.
+         */
+        NotThreaded,
+        
+        /**        
+         * If the reader supports threaded decoding, it will be used
+         * and the file will be decoded in a background thread. If the
+         * reader does not support threaded decoding, behaviour will
+         * be as for NotThreaded.
+         */
+        Threaded
+    };
+
+    struct Parameters {
+
+        /**
+         * Sample rate to open the file at. If zero (the default), the
+         * file's native rate will be used. If non-zero, the file will
+         * be automatically resampled to that rate.  You can query
+         * reader->getNativeRate() if you want to find out whether the
+         * file needed to be resampled.
+         */
+        sv_samplerate_t targetRate;
+
+        /**
+         * Normalisation to use. The default is Normalisation::None.
+         */
+        Normalisation normalisation;
+
+        /**
+         * Gapless mode to use. The default is GaplessMode::Gapless.
+         */
+        GaplessMode gaplessMode;
+
+        /**
+         * Threading mode. The default is ThreadingMode::NotThreaded.
+         */
+        ThreadingMode threadingMode;
+        
+        Parameters() :
+            targetRate(0),
+            normalisation(Normalisation::None),
+            gaplessMode(GaplessMode::Gapless),
+            threadingMode(ThreadingMode::NotThreaded)
+        { }
+    };
+    
     /**
      * Return an audio file reader initialised to the file at the
      * given path, or NULL if no suitable reader for this path is
      * available or the file cannot be opened.
      *
-     * If targetRate is non-zero, the file will be resampled to that
-     * rate (transparently).  You can query reader->getNativeRate()
-     * if you want to find out whether the file is being resampled
-     * or not.
-     *
-     * If normalised is true, the file data will be normalised to
-     * abs(max) == 1.0. Otherwise the file will not be normalised.
-     *
      * If a ProgressReporter is provided, it will be updated with
-     * progress status.  Caller retains ownership of the reporter
-     * object.
+     * progress status. This will only be meaningful if decoding is
+     * being carried out in non-threaded mode (either because the
+     * threaded parameter was not supplied or because the specific
+     * file reader used does not support it); in threaded mode,
+     * reported progress will jump straight to 100% before threading
+     * takes over. Caller retains ownership of the reporter object.
      *
      * Caller owns the returned object and must delete it after use.
      */
     static AudioFileReader *createReader(FileSource source,
-                                         sv_samplerate_t targetRate = 0,
-                                         bool normalised = false,
+                                         Parameters parameters,
                                          ProgressReporter *reporter = 0);
-
-    /**
-     * Return an audio file reader initialised to the file at the
-     * given path, or NULL if no suitable reader for this path is
-     * available or the file cannot be opened.  If the reader supports
-     * threaded decoding, it will be used and the file decoded in a
-     * background thread.
-     *
-     * If targetRate is non-zero, the file will be resampled to that
-     * rate (transparently).  You can query reader->getNativeRate()
-     * if you want to find out whether the file is being resampled
-     * or not.
-     *
-     * If normalised is true, the file data will be normalised to
-     * abs(max) == 1.0. Otherwise the file will not be normalised.
-     *
-     * If a ProgressReporter is provided, it will be updated with
-     * progress status.  This will only be meaningful if threading
-     * mode is not used because the file reader in use does not
-     * support it; otherwise progress as reported will jump straight
-     * to 100% before threading mode takes over.  Caller retains
-     * ownership of the reporter object.
-     *
-     * Caller owns the returned object and must delete it after use.
-     */
-    static AudioFileReader *createThreadingReader(FileSource source,
-                                                  sv_samplerate_t targetRate = 0,
-                                                  bool normalised = false,
-                                                  ProgressReporter *reporter = 0);
-
-protected:
-    static AudioFileReader *create(FileSource source,
-                                   sv_samplerate_t targetRate,
-                                   bool normalised,
-                                   bool threading,
-                                   ProgressReporter *reporter);
 };
 
 #endif
--- a/data/fileio/MP3FileReader.cpp	Tue Nov 29 17:09:07 2016 +0000
+++ b/data/fileio/MP3FileReader.cpp	Thu Dec 01 17:45:40 2016 +0000
@@ -71,7 +71,7 @@
     m_done = false;
     m_reporter = reporter;
 
-    if (m_gaplessMode == Gapless) {
+    if (m_gaplessMode == GaplessMode::Gapless) {
         CodedAudioFileReader::setFramesToTrim(DEFAULT_DECODER_DELAY, 0);
     }
     
@@ -397,7 +397,7 @@
         return MAD_FLOW_CONTINUE;
     }
 
-    if (m_gaplessMode == Gappy) {
+    if (m_gaplessMode == GaplessMode::Gappy) {
         // Our non-gapless mode does not even filter out the Xing/LAME
         // frame. That's because the main reason non-gapless mode
         // exists is for backward compatibility with MP3FileReader
--- a/data/fileio/MP3FileReader.h	Tue Nov 29 17:09:07 2016 +0000
+++ b/data/fileio/MP3FileReader.h	Thu Dec 01 17:45:40 2016 +0000
@@ -37,7 +37,7 @@
      * See http://lame.sourceforge.net/tech-FAQ.txt for a technical
      * explanation of the numbers here.
      */
-    enum GaplessMode {
+    enum class GaplessMode {
         /**
          * Trim unwanted samples from the start and end of the decoded
          * audio. From the start, trim a number of samples equal to
--- a/data/fileio/test/AudioFileReaderTest.h	Tue Nov 29 17:09:07 2016 +0000
+++ b/data/fileio/test/AudioFileReaderTest.h	Thu Dec 01 17:45:40 2016 +0000
@@ -18,6 +18,7 @@
 
 #include "../AudioFileReaderFactory.h"
 #include "../AudioFileReader.h"
+#include "../WavFileWriter.h"
 
 #include "AudioTestData.h"
 
@@ -32,6 +33,7 @@
 using namespace std;
 
 static QString audioDir = "svcore/data/fileio/test/testfiles";
+static QString diffDir  = "svcore/data/fileio/test/diffs";
 
 class AudioFileReaderTest : public QObject
 {
@@ -41,6 +43,127 @@
         return strdup(s.toLocal8Bit().data());
     }
 
+    void getFileMetadata(QString filename,
+                         QString &extension,
+                         sv_samplerate_t &rate,
+                         int &channels,
+                         int &bitdepth) {
+
+        QStringList fileAndExt = filename.split(".");
+        QStringList bits = fileAndExt[0].split("-");
+
+        extension = fileAndExt[1];
+        rate = bits[0].toInt();
+        channels = bits[1].toInt();
+        bitdepth = 16;
+        if (bits.length() > 2) {
+            bitdepth = bits[2].toInt();
+        }
+    }
+    
+    void getExpectedThresholds(QString filename,
+                               bool resampled,
+                               bool gapless,
+                               bool normalised,
+                               double &maxLimit,
+                               double &rmsLimit) {
+
+        QString extension;
+        sv_samplerate_t fileRate;
+        int channels;
+        int bitdepth;
+        getFileMetadata(filename, extension, fileRate, channels, bitdepth);
+        
+        if (normalised) {
+
+            if (extension == "ogg") {
+
+                // Our ogg is not especially high quality and is
+                // actually further from the original if normalised
+
+                maxLimit = 0.1;
+                rmsLimit = 0.03;
+
+            } else if (extension == "m4a" || extension == "aac") {
+
+                //!!! to be worked out
+                maxLimit = 1e-10;
+                rmsLimit = 1e-10;
+
+            } else if (extension == "mp3") {
+
+                if (resampled && !gapless) {
+
+                    // We expect worse figures here, because the
+                    // combination of uncompensated encoder delay +
+                    // resampling results in a fractional delay which
+                    // means the decoded signal is slightly out of
+                    // phase compared to the test signal
+
+                    maxLimit = 0.1;
+                    rmsLimit = 0.05;
+
+                } else {
+
+                    maxLimit = 0.05;
+                    rmsLimit = 0.01;
+                }
+
+            } else {
+
+                // supposed to be lossless then (wav, aiff, flac)
+                
+                if (bitdepth >= 16 && !resampled) {
+                    maxLimit = 1e-3;
+                    rmsLimit = 3e-4;
+                } else {
+                    maxLimit = 0.01;
+                    rmsLimit = 5e-3;
+                }
+            }
+            
+        } else { // !normalised
+            
+            if (extension == "ogg") {
+
+                maxLimit = 0.06;
+                rmsLimit = 0.03;
+
+            } else if (extension == "m4a" || extension == "aac") {
+
+                //!!! to be worked out
+                maxLimit = 1e-10;
+                rmsLimit = 1e-10;
+
+            } else if (extension == "mp3") {
+
+                // all mp3 figures are worse when not normalising
+                maxLimit = 0.1;
+                rmsLimit = 0.05;
+
+            } else {
+
+                // supposed to be lossless then (wav, aiff, flac)
+                
+                if (bitdepth >= 16 && !resampled) {
+                    maxLimit = 1e-3;
+                    rmsLimit = 3e-4;
+                } else {
+                    maxLimit = 0.02;
+                    rmsLimit = 0.01;
+                }
+            }
+        }
+    }
+
+    QString testName(QString filename, int rate, bool norm, bool gapless) {
+        return QString("%1 at %2%3%4")
+            .arg(filename)
+            .arg(rate)
+            .arg(norm ? " normalised": "")
+            .arg(gapless ? "" : " non-gapless");
+    }
+
 private slots:
     void init()
     {
@@ -48,35 +171,66 @@
             cerr << "ERROR: Audio test file directory \"" << audioDir << "\" does not exist" << endl;
             QVERIFY2(QDir(audioDir).exists(), "Audio test file directory not found");
         }
+        if (!QDir(diffDir).exists() && !QDir().mkpath(diffDir)) {
+            cerr << "ERROR: Audio diff directory \"" << diffDir << "\" does not exist and could not be created" << endl;
+            QVERIFY2(QDir(diffDir).exists(), "Audio diff directory not found and could not be created");
+        }
     }
 
     void read_data()
     {
         QTest::addColumn<QString>("audiofile");
+        QTest::addColumn<int>("rate");
+        QTest::addColumn<bool>("normalised");
+        QTest::addColumn<bool>("gapless");
         QStringList files = QDir(audioDir).entryList(QDir::Files);
+        int readRates[] = { 44100, 48000 };
+        bool norms[] = { false, true };
+        bool gaplesses[] = { true, false };
         foreach (QString filename, files) {
-            QTest::newRow(strOf(filename)) << filename;
+            for (int rate: readRates) {
+                for (bool norm: norms) {
+                    for (bool gapless: gaplesses) {
+
+                        if (QFileInfo(filename).suffix() != "mp3" &&
+                            !gapless) {
+                            continue;
+                        }
+                        
+                        QString desc = testName(filename, rate, norm, gapless);
+
+                        QTest::newRow(strOf(desc))
+                            << filename << rate << norm << gapless;
+                    }
+                }
+            }
         }
     }
 
     void read()
     {
         QFETCH(QString, audiofile);
+        QFETCH(int, rate);
+        QFETCH(bool, normalised);
+        QFETCH(bool, gapless);
 
-        sv_samplerate_t readRate = 48000;
+        sv_samplerate_t readRate(rate);
+        
+        cerr << "\naudiofile = " << audiofile << endl;
+
+        AudioFileReaderFactory::Parameters params;
+        params.targetRate = readRate;
+        params.normalisation = (normalised ?
+                                AudioFileReaderFactory::Normalisation::Peak :
+                                AudioFileReaderFactory::Normalisation::None);
+        params.gaplessMode = (gapless ?
+                              AudioFileReaderFactory::GaplessMode::Gapless :
+                              AudioFileReaderFactory::GaplessMode::Gappy);
 
 	AudioFileReader *reader =
 	    AudioFileReaderFactory::createReader
-	    (audioDir + "/" + audiofile, readRate);
-
-        QStringList fileAndExt = audiofile.split(".");
-        QStringList bits = fileAndExt[0].split("-");
-        QString extension = fileAndExt[1];
-        sv_samplerate_t nominalRate = bits[0].toInt();
-        int nominalChannels = bits[1].toInt();
-        int nominalDepth = 16;
-        if (bits.length() > 2) nominalDepth = bits[2].toInt();
-
+	    (audioDir + "/" + audiofile, params);
+        
 	if (!reader) {
 #if ( QT_VERSION >= 0x050000 )
 	    QSKIP("Unsupported file, skipping");
@@ -85,11 +239,25 @@
 #endif
 	}
 
-        QCOMPARE((int)reader->getChannelCount(), nominalChannels);
-        QCOMPARE(reader->getNativeRate(), nominalRate);
+        QString extension;
+        sv_samplerate_t fileRate;
+        int channels;
+        int fileBitdepth;
+        getFileMetadata(audiofile, extension, fileRate, channels, fileBitdepth);
+
+        QString diffFile = testName(audiofile, rate, normalised, gapless);
+        diffFile.replace(".", "_");
+        diffFile.replace(" ", "_");
+        diffFile += ".wav";
+        diffFile = QDir(diffDir).filePath(diffFile);
+        WavFileWriter diffWriter(diffFile, readRate, channels,
+                                 WavFileWriter::WriteToTarget); //!!! NB WriteToTemporary not working, why?
+        QVERIFY(diffWriter.isOK());
+        
+        QCOMPARE((int)reader->getChannelCount(), channels);
+        QCOMPARE(reader->getNativeRate(), fileRate);
         QCOMPARE(reader->getSampleRate(), readRate);
 
-	int channels = reader->getChannelCount();
 	AudioTestData tdata(readRate, channels);
 	
 	float *reference = tdata.getInterleavedData();
@@ -103,123 +271,151 @@
 	vector<float> test = reader->getInterleavedFrames(0, refFrames + 5000);
 	sv_frame_t read = test.size() / channels;
 
-        if (extension == "mp3" || extension == "aac" || extension == "m4a") {
-            // mp3s and aacs can have silence at start and end
+        bool perceptual = (extension == "mp3" ||
+                           extension == "aac" ||
+                           extension == "m4a");
+        
+        if (perceptual && !gapless) {
+            // allow silence at start and end
             QVERIFY(read >= refFrames);
         } else {
             QCOMPARE(read, refFrames);
         }
 
-        // Our limits are pretty relaxed -- we're not testing decoder
-        // or resampler quality here, just whether the results are
-        // plainly wrong (e.g. at wrong samplerate or with an offset).
-
-	double maxLimit = 0.01;
-        double meanLimit = 0.001;
-        double edgeLimit = maxLimit * 10; // in first or final edgeSize frames
+        bool resampled = readRate != fileRate;
+        double maxLimit, rmsLimit;
+        getExpectedThresholds(audiofile,
+                              resampled,
+                              gapless,
+                              normalised,
+                              maxLimit, rmsLimit);
+        
+        double edgeLimit = maxLimit * 3; // in first or final edgeSize frames
+        if (resampled && edgeLimit < 0.1) edgeLimit = 0.1;
         int edgeSize = 100; 
 
-        if (nominalDepth < 16) {
-            maxLimit = 0.02;
-            meanLimit = 0.02;
-        } else if (extension == "ogg" || extension == "mp3") {
-            maxLimit = 0.1;
-            meanLimit = 0.035;
-            edgeLimit = maxLimit * 3;
-        } else if (extension == "aac" || extension == "m4a") {
-            maxLimit = 0.3; // seems max diff can be quite large here
-                            // even when mean is fairly small
-            meanLimit = 0.01;
-            edgeLimit = maxLimit * 3;
-        }
-
         // And we ignore completely the last few frames when upsampling
-        int discard = 1 + int(round(readRate / nominalRate));
+        int discard = 1 + int(round(readRate / fileRate));
 
         int offset = 0;
 
-        if (extension == "aac" || extension == "m4a") {
-            // our m4a file appears to have a fixed offset of 1024 (at
-            // file sample rate)
-            //            offset = int(round((1024 / nominalRate) * readRate));
-            offset = 0;
-        }
+        if (perceptual) {
 
-        if (extension == "mp3") {
-            // ...while mp3s appear to vary. What we're looking for is
+            // Look for an initial offset. What we're looking for is
             // the first peak of the sinusoid in the first channel
             // (since we may have only the one channel). This should
-            // appear at 0.4ms (see AudioTestData.h)
+            // appear at 0.4ms (see AudioTestData.h).
+            
             int expectedPeak = int(0.0004 * readRate);
-//            std::cerr << "expectedPeak = " << expectedPeak << std::endl;
             for (int i = 1; i < read; ++i) {
                 if (test[i * channels] > 0.8 &&
                     test[(i+1) * channels] < test[i * channels]) {
                     offset = i - expectedPeak - 1;
-//                    std::cerr << "actual peak = " << i-1 << std::endl;
                     break;
                 }
             }
-//            std::cerr << "offset = " << offset << std::endl;
+
+            std::cerr << "offset = " << offset << std::endl;
+            std::cerr << "at file rate would be " << (offset / readRate) * fileRate << std::endl;
+
+            // Previously our m4a test file had a fixed offset of 1024
+            // at the file sample rate -- this may be because it was
+            // produced by FAAC which did not write in the delay as
+            // metadata? We now have an m4a produced by Core Audio
+            // which gives a 0 offset. What to do...
+
+            // Anyway, mp3s should have 0 offset in gapless mode and
+            // "something else" otherwise.
+            
+            if (gapless) {
+                QCOMPARE(offset, 0);
+            }
         }
 
+        vector<vector<float>> diffs(channels);
+            
 	for (int c = 0; c < channels; ++c) {
+
+            double maxDiff = 0.0;
+            double totalDiff = 0.0;
+            double totalSqrDiff = 0.0;
+	    int maxIndex = 0;
+
+//            cerr << "\nchannel " << c << ": ";
             
-	    float maxdiff = 0.f;
-	    int maxAt = 0;
-	    float totdiff = 0.f;
-
 	    for (int i = 0; i < refFrames; ++i) {
                 int ix = i + offset;
                 if (ix >= read) {
                     cerr << "ERROR: audiofile " << audiofile << " reads truncated (read-rate reference frames " << i << " onward, of " << refFrames << ", are lost)" << endl;
                     QVERIFY(ix < read);
                 }
+
+                float signeddiff =
+                    test[ix * channels + c] -
+                    reference[i * channels + c];
+                    
+                diffs[c].push_back(signeddiff);
+
                 if (ix + discard >= read) {
                     // we forgive the very edge samples when
                     // resampling (discard > 0)
                     continue;
                 }
-		float diff = fabsf(test[ix * channels + c] -
-				   reference[i * channels + c]);
-		totdiff += diff;
+                
+		double diff = fabs(signeddiff);
+
+		totalDiff += diff;
+                totalSqrDiff += diff * diff;
+                
                 // in edge areas, record this only if it exceeds edgeLimit
-                if (i < edgeSize || i + edgeSize >= read - offset) {
-                    if (diff > edgeLimit && diff > maxdiff) {
-                        maxdiff = diff;
-                        maxAt = i;
+                if (i < edgeSize || i + edgeSize >= refFrames) {
+                    if (diff > edgeLimit && diff > maxDiff) {
+                        maxDiff = diff;
+                        maxIndex = i;
                     }
                 } else {
-                    if (diff > maxdiff) {
-                        maxdiff = diff;
-                        maxAt = i;
+                    if (diff > maxDiff) {
+                        maxDiff = diff;
+                        maxIndex = i;
                     }
 		}
 	    }
+                
+	    double meanDiff = totalDiff / double(refFrames);
+            double rmsDiff = sqrt(totalSqrDiff / double(refFrames));
 
-            // check for spurious material at end
+	    cerr << "channel " << c << ": mean diff " << meanDiff << endl;
+	    cerr << "channel " << c << ":  rms diff " << rmsDiff << endl;
+	    cerr << "channel " << c << ":  max diff " << maxDiff << " at " << maxIndex << endl;
+            
+            if (rmsDiff >= rmsLimit) {
+		cerr << "ERROR: for audiofile " << audiofile << ": RMS diff = " << rmsDiff << " for channel " << c << " (limit = " << rmsLimit << ")" << endl;
+                QVERIFY(rmsDiff < rmsLimit);
+            }
+	    if (maxDiff >= maxLimit) {
+		cerr << "ERROR: for audiofile " << audiofile << ": max diff = " << maxDiff << " at frame " << maxIndex << " of " << read << " on channel " << c << " (limit = " << maxLimit << ", edge limit = " << edgeLimit << ", mean diff = " << meanDiff << ", rms = " << rmsDiff << ")" << endl;
+		QVERIFY(maxDiff < maxLimit);
+	    }
+
+            // and check for spurious material at end
+            
             for (sv_frame_t i = refFrames; i + offset < read; ++i) {
                 sv_frame_t ix = i + offset;
-                float quiet = 1e-6f;
+                float quiet = 0.1; //!!! allow some ringing - but let's come back to this, it should tail off
                 float mag = fabsf(test[ix * channels + c]);
                 if (mag > quiet) {
-                    cerr << "ERROR: audiofile " << audiofile << " contains spurious data after end of reference (found sample " << test[ix * channels + c] << " at index " << ix << " of channel " << c << ")" << endl;
+                    cerr << "ERROR: audiofile " << audiofile << " contains spurious data after end of reference (found sample " << test[ix * channels + c] << " at index " << ix << " of channel " << c << " after reference+offset ended at " << refFrames+offset << ")" << endl;
                     QVERIFY(mag < quiet);
                 }
             }
-                
-	    float meandiff = totdiff / float(read);
-//	    cerr << "meandiff on channel " << c << ": " << meandiff << endl;
-//	    cerr << "maxdiff on channel " << c << ": " << maxdiff << " at " << maxAt << endl;
-            if (meandiff >= meanLimit) {
-		cerr << "ERROR: for audiofile " << audiofile << ": mean diff = " << meandiff << " for channel " << c << endl;
-                QVERIFY(meandiff < meanLimit);
-            }
-	    if (maxdiff >= maxLimit) {
-		cerr << "ERROR: for audiofile " << audiofile << ": max diff = " << maxdiff << " at frame " << maxAt << " of " << read << " on channel " << c << " (mean diff = " << meandiff << ")" << endl;
-		QVERIFY(maxdiff < maxLimit);
-	    }
 	}
+
+        float **ptrs = new float*[channels];
+        for (int c = 0; c < channels; ++c) {
+            ptrs[c] = diffs[c].data();
+        }
+        diffWriter.writeSamples(ptrs, refFrames);
+        delete[] ptrs;
     }
 };
 
--- a/data/model/ReadOnlyWaveFileModel.cpp	Tue Nov 29 17:09:07 2016 +0000
+++ b/data/model/ReadOnlyWaveFileModel.cpp	Thu Dec 01 17:45:40 2016 +0000
@@ -53,15 +53,31 @@
     m_lastDirectReadCount(0)
 {
     m_source.waitForData();
+
     if (m_source.isOK()) {
-        bool normalise = Preferences::getInstance()->getNormaliseAudio();
-        m_reader = AudioFileReaderFactory::createThreadingReader
-            (m_source, targetRate, normalise);
+
+        Preferences *prefs = Preferences::getInstance();
+        
+        AudioFileReaderFactory::Parameters params;
+        params.targetRate = targetRate;
+
+        params.normalisation = prefs->getNormaliseAudio() ?
+            AudioFileReaderFactory::Normalisation::Peak :
+            AudioFileReaderFactory::Normalisation::None;
+
+        params.gaplessMode = prefs->getUseGaplessMode() ?
+            AudioFileReaderFactory::GaplessMode::Gapless :
+            AudioFileReaderFactory::GaplessMode::Gappy;
+        
+        params.threadingMode = AudioFileReaderFactory::ThreadingMode::Threaded;
+
+        m_reader = AudioFileReaderFactory::createReader(m_source, params);
         if (m_reader) {
             SVDEBUG << "ReadOnlyWaveFileModel::ReadOnlyWaveFileModel: reader rate: "
                       << m_reader->getSampleRate() << endl;
         }
     }
+    
     if (m_reader) setObjectName(m_reader->getTitle());
     if (objectName() == "") setObjectName(QFileInfo(m_path).fileName());
     if (isOK()) fillCache();