Add better comments and various nits

src/engine/bufferscalers/enginebufferscalerubberband.cpp

@@ -141,12 +141,12 @@ SINT EngineBufferScaleRubberBand::retrieveAndDeinterleave(
     //       immediately read those frames in addition to the frames we actually
     //       need for the output
     const SINT frames_to_read = math_min(frames_available, frames + m_remainingPaddingInOutput);
-    DEBUG_ASSERT(frames_to_read <= MAX_BUFFER_LEN);
+    DEBUG_ASSERT(frames_to_read <= m_buffers[0].size());
     SINT received_frames;
     {
         ScopedTimer t(QStringLiteral("RubberBand::retrieve"));
         received_frames = static_cast<SINT>(m_rubberBand.retrieve(
-                m_bufferPtrs.data(), frames_to_read));
+                m_bufferPtrs.data(), frames + m_remainingPaddingInOutput, m_buffers[0].size()));
     }
     SINT frame_offset = 0;
 

src/engine/bufferscalers/rubberbandwrapper.cpp

@@ -3,6 +3,7 @@
 #include "engine/bufferscalers/rubberbandworkerpool.h"
 #include "engine/engine.h"
 #include "util/assert.h"
+#include "util/sample.h"
 
 using RubberBand::RubberBandStretcher;
 
@@ -45,40 +46,51 @@ int RubberBandWrapper::available() const {
     }
     return available == std::numeric_limits<int>::max() ? 0 : available;
 }
