Add missing Rubber Band padding, preventing it from eating the initial transient

src/engine/bufferscalers/enginebufferscalerubberband.cpp

@@ -1,7 +1,5 @@
 #include "engine/bufferscalers/enginebufferscalerubberband.h"
 
-#include <rubberband/RubberBandStretcher.h>
-
 #include <QtDebug>
 
 #include "control/controlobject.h"
@@ -19,7 +17,7 @@ namespace {
 // TODO (XXX): this should be removed. It is only needed to work around
 // a Rubberband 1.3 bug.
 // This is the default increment from RubberBand 1.8.1.
-size_t kRubberBandBlockSize = 256;
+constexpr size_t kRubberBandBlockSize = 256;
 
 #define RUBBERBANDV3 (RUBBERBAND_API_MAJOR_VERSION >= 2 && RUBBERBAND_API_MINOR_VERSION >= 7)
 
@@ -28,22 +26,16 @@ size_t kRubberBandBlockSize = 256;
 EngineBufferScaleRubberBand::EngineBufferScaleRubberBand(
         ReadAheadManager* pReadAheadManager)
         : m_pReadAheadManager(pReadAheadManager),
-          m_buffer_back(SampleUtil::alloc(MAX_BUFFER_LEN)),
+          m_buffers{mixxx::SampleBuffer(MAX_BUFFER_LEN), mixxx::SampleBuffer(MAX_BUFFER_LEN)},
+          m_bufferPtrs{m_buffers[0].data(), m_buffers[1].data()},
+          m_interleavedReadBuffer(MAX_BUFFER_LEN),
           m_bBackwards(false),
           m_useEngineFiner(false) {
-    m_retrieve_buffer[0] = SampleUtil::alloc(MAX_BUFFER_LEN);
-    m_retrieve_buffer[1] = SampleUtil::alloc(MAX_BUFFER_LEN);
     // Initialize the internal buffers to prevent re-allocations
     // in the real-time thread.
     onSampleRateChanged();
 }
 
-EngineBufferScaleRubberBand::~EngineBufferScaleRubberBand() {
-    SampleUtil::free(m_buffer_back);
-    SampleUtil::free(m_retrieve_buffer[0]);
-    SampleUtil::free(m_retrieve_buffer[1]);
-}
-
 void EngineBufferScaleRubberBand::setScaleParameters(double base_rate,
                                                      double* pTempoRatio,
                                                      double* pPitchRatio) {
@@ -142,7 +134,7 @@ void EngineBufferScaleRubberBand::clear() {
     VERIFY_OR_DEBUG_ASSERT(m_pRubberBand) {
         return;
     }
-    m_pRubberBand->reset();
+    reset();
 }
 
 SINT EngineBufferScaleRubberBand::retrieveAndDeinterleave(
@@ -151,22 +143,25 @@ SINT EngineBufferScaleRubberBand::retrieveAndDeinterleave(
     SINT frames_available = m_pRubberBand->available();
     SINT frames_to_read = math_min(frames_available, frames);
     SINT received_frames = static_cast<SINT>(m_pRubberBand->retrieve(
-            m_retrieve_buffer, frames_to_read));
+            m_bufferPtrs.data(), frames_to_read));
+
+    DEBUG_ASSERT(received_frames <= static_cast<ssize_t>(m_buffers[0].size()));
 
     SampleUtil::interleaveBuffer(pBuffer,
-                                 m_retrieve_buffer[0],
-                                 m_retrieve_buffer[1],
-                                 received_frames);
+            m_buffers[0].data(),
+            m_buffers[1].data(),
+            received_frames);
     return received_frames;
 }
 
 void EngineBufferScaleRubberBand::deinterleaveAndProcess(
         const CSAMPLE* pBuffer, SINT frames, bool flush) {
+    DEBUG_ASSERT(frames <= static_cast<ssize_t>(m_buffers[0].size()));
 
     SampleUtil::deinterleaveBuffer(
-            m_retrieve_buffer[0], m_retrieve_buffer[1], pBuffer, frames);
+            m_buffers[0].data(), m_buffers[1].data(), pBuffer, frames);
 
