Merge pull request mixxxdj#4855 from ronso0/rubberband3-switch

Rubberband: switch between v2 and v3 at runtime

src/engine/bufferscalers/enginebufferscalerubberband.cpp

@@ -30,7 +30,8 @@ EngineBufferScaleRubberBand::EngineBufferScaleRubberBand(
         ReadAheadManager* pReadAheadManager)
         : m_pReadAheadManager(pReadAheadManager),
           m_buffer_back(SampleUtil::alloc(MAX_BUFFER_LEN)),
-          m_bBackwards(false) {
+          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
@@ -122,10 +123,12 @@ void EngineBufferScaleRubberBand::onSampleRateChanged() {
         m_pRubberBand.reset();
         return;
     }
-    RubberBandStretcher::Options rubberbandOptions = RubberBandStretcher::OptionProcessRealTime;
+    RubberBandStretcher::Options rubberbandOptions =
+            RubberBandStretcher::OptionProcessRealTime;
 #if RUBBERBANDV3
-    // TODO make this a runtime option
-    rubberbandOptions |= RubberBandStretcher::OptionEngineFiner;
+    if (m_useEngineFiner) {
+        rubberbandOptions |= RubberBandStretcher::OptionEngineFiner;
+    }
 #endif
 
     m_pRubberBand = std::make_unique<RubberBandStretcher>(
@@ -270,6 +273,18 @@ double EngineBufferScaleRubberBand::scaleBuffer(
     return framesRead;
 }
 
+// static
+bool EngineBufferScaleRubberBand::isEngineFinerAvailable() {
+    return RUBBERBANDV3;
+}
+
+void EngineBufferScaleRubberBand::useEngineFiner(bool enable) {
+    if (isEngineFinerAvailable()) {
+        m_useEngineFiner = enable;
+        onSampleRateChanged();
+    }
+}
+
 int EngineBufferScaleRubberBand::runningEngineVersion() {
 #if RUBBERBANDV3
     return m_pRubberBand->getEngineVersion();

src/engine/bufferscalers/enginebufferscalerubberband.h

@@ -17,6 +17,12 @@ class EngineBufferScaleRubberBand : public EngineBufferScale {
             ReadAheadManager* pReadAheadManager);
     ~EngineBufferScaleRubberBand() override;
 
+    // Let EngineBuffer know if engine v3 is available
+    static bool isEngineFinerAvailable();
+
+    // Enable engine v3 if available
+    void useEngineFiner(bool enable);
+
     void setScaleParameters(double base_rate,
                             double* pTempoRatio,
                             double* pPitchRatio) override;
@@ -47,4 +53,6 @@ class EngineBufferScaleRubberBand : public EngineBufferScale {
 
     // Holds the playback direction
     bool m_bBackwards;
+
+    bool m_useEngineFiner;
 };

src/engine/enginebuffer.cpp

@@ -166,10 +166,6 @@ EngineBuffer::EngineBuffer(const QString& group,
 
     m_pSampleRate = new ControlProxy("[Master]", "samplerate", this);
 
-    m_pKeylockEngine = new ControlProxy("[Master]", "keylock_engine", this);
-    m_pKeylockEngine->connectValueChanged(this, &EngineBuffer::slotKeylockEngineChanged,
-                                          Qt::DirectConnection);
-
     m_pTrackSamples = new ControlObject(ConfigKey(m_group, "track_samples"));
     m_pTrackSampleRate = new ControlObject(ConfigKey(m_group, "track_samplerate"));
 
@@ -239,6 +235,10 @@ EngineBuffer::EngineBuffer(const QString& group,
                                                m_pLoopingControl);
     m_pReadAheadManager->addRateControl(m_pRateControl);
 
+    m_pKeylockEngine = new ControlProxy("[Master]", "keylock_engine", this);
+    m_pKeylockEngine->connectValueChanged(this,
+            &EngineBuffer::slotKeylockEngineChanged,
+            Qt::DirectConnection);
     // Construct scaling objects
     m_pScaleLinear = new EngineBufferScaleLinear(m_pReadAheadManager);
     m_pScaleST = new EngineBufferScaleST(m_pReadAheadManager);
@@ -248,6 +248,7 @@ EngineBuffer::EngineBuffer(const QString& group,
     } else {
         m_pScaleKeylock = m_pScaleRB;
     }
+    slotKeylockEngineChanged(m_pKeylockEngine->get());
     m_pScaleVinyl = m_pScaleLinear;
     m_pScale = m_pScaleVinyl;
     m_pScale->clear();
@@ -711,13 +712,23 @@ void EngineBuffer::slotKeylockEngineChanged(double dIndex) {
     if (m_bScalerOverride) {
         return;
     }
-    // static_cast<KeylockEngine>(dIndex); direct cast produces a "not used" warning with gcc
-    int iEngine = static_cast<int>(dIndex);
-    KeylockEngine engine = static_cast<KeylockEngine>(iEngine);
-    if (engine == SOUNDTOUCH) {
+    const KeylockEngine engine = static_cast<KeylockEngine>(dIndex);
+    switch (engine) {
+    case KeylockEngine::SoundTouch:
         m_pScaleKeylock = m_pScaleST;
-    } else {
+        break;
+    case KeylockEngine::RubberBandFaster:
+        m_pScaleRB->useEngineFiner(false);
+        m_pScaleKeylock = m_pScaleRB;
+        break;
+    case KeylockEngine::RubberBandFiner:
+        m_pScaleRB->useEngineFiner(
+                true); // in case of Rubberband V2 it falls back to RUBBERBAND_FASTER
         m_pScaleKeylock = m_pScaleRB;
+        break;
+    default:
+        slotKeylockEngineChanged(static_cast<double>(defaultKeylockEngine()));
+        break;
     }
 }
 

src/engine/enginebuffer.h

@@ -5,8 +5,10 @@
 #include <QAtomicInt>
 #include <QMutex>
 #include <cfloat>
+#include <initializer_list>
 
 #include "control/controlvalue.h"
+#include "engine/bufferscalers/enginebufferscalerubberband.h"
 #include "engine/cachingreader/cachingreader.h"
 #include "engine/engineobject.h"
 #include "engine/sync/syncable.h"
@@ -76,10 +78,19 @@ class EngineBuffer : public EngineObject {
     };
     Q_DECLARE_FLAGS(SeekRequests, SeekRequest);
 
-    enum KeylockEngine {
-        SOUNDTOUCH,
-        RUBBERBAND,
-        KEYLOCK_ENGINE_COUNT,
+    // This enum is also used in mixxx.cfg
+    // Don't remove or swap values to keep backward compatibility
+    enum class KeylockEngine {
+        SoundTouch = 0,
+        RubberBandFaster = 1,
+        RubberBandFiner = 2,
+    };
+
+    // intended for iteration over the KeylockEngine enum
+    constexpr static std::initializer_list<KeylockEngine> kKeylockEngines = {
+            KeylockEngine::SoundTouch,
+            KeylockEngine::RubberBandFaster,
+            KeylockEngine::RubberBandFiner,
     };
 
     // This value is used to make sure the initial seek after loading a track is
@@ -144,15 +155,37 @@ class EngineBuffer : public EngineObject {
 
     static QString getKeylockEngineName(KeylockEngine engine) {
         switch (engine) {
-        case SOUNDTOUCH:
+        case KeylockEngine::SoundTouch:
             return tr("Soundtouch (faster)");
-        case RUBBERBAND:
+        case KeylockEngine::RubberBandFaster:
             return tr("Rubberband (better)");
+        case KeylockEngine::RubberBandFiner:
+            if (EngineBufferScaleRubberBand::isEngineFinerAvailable()) {
+                return tr("Rubberband R3 (near-hi-fi quality)");
+            }
+            [[fallthrough]];
         default:
-            return tr("Unknown (bad value)");
+            return tr("Unknown, using Rubberband (better)");
         }
     }
 
+    static bool isKeylockEngineAvailable(KeylockEngine engine) {
+        switch (engine) {
+        case KeylockEngine::SoundTouch:
+            return true;
+        case KeylockEngine::RubberBandFaster:
+            return true;
+        case KeylockEngine::RubberBandFiner:
+            return EngineBufferScaleRubberBand::isEngineFinerAvailable();
+        default:
+            return false;
+        }
+    }
+
+    constexpr static KeylockEngine defaultKeylockEngine() {
+        return KeylockEngine::RubberBandFaster;
+    }
+
     // Request that the EngineBuffer load a track. Since the process is
     // asynchronous, EngineBuffer will emit a trackLoaded signal when the load
     // has completed.
@@ -410,4 +443,5 @@ class EngineBuffer : public EngineObject {
     QSharedPointer<VisualPlayPosition> m_visualPlayPos;
 };
 
+Q_DECLARE_METATYPE(EngineBuffer::KeylockEngine)
 Q_DECLARE_OPERATORS_FOR_FLAGS(EngineBuffer::SeekRequests)

src/engine/enginemaster.cpp

@@ -173,10 +173,9 @@ EngineMaster::EngineMaster(
             ConfigKey(EngineXfader::kXfaderConfigKey, "xFaderReverse"));
     m_pXFaderReverse->setButtonMode(ControlPushButton::TOGGLE);
 
-    m_pKeylockEngine = new ControlObject(ConfigKey(group, "keylock_engine"),
-                                         true, false, true);
-    m_pKeylockEngine->set(pConfig->getValueString(
-            ConfigKey(group, "keylock_engine")).toDouble());
+    m_pKeylockEngine = new ControlObject(ConfigKey(group, "keylock_engine"), true, false, true);
+    m_pKeylockEngine->set(pConfig->getValue(ConfigKey(group, "keylock_engine"),
+            static_cast<double>(EngineBuffer::defaultKeylockEngine())));
 
     // TODO: Make this read only and make EngineMaster decide whether
     // processing the master mix is necessary.

src/preferences/dialog/dlgprefsound.cpp

@@ -83,10 +83,11 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
             &DlgPrefSound::engineClockChanged);
 
     keylockComboBox->clear();
-    for (int i = 0; i < EngineBuffer::KEYLOCK_ENGINE_COUNT; ++i) {
-        keylockComboBox->addItem(
-                EngineBuffer::getKeylockEngineName(
-                        static_cast<EngineBuffer::KeylockEngine>(i)));
+    for (const auto engine : EngineBuffer::kKeylockEngines) {
+        if (EngineBuffer::isKeylockEngineAvailable(engine)) {
+            keylockComboBox->addItem(
+                    EngineBuffer::getKeylockEngineName(engine), QVariant::fromValue(engine));
+        }
     }
 
     m_pLatencyCompensation = new ControlProxy("[Master]", "microphoneLatencyCompensation", this);
@@ -307,9 +308,9 @@ void DlgPrefSound::slotApply() {
     SoundDeviceError err = SOUNDDEVICE_ERROR_OK;
     {
         ScopedWaitCursor cursor;
-        m_pKeylockEngine->set(keylockComboBox->currentIndex());
+        m_pKeylockEngine->set(keylockComboBox->currentData().toDouble());
         m_pSettings->set(ConfigKey("[Master]", "keylock_engine"),
-                       ConfigValue(keylockComboBox->currentIndex()));
+                ConfigValue(keylockComboBox->currentData().toInt()));
 
         err = m_pSoundManager->setConfig(m_config);
     }
@@ -492,10 +493,19 @@ void DlgPrefSound::loadSettings(const SoundManagerConfig &config) {
         engineClockComboBox->setCurrentIndex(0);
     }
 
-    // Default keylock is Rubberband.
-    int keylock_engine = m_pSettings->getValue(
-            ConfigKey("[Master]", "keylock_engine"), 1);
-    keylockComboBox->setCurrentIndex(keylock_engine);
+    // Default keylock engine is Rubberband Faster (v2)
+    const auto keylockEngine = static_cast<EngineBuffer::KeylockEngine>(
+            m_pSettings->getValue(ConfigKey("[Master]", "keylock_engine"),
+                    static_cast<int>(EngineBuffer::defaultKeylockEngine())));
+    const auto keylockEngineVariant = QVariant::fromValue(keylockEngine);
+    const int index = keylockComboBox->findData(keylockEngineVariant);
+    if (index >= 0) {
+        keylockComboBox->setCurrentIndex(index);
+    } else {
+        keylockComboBox->addItem(
+                EngineBuffer::getKeylockEngineName(keylockEngine), keylockEngineVariant);
+        keylockComboBox->setCurrentIndex(keylockComboBox->count() - 1);
+    }
 
     m_loading = false;
     // DlgPrefSoundItem has it's own inhibit flag
@@ -705,8 +715,14 @@ void DlgPrefSound::slotResetToDefaults() {
     SoundManagerConfig newConfig(m_pSoundManager);
     newConfig.loadDefaults(m_pSoundManager, SoundManagerConfig::ALL);
     loadSettings(newConfig);
-    keylockComboBox->setCurrentIndex(EngineBuffer::RUBBERBAND);
-    m_pKeylockEngine->set(EngineBuffer::RUBBERBAND);
+
+    const auto keylockEngine = EngineBuffer::defaultKeylockEngine();
+    const int index = keylockComboBox->findData(QVariant::fromValue(keylockEngine));
+    DEBUG_ASSERT(index >= 0);
+    if (index >= 0) {
+        keylockComboBox->setCurrentIndex(index);
+    }
+    m_pKeylockEngine->set(static_cast<double>(keylockEngine));
 
     masterMixComboBox->setCurrentIndex(1);
     m_pMasterEnabled->set(1.0);

src/test/enginebuffertest.cpp

@@ -103,7 +103,7 @@ TEST_F(EngineBufferTest, SlowRubberBand) {
 
     // With Soundtouch, it should switch the scaler as well
     ControlObject::set(ConfigKey("[Master]", "keylock_engine"),
-                       static_cast<double>(EngineBuffer::SOUNDTOUCH));
+            static_cast<double>(EngineBuffer::KeylockEngine::SoundTouch));
     ProcessBuffer();
     EXPECT_EQ(m_pMockScaleVinyl1, m_pChannel1->getEngineBuffer()->m_pScale);
 
@@ -113,14 +113,14 @@ TEST_F(EngineBufferTest, SlowRubberBand) {
 
     // With Rubberband, and transport stopped it should be still keylock
     ControlObject::set(ConfigKey("[Master]", "keylock_engine"),
-                       static_cast<double>(EngineBuffer::RUBBERBAND));
+            static_cast<double>(EngineBuffer::KeylockEngine::RubberBandFaster));
     ControlObject::set(ConfigKey(m_sGroup1, "rateSearch"), 0.0);
     ProcessBuffer();
     EXPECT_EQ(m_pMockScaleKeylock1, m_pChannel1->getEngineBuffer()->m_pScale);
 
     ControlObject::set(ConfigKey(m_sGroup1, "rateSearch"), 0.0072);
 
-    // Paying at low rate, the vinyl scaler should be used
+    // Playing at low rate, the vinyl scaler should be used
     ProcessBuffer();
     EXPECT_EQ(m_pMockScaleVinyl1, m_pChannel1->getEngineBuffer()->m_pScale);
 }
@@ -205,7 +205,7 @@ TEST_F(EngineBufferE2ETest, SoundTouchCrashTest) {
     // Soundtouch has a bug where a pitch value of zero causes an infinite loop
     // and crash.
     ControlObject::set(ConfigKey("[Master]", "keylock_engine"),
-                       static_cast<double>(EngineBuffer::SOUNDTOUCH));
+            static_cast<double>(EngineBuffer::KeylockEngine::SoundTouch));
     ControlObject::set(ConfigKey(m_sGroup1, "pitch"), 1.2);
     ControlObject::set(ConfigKey(m_sGroup1, "rate"), 0.05);
     ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0);
@@ -279,7 +279,7 @@ TEST_F(EngineBufferE2ETest, ReverseTest) {
 TEST_F(EngineBufferE2ETest, DISABLED_SoundTouchToggleTest) {
    // Test various cases where SoundTouch toggles on and off.
    ControlObject::set(ConfigKey("[Master]", "keylock_engine"),
-                      static_cast<double>(EngineBuffer::SOUNDTOUCH));
+           static_cast<double>(EngineBuffer::KeylockEngine::SoundTouch));
    ControlObject::set(ConfigKey(m_sGroup1, "rate"), 0.5);
    ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0);
    ProcessBuffer();
@@ -305,7 +305,7 @@ TEST_F(EngineBufferE2ETest, DISABLED_SoundTouchToggleTest) {
 TEST_F(EngineBufferE2ETest, DISABLED_RubberbandToggleTest) {
    // Test various cases where Rubberband toggles on and off.
    ControlObject::set(ConfigKey("[Master]", "keylock_engine"),
-                      static_cast<double>(EngineBuffer::RUBBERBAND));
+           static_cast<double>(EngineBuffer::KeylockEngine::RubberBandFaster));
    ControlObject::set(ConfigKey(m_sGroup1, "rate"), 0.5);
    ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0);
    ProcessBuffer();
@@ -336,7 +336,7 @@ TEST_F(EngineBufferE2ETest, DISABLED_KeylockReverseTest) {
     // Confirm that when toggling reverse while keylock is on, interpolation
     // is smooth.
     ControlObject::set(ConfigKey("[Master]", "keylock_engine"),
-                       static_cast<double>(EngineBuffer::SOUNDTOUCH));
+            static_cast<double>(EngineBuffer::KeylockEngine::SoundTouch));
     ControlObject::set(ConfigKey(m_sGroup1, "keylockMode"),
                        0.0);
     ControlObject::set(ConfigKey(m_sGroup1, "rate"), 0.5);
@@ -365,7 +365,7 @@ TEST_F(EngineBufferE2ETest, SoundTouchReverseTest) {
     // This test must not crash when changing to reverse while pitch is tweaked
     // Testing bug #1458263
     ControlObject::set(ConfigKey("[Master]", "keylock_engine"),
-                       static_cast<double>(EngineBuffer::SOUNDTOUCH));
+            static_cast<double>(EngineBuffer::KeylockEngine::SoundTouch));
     ControlObject::set(ConfigKey(m_sGroup1, "pitch"), -1);
     ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0);
     ProcessBuffer();
@@ -379,7 +379,7 @@ TEST_F(EngineBufferE2ETest, RubberbandReverseTest) {
     // This test must not crash when changing to reverse while pitch is tweaked
     // Testing bug #1458263
     ControlObject::set(ConfigKey("[Master]", "keylock_engine"),
-                       static_cast<double>(EngineBuffer::RUBBERBAND));
+            static_cast<double>(EngineBuffer::KeylockEngine::RubberBandFaster));
     ControlObject::set(ConfigKey(m_sGroup1, "pitch"), -1);
     ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0);
     ProcessBuffer();