Follow-up: Verify properties of audio signals

build/depends.py

@@ -1073,6 +1073,7 @@ def sources(self, build):
                    "util/rotary.cpp",
                    "util/logging.cpp",
                    "util/cmdlineargs.cpp",
+                   "util/audiosignal.cpp",
 
                    '#res/mixxx.qrc'
                    ]

plugins/soundsourcem4a/SConscript

@@ -15,6 +15,7 @@ m4a_sources = [
     "sources/soundsource.cpp",
     "sources/soundsourceplugin.cpp",
     "sources/audiosource.cpp",
+    "util/audiosignal.cpp",
     "util/samplebuffer.cpp",
     "util/singularsamplebuffer.cpp",
     "util/sample.cpp",

plugins/soundsourcemediafoundation/SConscript

@@ -18,7 +18,7 @@ if int(build.flags['mediafoundation']):
     else:
         env["LINKFLAGS"].remove("/subsystem:windows,5.01")
     ssmediafoundation_bin = env.SharedLibrary('soundsourcemediafoundation',
-        ['soundsourcemediafoundation.cpp', 'sources/soundsourceplugin.cpp', 'sources/soundsource.cpp', 'sources/audiosource.cpp', 'util/samplebuffer.cpp', 'util/sample.cpp', 'track/trackmetadata.cpp', 'track/trackmetadatataglib.cpp', 'track/tracknumbers.cpp', 'track/replaygain.cpp', 'track/bpm.cpp' ],
+        ['soundsourcemediafoundation.cpp', 'sources/soundsourceplugin.cpp', 'sources/soundsource.cpp', 'sources/audiosource.cpp', 'util/audiosignal.cpp', 'util/samplebuffer.cpp', 'util/sample.cpp', 'track/trackmetadata.cpp', 'track/trackmetadatataglib.cpp', 'track/tracknumbers.cpp', 'track/replaygain.cpp', 'track/bpm.cpp' ],
         LINKCOM  = [env['LINKCOM'],
             'mt.exe -nologo -manifest ${TARGET}.manifest -outputresource:$TARGET;1'])
     Return("ssmediafoundation_bin")

plugins/soundsourcemediafoundation/soundsourcemediafoundation.cpp

@@ -496,7 +496,7 @@ bool SoundSourceMediaFoundation::configureAudioStream(const AudioSourceConfig& a
     } else {
         qDebug() << "Number of channels in input stream" << numChannels;
     }
-    if (audioSrcCfg.hasChannelCount()) {
+    if (audioSrcCfg.hasValidChannelCount()) {
         numChannels = audioSrcCfg.getChannelCount();
         hr = pAudioType->SetUINT32(
                 MF_MT_AUDIO_NUM_CHANNELS, numChannels);
@@ -520,7 +520,7 @@ bool SoundSourceMediaFoundation::configureAudioStream(const AudioSourceConfig& a
     } else {
         qDebug() << "Samples per second in input stream" << samplesPerSecond;
     }
-    if (audioSrcCfg.hasSamplingRate()) {
+    if (audioSrcCfg.hasValidSamplingRate()) {
         samplesPerSecond = audioSrcCfg.getSamplingRate();
         hr = pAudioType->SetUINT32(
                 MF_MT_AUDIO_SAMPLES_PER_SECOND, samplesPerSecond);

plugins/soundsourcewv/SConscript

@@ -15,6 +15,7 @@ wv_sources = [
     "sources/soundsource.cpp",
     "sources/soundsourceplugin.cpp",
     "sources/audiosource.cpp",
+    "util/audiosignal.cpp",
     "util/samplebuffer.cpp",
     "util/singularsamplebuffer.cpp",
     "util/sample.cpp",

src/sources/audiosource.cpp

@@ -92,4 +92,23 @@ SINT AudioSource::readSampleFramesStereo(
     }
 }
 
+bool AudioSource::verifyReadable() const {
+    bool result = AudioSignal::verifyReadable();
+    if (hasBitrate()) {
+        DEBUG_ASSERT_AND_HANDLE(isValidBitrate(m_bitrate)) {
+            qWarning() << "Invalid bitrate [kbps]:"
+                    << getBitrate();
+            // Don't set the result to false, because bitrate is only
+            // an  informational property that does not effect the ability
+            // to decode audio data!
+        }
+    }
+    if (isEmpty()) {
+        qWarning() << "AudioSource is empty and does not provide any audio data!";
+        // Don't set the result to false, even if reading from an empty source
+        // is pointless!
+    }
+    return result;
+}
+
 }

src/sources/audiosource.h

@@ -45,25 +45,20 @@ class AudioSource: public UrlResource, public AudioSignal {
     // The actual duration in seconds.
     // Well defined only for valid files!
     inline bool hasDuration() const {
-        return isValid();
+        return hasValidSamplingRate();
     }
     inline double getDuration() const {
         DEBUG_ASSERT(hasDuration()); // prevents division by zero
         return double(getFrameCount()) / double(getSamplingRate());
     }
 
-    // The bitrate is measured in kbit/s (kbps).
-    inline static bool isValidBitrate(SINT bitrate) {
-        return kBitrateZero < bitrate;
-    }
+    // The bitrate is optional and measured in kbit/s (kbps).
+    // It depends on the metadata and decoder if a value for the
+    // bitrate is available.
     inline bool hasBitrate() const {
-        return isValidBitrate(m_bitrate);
+        return kBitrateZero < m_bitrate;
     }
-    // Setting the bitrate is optional when opening a file.
-    // The bitrate is not needed for decoding, it is only used
-    // for informational purposes.
     inline SINT getBitrate() const {
-        DEBUG_ASSERT(hasBitrate()); // prevents reading an invalid bitrate
         return m_bitrate;
     }
 
@@ -182,6 +177,8 @@ class AudioSource: public UrlResource, public AudioSignal {
             SINT* pMaxFrameIndexOfInterval,
             SINT maxFrameIndexOfAudioSource);
 
+    bool verifyReadable() const override;
+
   protected:
     explicit AudioSource(const QUrl& url);
     explicit AudioSource(const AudioSource& other) = default;
@@ -191,6 +188,9 @@ class AudioSource: public UrlResource, public AudioSignal {
     }
     void setFrameCount(SINT frameCount);
 
+    inline static bool isValidBitrate(SINT bitrate) {
+        return kBitrateZero <= bitrate;
+    }
     void setBitrate(SINT bitrate);
 
     SINT getSampleBufferSize(

src/sources/soundsourcecoreaudio.cpp

@@ -78,7 +78,7 @@ SoundSource::OpenResult SoundSourceCoreAudio::tryOpen(const AudioSourceConfig& a
 
     // create the output format
     const UInt32 numChannels =
-            audioSrcCfg.hasChannelCount() ? audioSrcCfg.getChannelCount() : 2;
+            audioSrcCfg.hasValidChannelCount() ? audioSrcCfg.getChannelCount() : 2;
     m_outputFormat = CAStreamBasicDescription(m_inputFormat.mSampleRate,
             numChannels, CAStreamBasicDescription::kPCMFormatFloat32, true);
 

src/sources/soundsourceflac.cpp

@@ -415,7 +415,7 @@ void SoundSourceFLAC::flacMetadata(const FLAC__StreamMetadata* metadata) {
     {
         const SINT channelCount = metadata->data.stream_info.channels;
         if (isValidChannelCount(channelCount)) {
-            if (hasChannelCount()) {
+            if (hasValidChannelCount()) {
                 // already set before -> check for consistency
                 if (getChannelCount() != channelCount) {
                     qWarning() << "Unexpected channel count:"
@@ -431,7 +431,7 @@ void SoundSourceFLAC::flacMetadata(const FLAC__StreamMetadata* metadata) {
         }
         const SINT samplingRate = metadata->data.stream_info.sample_rate;
         if (isValidSamplingRate(samplingRate)) {
-            if (hasSamplingRate()) {
+            if (hasValidSamplingRate()) {
                 // already set before -> check for consistency
                 if (getSamplingRate() != samplingRate) {
                     qWarning() << "Unexpected sampling rate:"

src/sources/soundsourcemp3.cpp

@@ -185,8 +185,8 @@ void SoundSourceMp3::finishDecoding() {
 }
 
 SoundSource::OpenResult SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) {
-    DEBUG_ASSERT(!hasChannelCount());
-    DEBUG_ASSERT(!hasSamplingRate());
+    DEBUG_ASSERT(!hasValidChannelCount());
+    DEBUG_ASSERT(!hasValidSamplingRate());
 
     DEBUG_ASSERT(!m_file.isOpen());
     if (!m_file.open(QIODevice::ReadOnly)) {

src/sources/soundsourcepluginapi.h

@@ -1,8 +1,9 @@
 #ifndef MIXXX_SOUNDSOURCEPLUGINAPI_H
 #define MIXXX_SOUNDSOURCEPLUGINAPI_H
 
-#define MIXXX_SOUNDSOURCEPLUGINAPI_VERSION 12
+#define MIXXX_SOUNDSOURCEPLUGINAPI_VERSION 13
 // SoundSource Plugin API version history:
+//  13 - Mixxx 2.1.0 - New function in base class for verifying audio properties
 //  12 - Mixxx 2.1.0 - New result codes for opening files
 //  11 - Mixxx 2.1.0 - Add function for writing metadata to SoundSource
 //  10 - Mixxx 2.1.0 - Add priority to SoundSourceProvider interface

src/sources/soundsourceproxy.cpp

@@ -583,7 +583,7 @@ mixxx::AudioSourcePointer SoundSourceProxy::openAudioSource(const mixxx::AudioSo
                      << getUrl().toString()
                      << "with provider"
                      << getSoundSourceProvider()->getName();
-            if ((mixxx::SoundSource::OpenResult::SUCCEEDED == openResult) && m_pSoundSource->isValid()) {
+            if ((mixxx::SoundSource::OpenResult::SUCCEEDED == openResult) && m_pSoundSource->verifyReadable()) {
                 m_pAudioSource =
                         AudioSourceProxy::create(m_pTrack, m_pSoundSource);
                 if (m_pAudioSource->isEmpty()) {

src/util/audiosignal.cpp

@@ -0,0 +1,32 @@
+#include <QtDebug>
+
+#include "util/audiosignal.h"
+
+namespace mixxx {
+
+bool AudioSignal::verifyReadable() const {
+    bool result = true;
+    if (!hasValidChannelCount()) {
+        qWarning() << "Invalid number of channels:"
+                << getChannelCount()
+                << "is out of range ["
+                << kChannelCountMin
+                << ","
+                << kChannelCountMax
+                << "]";
+        result = false;
+    }
+    if (!hasValidSamplingRate()) {
+        qWarning() << "Invalid sampling rate [Hz]:"
+                << getSamplingRate()
+                << "is out of range ["
+                << kSamplingRateMin
+                << ","
+                << kSamplingRateMax
+                << "]";
+        result = false;
+    }
+    return result;
+}
+
+} // namespace mixxx

src/util/audiosignal.h

@@ -32,21 +32,25 @@ class AudioSignal {
     };
 
     static const SINT kChannelCountZero    = 0;
+    static const SINT kChannelCountDefault = kChannelCountZero;
     static const SINT kChannelCountMono    = 1;
+    static const SINT kChannelCountMin     = kChannelCountMono; // lower bound
     static const SINT kChannelCountStereo  = 2;
-    static const SINT kChannelCountDefault = kChannelCountZero;
+    static const SINT kChannelCountMax     = 256; // upper bound (8-bit unsigned integer)
 
     static bool isValidChannelCount(SINT channelCount) {
-        return kChannelCountZero < channelCount;
+        return (kChannelCountMin <= channelCount) && (kChannelCountMax >= channelCount);
     }
 
     static const SINT kSamplingRateZero    = 0;
-    static const SINT kSamplingRateMin     = 8000; // lower bound
-    static const SINT kSamplingRateMax     = 192000; // upper bound
+    static const SINT kSamplingRateDefault = kSamplingRateZero;
+    static const SINT kSamplingRateMin     = 8000; // lower bound (= minimum MP3 sampling rate)
+    static const SINT kSamplingRate32kHz   = 32000;
     static const SINT kSamplingRateCD      = 44100;
     static const SINT kSamplingRate48kHz   = 48000;
     static const SINT kSamplingRate96kHz   = 96000;
-    static const SINT kSamplingRateDefault = kSamplingRateZero;
+    static const SINT kSamplingRate192kHz  = 192000;
+    static const SINT kSamplingRateMax     = kSamplingRate192kHz; // upper bound
 
     static bool isValidSamplingRate(SINT samplingRate) {
         return (kSamplingRateMin <= samplingRate) && (kSamplingRateMax >= samplingRate);
@@ -56,13 +60,15 @@ class AudioSignal {
         : m_sampleLayout(sampleLayout),
           m_channelCount(kChannelCountDefault),
           m_samplingRate(kSamplingRateDefault) {
+        DEBUG_ASSERT(!hasValidChannelCount());
+        DEBUG_ASSERT(!hasValidSamplingRate());
     }
     AudioSignal(SampleLayout sampleLayout, SINT channelCount, SINT samplingRate)
         : m_sampleLayout(sampleLayout),
           m_channelCount(channelCount),
           m_samplingRate(samplingRate) {
-        DEBUG_ASSERT(kChannelCountZero <= m_channelCount);
-        DEBUG_ASSERT(kSamplingRateZero <= m_samplingRate);
+        DEBUG_ASSERT(kChannelCountZero <= m_channelCount); // unsigned value
+        DEBUG_ASSERT(kSamplingRateZero <= m_samplingRate); // unsigned value
     }
     virtual ~AudioSignal() {}
 
@@ -75,7 +81,7 @@ class AudioSignal {
     SINT getChannelCount() const {
         return m_channelCount;
     }
-    bool hasChannelCount() const {
+    bool hasValidChannelCount() const {
         return isValidChannelCount(getChannelCount());
     }
 
@@ -89,42 +95,54 @@ class AudioSignal {
     SINT getSamplingRate() const {
         return m_samplingRate;
     }
-    bool hasSamplingRate() const {
+    bool hasValidSamplingRate() const {
         return isValidSamplingRate(getSamplingRate());
     }
 
-    // Check for valid properties. Subclasses may override this function
-    // to add more constraints. Derived functions should always call the
-    // implementation of the super class and concatenate the result with
-    // && (logical and).
-    virtual bool isValid() const {
-        return hasChannelCount() && hasSamplingRate();
-    }
+    // Verifies various properties to ensure that the audio data is
+    // actually readable. Warning messages are logged for properties
+    // with invalid values for diagnostic purposes.
+    //
+    // Subclasses may override this function for checking additional
+    // properties in derived classes. Derived functions should always
+    // call the implementation of the super class first:
+    //
+    // bool DerivedClass::verifyReadable() const {
+    //     bool result = BaseClass::validate();
+    //     if (my property is invalid) {
+    //         qWarning() << ...warning message...
+    //         result = false;
+    //     }
+    //     return result;
+    // }
+    virtual bool verifyReadable() const;
 
     // Conversion: #samples / sample offset -> #frames / frame offset
     template<typename T>
     inline T samples2frames(T samples) const {
-        DEBUG_ASSERT(hasChannelCount());
+        DEBUG_ASSERT(hasValidChannelCount());
         DEBUG_ASSERT(0 == (samples % getChannelCount()));
         return samples / getChannelCount();
     }
 
     // Conversion: #frames / frame offset -> #samples / sample offset
     template<typename T>
     inline T frames2samples(T frames) const {
-        DEBUG_ASSERT(hasChannelCount());
+        DEBUG_ASSERT(hasValidChannelCount());
         return frames * getChannelCount();
     }
 
 protected:
     void setChannelCount(SINT channelCount) {
+        DEBUG_ASSERT(kChannelCountZero <= m_channelCount); // unsigned value
         m_channelCount = channelCount;
     }
     void resetChannelCount() {
         m_channelCount = kChannelCountDefault;
     }
 
     void setSamplingRate(SINT samplingRate) {
+        DEBUG_ASSERT(kSamplingRateZero <= m_samplingRate); // unsigned value
         m_samplingRate = samplingRate;
     }
     void resetSamplingRate() {