-    m_pRubberBand->process(m_retrieve_buffer,
+    m_pRubberBand->process(m_bufferPtrs.data(),
             frames,
             flush);
 }
@@ -199,10 +194,10 @@ double EngineBufferScaleRubberBand::scaleBuffer(
         read += getOutputSignal().frames2samples(received_frames);
 
         if (break_out_after_retrieve_and_reset_rubberband) {
-            //qDebug() << "break_out_after_retrieve_and_reset_rubberband";
+            // qDebug() << "break_out_after_retrieve_and_reset_rubberband";
             // If we break out early then we have flushed RubberBand and need to
             // reset it.
-            m_pRubberBand->reset();
+            reset();
             break;
         }
 
@@ -221,26 +216,26 @@ double EngineBufferScaleRubberBand::scaleBuffer(
                 iLenFramesRequired = kRubberBandBlockSize;
             }
         }
-        //qDebug() << "iLenFramesRequired" << iLenFramesRequired;
+        // qDebug() << "iLenFramesRequired" << iLenFramesRequired;
 
         if (remaining_frames > 0 && iLenFramesRequired > 0) {
             SINT iAvailSamples = m_pReadAheadManager->getNextSamples(
-                        // The value doesn't matter here. All that matters is we
-                        // are going forward or backward.
-                        (m_bBackwards ? -1.0 : 1.0) * m_dBaseRate * m_dTempoRatio,
-                        m_buffer_back,
-                        getOutputSignal().frames2samples(iLenFramesRequired));
+                    // The value doesn't matter here. All that matters is we
+                    // are going forward or backward.
+                    (m_bBackwards ? -1.0 : 1.0) * m_dBaseRate * m_dTempoRatio,
+                    m_interleavedReadBuffer.data(),
+                    getOutputSignal().frames2samples(iLenFramesRequired));
             SINT iAvailFrames = getOutputSignal().samples2frames(iAvailSamples);
 
             if (iAvailFrames > 0) {
                 last_read_failed = false;
-                deinterleaveAndProcess(m_buffer_back, iAvailFrames, false);
+                deinterleaveAndProcess(m_interleavedReadBuffer.data(), iAvailFrames, false);
             } else {
                 if (last_read_failed) {
                     // Flush and break out after the next retrieval. If we are
                     // at EOF this serves to get the last samples out of
                     // RubberBand.
-                    deinterleaveAndProcess(m_buffer_back, 0, true);
+                    deinterleaveAndProcess(m_interleavedReadBuffer.data(), 0, true);
                     break_out_after_retrieve_and_reset_rubberband = true;
                 }
                 last_read_failed = true;
@@ -286,3 +281,40 @@ int EngineBufferScaleRubberBand::runningEngineVersion() {
     return 2;
 #endif
 }
+
+void EngineBufferScaleRubberBand::reset() {
+    m_pRubberBand->reset();
+
+    // As mentioned in the docs (https://breakfastquay.com/rubberband/code-doc/)
+    // and FAQ (https://breakfastquay.com/rubberband/integration.html#faqs), you
+    // need to run some silent samples through the time stretching engine first
+    // before using it. Otherwise it will eat add a short fade-in, destroying
+    // the initial transient.
+#if RUBBERBANDV3
+    size_t remaining_padding = m_pRubberBand->getPreferredStartPad();
+#else
+    // This _should_ be equal to the latency in older Rubber Band versions:
+    // https://github.com/breakfastquay/rubberband/blob/c5f99d5ff2cba2f4f1def6c38c7843bbb9ac7a78/main/main.cpp#L652
+    size_t remaining_padding = m_pRubberBand->getLatency();
+#endif
+    std::fill_n(m_buffers[0].span().begin(), kRubberBandBlockSize, 0.0f);
+    std::fill_n(m_buffers[1].span().begin(), kRubberBandBlockSize, 0.0f);
+    while (remaining_padding > 0) {
+        const size_t pad_samples = std::min<size_t>(remaining_padding, kRubberBandBlockSize);
+        m_pRubberBand->process(m_bufferPtrs.data(), pad_samples, false);
+
+        remaining_padding -= pad_samples;
+    }
+
+#if RUBBERBANDV3
+    size_t padding_to_drop = m_pRubberBand->getStartDelay();
+#else
+    size_t padding_to_drop = m_pRubberBand->getLatency();
+#endif
+    while (padding_to_drop > 0) {
+        const size_t drop_samples = std::min<size_t>(padding_to_drop, kRubberBandBlockSize);
+        m_pRubberBand->retrieve(m_bufferPtrs.data(), drop_samples);
+
+        padding_to_drop -= drop_samples;
+    }
+}

src/engine/bufferscalers/enginebufferscalerubberband.h

@@ -1,21 +1,27 @@
 #pragma once
 
+#include <rubberband/RubberBandStretcher.h>
+
+#include <array>
+
 #include "engine/bufferscalers/enginebufferscale.h"
 #include "util/memory.h"
-
-namespace RubberBand {
-class RubberBandStretcher;
-}  // namespace RubberBand
+#include "util/samplebuffer.h"
 
 class ReadAheadManager;
 
 // Uses librubberband to scale audio.  This class is not thread safe.
-class EngineBufferScaleRubberBand : public EngineBufferScale {
+class EngineBufferScaleRubberBand final : public EngineBufferScale {
     Q_OBJECT
   public:
     explicit EngineBufferScaleRubberBand(
             ReadAheadManager* pReadAheadManager);
-    ~EngineBufferScaleRubberBand() override;
+
+    EngineBufferScaleRubberBand(const EngineBufferScaleRubberBand&) = delete;
+    EngineBufferScaleRubberBand& operator=(const EngineBufferScaleRubberBand&) = delete;
+
+    EngineBufferScaleRubberBand(EngineBufferScaleRubberBand&&) = delete;
+    EngineBufferScaleRubberBand& operator=(EngineBufferScaleRubberBand&&) = delete;
 
     // Let EngineBuffer know if engine v3 is available
     static bool isEngineFinerAvailable();
@@ -39,6 +45,10 @@ class EngineBufferScaleRubberBand : public EngineBufferScale {
     void onSampleRateChanged() override;
 
     int runningEngineVersion();
+    /// Reset the rubberband instance and run the prerequisite amount of padding
+    /// through it. This should be used instead of calling
+    /// `m_pRubberBand->reset()` directly.
+    void reset();
 
     void deinterleaveAndProcess(const CSAMPLE* pBuffer, SINT frames, bool flush);
     SINT retrieveAndDeinterleave(CSAMPLE* pBuffer, SINT frames);
@@ -48,8 +58,17 @@ class EngineBufferScaleRubberBand : public EngineBufferScale {
 
     std::unique_ptr<RubberBand::RubberBandStretcher> m_pRubberBand;
 
-    CSAMPLE* m_retrieve_buffer[2];
-    CSAMPLE* m_buffer_back;
+    /// The audio buffers samples used to send audio to Rubber Band and to
+    /// receive processed audio from Rubber Band. This is needed because Mixxx
+    /// uses interleaved buffers in most other places.
+    std::array<mixxx::SampleBuffer, 2> m_buffers;
+    /// These point to the buffers in `m_buffers`. They can be defined here
+    /// since this object cannot be moved or copied.
+    std::array<float*, 2> m_bufferPtrs;
+
+    /// Contains interleaved samples read from `m_pReadAheadManager`. These need
+    /// to be deinterleaved before they can be passed to Rubber Band.
+    mixxx::SampleBuffer m_interleavedReadBuffer;
 
     // Holds the playback direction
     bool m_bBackwards;