-size_t RubberBandWrapper::retrieve(float* const* output, size_t samples) const {
+size_t RubberBandWrapper::retrieve(
+        float* const* output, size_t samples, SINT channelBufferSize) const {
+    // ensure we don't fetch more samples than we really have available.
+    samples = std::min(static_cast<size_t>(available()), samples);
+    VERIFY_OR_DEBUG_ASSERT(samples <= channelBufferSize) {
+        return 0;
+    }
     if (m_pInstances.size() == 1) {
         return m_pInstances[0]->retrieve(output, samples);
     } else {
-        // ensure we don't fetch more samples than we really have available.
-        samples = std::min(static_cast<size_t>(available()), samples);
         for (const auto& stretcher : m_pInstances) {
-#ifdef MIXXX_DEBUG_ASSERTIONS_ENABLED
-            size_t numSamplesRequested =
-#endif
+            size_t numSamplesRetrieved =
                     stretcher->retrieve(output, samples);
             // there is something very wrong if we got a different of amount
             // of samples than we requested
-            DEBUG_ASSERT(numSamplesRequested == samples);
+            // We clear the buffer to limit the damage, but the signal
+            // interruption will still create undesirable audio artefacts.
+            VERIFY_OR_DEBUG_ASSERT(numSamplesRetrieved == samples) {
+                if (samples > numSamplesRetrieved) {
+                    for (int ch = 0; ch < getChannelPerWorker(); ch++) {
+                        SampleUtil::clear(output[ch] + numSamplesRetrieved,
+                                samples - numSamplesRetrieved);
+                    }
+                }
+            }
             output += getChannelPerWorker();
         }
         return samples;
     }
 }
 size_t RubberBandWrapper::getInputIncrement() const {
     VERIFY_OR_DEBUG_ASSERT(isValid()) {
-        return -1;
+        return {};
     }
     return m_pInstances[0]->getInputIncrement();
 }
 size_t RubberBandWrapper::getLatency() const {
     VERIFY_OR_DEBUG_ASSERT(isValid()) {
-        return -1;
+        return {};
     }
     return m_pInstances[0]->getLatency();
 }
 double RubberBandWrapper::getPitchScale() const {
     VERIFY_OR_DEBUG_ASSERT(isValid()) {
-        return -1;
+        return {};
     }
     return m_pInstances[0]->getPitchScale();
 }
@@ -87,7 +99,7 @@ double RubberBandWrapper::getPitchScale() const {
 // for how these two functions were implemented within librubberband itself
 size_t RubberBandWrapper::getPreferredStartPad() const {
     VERIFY_OR_DEBUG_ASSERT(isValid()) {
-        return -1;
+        return {};
     }
 #if RUBBERBANDV3
     return m_pInstances[0]->getPreferredStartPad();
@@ -101,7 +113,7 @@ size_t RubberBandWrapper::getPreferredStartPad() const {
 }
 size_t RubberBandWrapper::getStartDelay() const {
     VERIFY_OR_DEBUG_ASSERT(isValid()) {
-        return -1;
+        return {};
     }
 #if RUBBERBANDV3
     return m_pInstances[0]->getStartDelay();
@@ -121,11 +133,16 @@ void RubberBandWrapper::process(const float* const* input, size_t samples, bool
         RubberBandWorkerPool* pPool = RubberBandWorkerPool::instance();
         for (auto& pInstance : m_pInstances) {
             pInstance->set(input, samples, isFinal);
+            // We try to get the stretching job ran by the RBPool if there is a
+            // worker slot available
             if (!pPool->tryStart(pInstance.get())) {
+                // Otherwise, it means the main thread should take care of the stretching
                 pInstance->run();
             }
             input += pPool->channelPerWorker();
         }
+        // We always perform a wait, even for task that were ran in the main
+        // thread, so it resets the semaphore
         for (auto& pInstance : m_pInstances) {
             pInstance->waitReady();
         }
@@ -150,6 +167,10 @@ void RubberBandWrapper::setup(mixxx::audio::SampleRate sampleRate,
     auto channelPerWorker = getChannelPerWorker();
     qDebug() << "RubberBandWrapper::setup" << channelPerWorker;
     VERIFY_OR_DEBUG_ASSERT(0 == chCount % channelPerWorker) {
+        // If we have an uneven number of channel, which we can't evenly
+        // distribute across the RubberBandPool workers, we fallback to using a
+        // single instance to limit the audio impefection that may come from
+        // using RB withg different parameters.
         m_pInstances.emplace_back(
                 std::make_unique<RubberBandTask>(
                         sampleRate, chCount, opt));

src/engine/bufferscalers/rubberbandwrapper.h

@@ -12,7 +12,7 @@ class RubberBandWrapper {
     void setTimeRatio(double ratio);
     std::size_t getSamplesRequired() const;
     int available() const;
-    size_t retrieve(float* const* output, size_t samples) const;
+    size_t retrieve(float* const* output, size_t samples, SINT channelBufferSize) const;
     size_t getInputIncrement() const;
     size_t getLatency() const;
     double getPitchScale() const;

src/preferences/dialog/dlgprefsound.cpp

@@ -37,14 +37,23 @@ bool soundItemAlreadyExists(const AudioPath& output, const QWidget& widget) {
 }
 
 #ifdef __RUBBERBAND__
-const QString kKeylockMultiThreadedAvailable = QObject::tr(
-        "<p><span style=\"font-weight:600;\">Warning!</span></p><p>Using multi "
-        "threading may result in pitch and tone imperfection, and this is "
-        "mono-incompatible, due to third party limitations.</p>");
-const QString kKeylockMultiThreadedUnavailableMono = QObject::tr(
-        "<i>Multi threading mode is incompatible with mono main mix.</i>");
-const QString kKeylockMultiThreadedUnavailableRubberband = QObject::tr(
-        "<i>Multi threading mode is only available with RubberBand.</i>");
+const QString kKeylockMultiThreadedAvailable =
+        QStringLiteral("<p><span style=\"font-weight:600;\">") +
+        QObject::tr("Warning!") + QStringLiteral("</span></p><p>") +
+        QObject::tr(
+                "Using multi "
+                "threading may result in pitch and tone imperfection, and this "
+                "is "
+                "mono-incompatible, due to third party limitations.") +
+        QStringLiteral("</p>");
+const QString kKeylockMultiThreadedUnavailableMono = QStringLiteral("<i>") +
+        QObject::tr(
+                "Multi threading mode is incompatible with mono main mix.") +
+        QStringLiteral("</i>");
+const QString kKeylockMultiThreadedUnavailableRubberband =
+        QStringLiteral("<i>") +
+        QObject::tr("Multi threading mode is only available with RubberBand.") +
+        QStringLiteral("</i>");
 #endif
 } // namespace