Add further asserts, comments and reword misleading symbols

Co-authored-by: Daniel Schürmann <[email protected]>
Signed-off-by: Antoine C <[email protected]>

src/audio/frame.h

@@ -45,7 +45,7 @@ class FramePos final {
     }
 
     static constexpr FramePos fromSamplePos(double samplePos,
-            mixxx::audio::SignalInfo signalInfo) {
+            const mixxx::audio::SignalInfo& signalInfo) {
         return FramePos(static_cast<double>(samplePos) / signalInfo.getChannelCount());
     }
 

src/engine/bufferscalers/enginebufferscale.cpp

@@ -5,7 +5,7 @@
 #include "soundio/soundmanagerconfig.h"
 
 EngineBufferScale::EngineBufferScale()
-        : m_outputSignal(
+        : m_signal(
                   mixxx::audio::SignalInfo(
                           mixxx::kEngineChannelOutputCount,
                           mixxx::audio::SampleRate())),
@@ -14,25 +14,25 @@ EngineBufferScale::EngineBufferScale()
           m_dTempoRatio(1.0),
           m_dPitchRatio(1.0),
           m_effectiveRate(1.0) {
-    DEBUG_ASSERT(!m_outputSignal.isValid());
+    DEBUG_ASSERT(!m_signal.isValid());
 }
 
-void EngineBufferScale::setOutputSignal(
+void EngineBufferScale::setSignal(
         mixxx::audio::SampleRate sampleRate,
         mixxx::audio::ChannelCount channelCount) {
     DEBUG_ASSERT(sampleRate.isValid());
     DEBUG_ASSERT(channelCount.isValid());
     bool changed = false;
-    if (sampleRate != m_outputSignal.getSampleRate()) {
-        m_outputSignal.setSampleRate(sampleRate);
+    if (sampleRate != m_signal.getSampleRate()) {
+        m_signal.setSampleRate(sampleRate);
         changed = true;
     }
-    if (channelCount != m_outputSignal.getChannelCount()) {
-        m_outputSignal.setChannelCount(channelCount);
+    if (channelCount != m_signal.getChannelCount()) {
+        m_signal.setChannelCount(channelCount);
         changed = true;
     }
     if (changed) {
-        onOutputSignalChanged();
+        onSignalChanged();
     }
-    DEBUG_ASSERT(m_outputSignal.isValid());
+    DEBUG_ASSERT(m_signal.isValid());
 }

src/engine/bufferscalers/enginebufferscale.h

@@ -43,12 +43,12 @@ class EngineBufferScale : public QObject {
     }
 
     // Set the desired output signal.
-    void setOutputSignal(
+    void setSignal(
             mixxx::audio::SampleRate sampleRate,
             mixxx::audio::ChannelCount channelCout);
 
     const mixxx::audio::SignalInfo& getOutputSignal() const {
-        return m_outputSignal;
+        return m_signal;
     }
 
     // Called from EngineBuffer when seeking, to ensure the buffers are flushed */
@@ -65,9 +65,9 @@ class EngineBufferScale : public QObject {
             SINT iOutputBufferSize) = 0;
 
   private:
-    mixxx::audio::SignalInfo m_outputSignal;
+    mixxx::audio::SignalInfo m_signal;
 
-    virtual void onOutputSignalChanged() = 0;
+    virtual void onSignalChanged() = 0;
 
   protected:
     double m_dBaseRate;

src/engine/bufferscalers/enginebufferscalelinear.cpp

@@ -17,15 +17,15 @@ EngineBufferScaleLinear::EngineBufferScaleLinear(ReadAheadManager *pReadAheadMan
       m_dOldRate(1.0),
       m_dCurrentFrame(0.0),
       m_dNextFrame(0.0) {
-    onOutputSignalChanged();
+    onSignalChanged();
     SampleUtil::clear(m_bufferInt, kiLinearScaleReadAheadLength);
 }
 
 EngineBufferScaleLinear::~EngineBufferScaleLinear() {
     SampleUtil::free(m_bufferInt);
 }
 
-void EngineBufferScaleLinear::onOutputSignalChanged() {
+void EngineBufferScaleLinear::onSignalChanged() {
     m_floorSampleOld.resize(getOutputSignal().getChannelCount());
     std::fill(m_floorSampleOld.begin(), m_floorSampleOld.end(), 0.0);
 }
@@ -44,7 +44,7 @@ void EngineBufferScaleLinear::clear() {
     // Clear out buffer and saved sample data
     m_bufferIntSize = 0;
     m_dNextFrame = 0;
-    onOutputSignalChanged();
+    onSignalChanged();
 }
 
 // laurent de soras - punked from musicdsp.org (mad props)

src/engine/bufferscalers/enginebufferscalelinear.h

@@ -24,7 +24,7 @@ class EngineBufferScaleLinear : public EngineBufferScale  {
                              double* pPitchRatio) override;
 
   private:
-    void onOutputSignalChanged() override;
+    void onSignalChanged() override;
 
     double do_scale(CSAMPLE* buf, SINT buf_size);
     SINT do_copy(CSAMPLE* buf, SINT buf_size);

src/engine/bufferscalers/enginebufferscalerubberband.cpp

@@ -25,7 +25,7 @@ EngineBufferScaleRubberBand::EngineBufferScaleRubberBand(
           m_useEngineFiner(false) {
     // Initialize the internal buffers to prevent re-allocations
     // in the real-time thread.
-    onOutputSignalChanged();
+    onSignalChanged();
 }
 
 void EngineBufferScaleRubberBand::setScaleParameters(double base_rate,
@@ -92,7 +92,7 @@ void EngineBufferScaleRubberBand::setScaleParameters(double base_rate,
     m_dPitchRatio = *pPitchRatio;
 }
 
-void EngineBufferScaleRubberBand::onOutputSignalChanged() {
+void EngineBufferScaleRubberBand::onSignalChanged() {
     // TODO: Resetting the sample rate will cause internal
     // memory allocations that may block the real-time thread.
     // When is this function actually invoked??
@@ -200,10 +200,20 @@ SINT EngineBufferScaleRubberBand::retrieveAndDeinterleave(
         break;
     default: {
         int chCount = getOutputSignal().getChannelCount();
+        // The buffers samples are ordered as following
+        //  m_buffers#1 = 11..
+        //  m_buffers#2 = 22..
+        //  m_buffers#3 = 33..
+        //  m_buffers#4 = 44..
+        //  m_buffers#X = XX..
+        // And need to be reordered as following in pBuffer
+        //  1234..X1234...X...
+        //
+        // Because of the unanticipated number of buffer and channel, we cannot
+        // use any SampleUtil in this case
         for (SINT frameIdx = 0; frameIdx < frames; ++frameIdx) {
             for (int channel = 0; channel < chCount; channel++) {
-                m_buffers[channel].data()[frameIdx] =
-                        pBuffer[frameIdx * chCount + channel];
+                pBuffer[frameIdx * chCount + channel] = m_buffers[channel].data()[frameIdx];
             }
         }
     } break;
@@ -243,10 +253,21 @@ void EngineBufferScaleRubberBand::deinterleaveAndProcess(
         break;
     default: {
         int chCount = getOutputSignal().getChannelCount();
-        for (SINT i = 0; i < frames; ++i) {
+        // The sampler are ordered as following in pBuffer
+        //    1234..X1234...X...
+        // And need to be reordered as following
+        // m_buffers#1 = 11..
+        // m_buffers#2 = 22..
+        // m_buffers#3 = 33..
+        // m_buffers#4 = 44..
+        // m_buffers#X = XX..
+        //
+        // Because of the unanticipated number of buffer and channel, we cannot
+        // use any SampleUtil in this case
+        for (SINT frameIdx = 0; frameIdx < frames; ++frameIdx) {
             for (int channel = 0; channel < chCount; channel++) {
-                m_buffers[channel].data()[i] =
-                        pBuffer[i * chCount + channel];
+                m_buffers[channel].data()[frameIdx] =
+                        pBuffer[frameIdx * chCount + channel];
             }
         }
     } break;
@@ -346,7 +367,7 @@ bool EngineBufferScaleRubberBand::isEngineFinerAvailable() {
 void EngineBufferScaleRubberBand::useEngineFiner(bool enable) {
     if (isEngineFinerAvailable()) {
         m_useEngineFiner = enable;
-        onOutputSignalChanged();
+        onSignalChanged();
     }
 }
 

src/engine/bufferscalers/enginebufferscalerubberband.h

@@ -42,7 +42,7 @@ class EngineBufferScaleRubberBand final : public EngineBufferScale {
 
   private:
     // Reset RubberBand library with new audio signal
-    void onOutputSignalChanged() override;
+    void onSignalChanged() override;
 
     /// Calls `m_pRubberBand->getPreferredStartPad()`, with backwards
     /// compatibility for older librubberband versions.

src/engine/bufferscalers/enginebufferscalest.cpp

@@ -37,7 +37,7 @@ EngineBufferScaleST::EngineBufferScaleST(ReadAheadManager* pReadAheadManager)
     m_pSoundTouch->setSetting(SETTING_USE_QUICKSEEK, 1);
     // Initialize the internal buffers to prevent re-allocations
     // in the real-time thread.
-    onOutputSignalChanged();
+    onSignalChanged();
 }
 
 EngineBufferScaleST::~EngineBufferScaleST() {
@@ -89,7 +89,7 @@ void EngineBufferScaleST::setScaleParameters(double base_rate,
     // changed direction. I removed it because this is handled by EngineBuffer.
 }
 
-void EngineBufferScaleST::onOutputSignalChanged() {
+void EngineBufferScaleST::onSignalChanged() {
     int backBufferSize = kBackBufferFrameSize * getOutputSignal().getChannelCount();
     if (m_bufferBack.size() == backBufferSize) {
         m_bufferBack.clear();

src/engine/bufferscalers/enginebufferscalest.h

@@ -32,7 +32,7 @@ class EngineBufferScaleST : public EngineBufferScale {
     void clear() override;
 
   private:
-    void onOutputSignalChanged() override;
+    void onSignalChanged() override;
 
     // The read-ahead manager that we use to fetch samples
     ReadAheadManager* m_pReadAheadManager;

src/engine/cachingreader/cachingreaderchunk.cpp

@@ -53,7 +53,7 @@ mixxx::IndexRange CachingReaderChunk::bufferSampleFrames(
                     mixxx::audio::ChannelCount::stereo() !=
             0) {
         // This happens if the audio source only contain a mono channel, or an
-        // uneven number of channel
+        // odd number of channel
         mixxx::AudioSourceStereoProxy audioSourceProxy(
                 pAudioSource,
                 tempOutputBuffer);

src/engine/cachingreader/cachingreaderworker.cpp

@@ -206,14 +206,20 @@ void CachingReaderWorker::loadTrack(const TrackPointer& pTrack) {
 
     // It is critical that the audio source doesn't contain more channels than
     // requested as this could lead to overflow when reading chunks
-    VERIFY_OR_DEBUG_ASSERT(m_pAudioSource->getSignalInfo().getChannelCount() <=
-            m_maxSupportedChannel) {
+    VERIFY_OR_DEBUG_ASSERT(m_pAudioSource->getSignalInfo().getChannelCount() >=
+                    mixxx::audio::ChannelCount::mono() &&
+            m_pAudioSource->getSignalInfo().getChannelCount() <=
+                    m_maxSupportedChannel) {
         m_pAudioSource.reset(); // Close open file handles
         const auto update = ReaderStatusUpdate::trackUnloaded();
         m_pReaderStatusFIFO->writeBlocking(&update, 1);
         emit trackLoadFailed(pTrack,
-                tr("The file '%1' could not be loaded.")
-                        .arg(QDir::toNativeSeparators(pTrack->getLocation())));
+                tr("The file '%1' could not be loaded because it contains %2 "
+                   "channels, and only 1 to %3 are supported.")
+                        .arg(QDir::toNativeSeparators(pTrack->getLocation()),
+                                QString::number(m_pAudioSource->getSignalInfo()
+                                                        .getChannelCount()),
+                                QString::number(m_maxSupportedChannel)));
         return;
     }
 

src/engine/enginebuffer.cpp

@@ -900,8 +900,8 @@ void EngineBuffer::processTrackLocked(
     // (1.0 being normal rate. 2.0 plays at 2x speed -- 2 track seconds
     // pass for every 1 real second). Depending on whether
     // keylock is enabled, this is applied to either the rate or the tempo.
-    int outputBufferSize = iBufferSize,
-        stereoPairCount = m_channelCount / mixxx::audio::ChannelCount::stereo();
+    int outputBufferSize = iBufferSize;
+    int stereoPairCount = m_channelCount / mixxx::audio::ChannelCount::stereo();
     // The speed is calculated out of the buffer size for the stereo channel
     // output, after mixing multi channel (stem) together
     if (stereoPairCount > 1) {
@@ -1194,10 +1194,10 @@ void EngineBuffer::process(CSAMPLE* pOutput, const int iBufferSize) {
     // If the sample rate has changed, force Rubberband to reset so that
     // it doesn't reallocate when the user engages keylock during playback.
     // We do this even if rubberband is not active.
-    m_pScaleLinear->setOutputSignal(m_sampleRate, m_channelCount);
-    m_pScaleST->setOutputSignal(m_sampleRate, m_channelCount);
+    m_pScaleLinear->setSignal(m_sampleRate, m_channelCount);
+    m_pScaleST->setSignal(m_sampleRate, m_channelCount);
 #ifdef __RUBBERBAND__
-    m_pScaleRB->setOutputSignal(m_sampleRate, m_channelCount);
+    m_pScaleRB->setSignal(m_sampleRate, m_channelCount);
 #endif
 
     bool hasStableTrack = m_pTrackLoaded->toBool() && m_iTrackLoading.loadAcquire() == 0;

src/test/enginebufferscalelineartest.cpp

@@ -82,7 +82,7 @@ class EngineBufferScaleLinearTest : public MixxxTest {
     void SetRate(double rate) {
         double tempoRatio = rate;
         double pitchRatio = rate;
-        m_pScaler->setOutputSignal(mixxx::audio::SampleRate(44100),
+        m_pScaler->setSignal(mixxx::audio::SampleRate(44100),
                 mixxx::audio::ChannelCount::stereo());
         m_pScaler->setScaleParameters(
                 1.0, &tempoRatio, &pitchRatio);

src/test/mockedenginebackendtest.h

@@ -55,7 +55,7 @@ class MockScaler : public EngineBufferScale {
     }
 
   private:
-    void onOutputSignalChanged() override {
+    void onSignalChanged() override {
     }
 
     double m_processedTempo;