From 91beaea89636291766470c8923ebe824513cc30a Mon Sep 17 00:00:00 2001
From: ronso0 <ronso0@mixxx.org>
Date: Mon, 10 Feb 2025 14:09:36 +0100
Subject: [PATCH] PositionScratchController: fix false high rate seeking

---
 src/engine/controls/ratecontrol.cpp      |  4 ++
 src/engine/controls/ratecontrol.h        |  1 +
 src/engine/positionscratchcontroller.cpp | 51 +++++++++++++++++++-----
 src/engine/positionscratchcontroller.h   |  1 +
 4 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/src/engine/controls/ratecontrol.cpp b/src/engine/controls/ratecontrol.cpp
index 08cec27595d..7eb755a008e 100644
--- a/src/engine/controls/ratecontrol.cpp
+++ b/src/engine/controls/ratecontrol.cpp
@@ -613,3 +613,7 @@ void RateControl::notifyWrapAround(mixxx::audio::FramePos triggerPos,
     m_jumpPos = triggerPos;
     m_targetPos = targetPos;
 }
+
+void RateControl::notifySeek(mixxx::audio::FramePos position) {
+    m_pScratchController->notifySeek(position);
+}
diff --git a/src/engine/controls/ratecontrol.h b/src/engine/controls/ratecontrol.h
index f13836e98b2..594a0824d70 100644
--- a/src/engine/controls/ratecontrol.h
+++ b/src/engine/controls/ratecontrol.h
@@ -76,6 +76,7 @@ class RateControl : public EngineControl {
   // PositionScratchController can correctly interpret the sample position delta.
   void notifyWrapAround(mixxx::audio::FramePos triggerPos,
           mixxx::audio::FramePos targetPos);
+  void notifySeek(mixxx::audio::FramePos position) override;
 
 public slots:
   void slotRateRangeChanged(double);
diff --git a/src/engine/positionscratchcontroller.cpp b/src/engine/positionscratchcontroller.cpp
index 07962a5edb3..510c5b0496a 100644
--- a/src/engine/positionscratchcontroller.cpp
+++ b/src/engine/positionscratchcontroller.cpp
@@ -94,6 +94,9 @@ PositionScratchController::PositionScratchController(const QString& group)
           m_isScratching(false),
           m_inertiaEnabled(false),
           m_prevSamplePos(0),
+          // TODO we might as well use FramePos in order to use more convenient
+          // mixxx::audio::kInvalidFramePos, then convert to sample pos on the fly
+          m_seekSamplePos(std::numeric_limits<double>::quiet_NaN()),
           m_samplePosDeltaSum(0),
           m_scratchTargetDelta(0),
           m_scratchStartPos(0),
@@ -159,6 +162,15 @@ void PositionScratchController::process(double currentSamplePos,
         slotUpdateFilterParameters(m_pMainSampleRate->get());
     }
 
+    bool adoptSeekPos = false;
+    if (!util_isnan(m_seekSamplePos)) {
+        // If we were notified about a seek, adopt the new position immediately.
+        m_prevSamplePos = m_seekSamplePos;
+        m_seekSamplePos = std::numeric_limits<double>::quiet_NaN();
+
+        adoptSeekPos = true;
+    }
+
     double scratchPosition = 0;
     m_mouseSampleTime += m_dt;
     if (m_mouseSampleTime >= kDefaultSampleInterval || !m_isScratching) {
@@ -277,13 +289,26 @@ void PositionScratchController::process(double currentSamplePos,
                 }
 
                 if (calcRate) {
-                    double ctrlError = m_pRateIIFilter->filter(
-                            scratchTargetDelta - m_samplePosDeltaSum);
-                    m_rate = m_pVelocityController->observation(ctrlError);
-                    m_rate /= ceil(kDefaultSampleInterval / m_dt);
-                    if (fabs(m_rate) < MIN_SEEK_SPEED) {
-                        // we cannot get closer
+                    if (adoptSeekPos) {
+                        // If we just adopted the seek we need to avoid false high rate
+                        // and simply assume rate 0.
+                        // See comment in notifySeek().
                         m_rate = 0;
+                    } else {
+                        double ctrlError = m_pRateIIFilter->filter(
+                                scratchTargetDelta - m_samplePosDeltaSum);
+                        m_rate = m_pVelocityController->observation(ctrlError);
+                        m_rate /= ceil(kDefaultSampleInterval / m_dt);
+                        // ??
+                        // Note: The following SoundTouch changes the also rate by a ramp
+                        // This looks like average of the new and the old rate independent
+                        // from m_dt. Ramping is disabled when direction changes or rate = 0;
+                        // (determined experimentally)
+                        if (fabs(m_rate) < MIN_SEEK_SPEED) {
+                            // we cannot get closer
+                            // TODO ramp to new rate?
+                            m_rate = 0;
+                        }
                     }
                 }
 
@@ -321,7 +346,15 @@ void PositionScratchController::process(double currentSamplePos,
 
 void PositionScratchController::notifySeek(mixxx::audio::FramePos position) {
     DEBUG_ASSERT(position.isValid());
-    // scratching continues after seek due to calculating the relative distance traveled
-    // in m_samplePosDeltaSum
-    m_prevSamplePos = position.toEngineSamplePos();
+    const double newPos = position.toEngineSamplePos();
+    if (!isEnabled()) {
+        // not scratching, ignore";
+        return;
+    } else if (m_prevSamplePos == newPos) {
+        // no-op
+        return;
+    }
+    // Scratching continues after seek due to calculating the relative
+    // distance traveled in m_samplePosDeltaSum
+    m_seekSamplePos = newPos;
 }
diff --git a/src/engine/positionscratchcontroller.h b/src/engine/positionscratchcontroller.h
index 6970256a474..011f8cf78da 100644
--- a/src/engine/positionscratchcontroller.h
+++ b/src/engine/positionscratchcontroller.h
@@ -49,6 +49,7 @@ class PositionScratchController : public QObject {
     bool m_isScratching;
     bool m_inertiaEnabled;
     double m_prevSamplePos;
+    double m_seekSamplePos;
     double m_samplePosDeltaSum;
     double m_scratchTargetDelta;
     double m_scratchStartPos;