(fix) Waveform scratching: fix seeking to hotcue (reduced, alt. for #14059)

src/engine/positionscratchcontroller.cpp

@@ -69,13 +69,15 @@ namespace {
 
 constexpr double kDefaultSampleInterval = 0.016;
 // The max wait time when no new position has been set
+// TODO Make threshold configurable for controller use?
 constexpr double kMoveDelayMax = 0.04;
 // The rate threshold above which disabling position scratching will enable
 // an 'inertia' mode.
 constexpr double kThrowThreshold = 2.5;
 // Max velocity we would like to stop in a given time period.
 constexpr double kMaxVelocity = 100;
 // Seconds to stop a throw at the max velocity.
+// TODO make configurable, eg. to customize spinbacks with controllers
 constexpr double kTimeToStop = 1.0;
 
 } // anonymous namespace
@@ -93,15 +95,19 @@ 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),
           m_rate(0),
           m_moveDelay(0),
-          m_mouseSampleTime(0),
-          m_bufferSize(-1), // ?
+          m_scratchPosSampleTime(0),
+          m_bufferSize(-1),
           m_dt(1),
           m_callsPerDt(1),
+          m_callsToStop(1),
           m_p(1),
           m_d(1),
           m_f(0.4) {
@@ -123,6 +129,8 @@ void PositionScratchController::slotUpdateFilterParameters(double sampleRate) {
     // lowpass filter
     m_callsPerDt = static_cast<int>(ceil(kDefaultSampleInterval / m_dt));
 
+    m_callsToStop = m_dt / kTimeToStop;
+
     // Tweak PD controller for different latencies
     m_p = 0.3;
     m_d = m_p / -2;
@@ -142,26 +150,13 @@ void PositionScratchController::process(double currentSamplePos,
         int wrappedAround,
         mixxx::audio::FramePos trigger,
         mixxx::audio::FramePos target) {
-    bool scratchEnable = m_pScratchEnable->get() != 0;
-
-    if (!m_isScratching && !scratchEnable) {
-        // We were not previously in scratch mode are still not in scratch
-        // mode. Do nothing
-        return;
-    }
+    bool scratchEnable = m_pScratchEnable->toBool();
 
     if (bufferSize != m_bufferSize) {
         m_bufferSize = bufferSize;
         slotUpdateFilterParameters(m_pMainSampleRate->get());
     }
 
-    double scratchPosition = 0;
-    m_mouseSampleTime += m_dt;
-    if (m_mouseSampleTime >= kDefaultSampleInterval || !m_isScratching) {
-        scratchPosition = m_pScratchPos->get();
-        m_mouseSampleTime = 0;
-    }
-
     if (m_isScratching) {
         if (m_inertiaEnabled) {
             // If we got here then we're not scratching and we're in inertia
@@ -179,10 +174,10 @@ void PositionScratchController::process(double currentSamplePos,
             // constants. Roughly we backsolve what the decay should be if we want to
             // stop a throw of max velocity kMaxVelocity in kTimeToStop seconds. Here is
             // the derivation:
-            // kMaxVelocity * alpha ^ (# callbacks to stop in) = decayThreshold
+            // decayThreshold = kMaxVelocity * alpha ^ (# callbacks to stop in)
             // # callbacks = kTimeToStop / m_dt
             // alpha = (decayThreshold / kMaxVelocity) ^ (m_dt / kTimeToStop)
-            const double kExponentialDecay = pow(decayThreshold / kMaxVelocity, m_dt / kTimeToStop);
+            const double kExponentialDecay = pow(decayThreshold / kMaxVelocity, m_callsToStop);
 
             m_rate *= kExponentialDecay;
 
@@ -227,22 +222,24 @@ void PositionScratchController::process(double currentSamplePos,
             // This is required to scratch within loop boundaries.
             m_samplePosDeltaSum += (sampleDelta) / (bufferSize * baseSampleRate);
 
-            // If we may have a new position, calculate scratch parameters and
+            m_scratchPosSampleTime += m_dt;
+            // If the kDefaultSampleInterval has expired, calculate scratch parameters and
             // eventually the new rate.
             // Else, continue with the last rate.
-            if (m_mouseSampleTime == 0) {
+            if (m_scratchPosSampleTime >= kDefaultSampleInterval) {
+                m_scratchPosSampleTime = 0;
+
                 // Set the scratch target to the current set position
                 // and normalize to one buffer
-                double scratchTargetDelta = (scratchPosition - m_scratchStartPos) /
+                double scratchTargetDelta = (m_pScratchPos->get() - m_scratchStartPos) /
                         (bufferSize * baseSampleRate);
 
                 bool calcRate = true;
 
                 if (m_scratchTargetDelta == scratchTargetDelta) {
-                    // We get here if the next mouse position is delayed, the mouse
-                    // is stopped or moves very slowly. Since we don't know the case
-                    // we assume delayed mouse updates for 40 ms.
-                    // TODO Make threshold configurable for controller use?
+                    // We get here if the next mouse position is delayed, the
+                    // mouse is stopped or moves very slowly. Since we don't
+                    // know the case we assume delayed mouse updates for 40 ms.
                     m_moveDelay += m_dt * m_callsPerDt;
                     if (m_moveDelay < kMoveDelayMax) {
                         // Assume a missing Mouse Update and continue with the
@@ -265,15 +262,15 @@ void PositionScratchController::process(double currentSamplePos,
                     m_scratchTargetDelta = scratchTargetDelta;
                 }
 
+                // If we just adopted the seek position we need to avoid false
+                // high rate and simply report the previous rate.
+                // It'll adapt to the scratch speed in the next run.
+                // Setting rate to 0 has the same effect apparently.
                 if (calcRate) {
                     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)
+                    m_rate /= m_callsPerDt;
                     if (fabs(m_rate) < MIN_SEEK_SPEED) {
                         // we cannot get closer
                         m_rate = 0;
@@ -283,8 +280,8 @@ void PositionScratchController::process(double currentSamplePos,
                 // qDebug() << m_rate << scratchTargetDelta << m_samplePosDeltaSum << m_dt;
             }
         } else {
-            // We were previously in scratch mode and are no longer in scratch
-            // mode. Disable everything, or optionally enable inertia mode if
+            // We quit scratch mode.
+            // Disable everything, or optionally enable inertia mode if
             // the previous rate was high enough to count as a 'throw'
             if (fabs(m_rate) > kThrowThreshold) {
                 m_inertiaEnabled = true;
@@ -294,26 +291,43 @@ void PositionScratchController::process(double currentSamplePos,
             //qDebug() << "disable";
         }
     } else if (scratchEnable) {
-        // We were not previously in scratch mode but now are in scratch
-        // mode. Enable scratching.
+        // We were not previously in scratch mode but now we are.
+        // Enable scratching.
         m_isScratching = true;
         m_inertiaEnabled = false;
         m_moveDelay = 0;
         // Set up initial values, in a way that the system is settled
         m_rate = releaseRate;
-        m_samplePosDeltaSum = -(releaseRate / m_p) *
-                m_callsPerDt; // Set to the remaining error of a p controller
+        // Set to the remaining error of a p controller
+        m_samplePosDeltaSum = -(releaseRate / m_p) * m_callsPerDt;
         m_pVelocityController->reset(-m_samplePosDeltaSum);
         m_pRateIIFilter->reset(-m_samplePosDeltaSum);
-        m_scratchStartPos = scratchPosition;
+        // The "scratch_position" CO contains the distance traveled. We use this
+        // to calculate the traveled distance of the mouse compared to m_scratchStartPos.
+        // When it's set by WWaveformViewer::mousePressEvent or mouseMoveEvent,
+        // this is equal to traveled frames * 2 but usually unrelated to the
+        // the actual position in the track.
+        // Note that "scratch_position" can also be set by anyone, eg. by wheel
+        // functions of controller mappings. In such cases its value depends on
+        // the scaling of the original wheel position / wheel tick values and
+        // may be entirely unrelated to audio frames.
+        m_scratchStartPos = m_pScratchPos->get();
+        m_scratchPosSampleTime = 0;
         // qDebug() << "scratchEnable()" << currentSamplePos;
     }
+
+    if (!util_isnan(m_seekSamplePos)) {
+        // We need to transpose all buffers to compensate the seek
+        // in a way that the next process call does not even notice anything
+        currentSamplePos = m_seekSamplePos;
+        m_seekSamplePos = std::numeric_limits<double>::quiet_NaN();
+    }
     m_prevSamplePos = 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();
+    // Scratching continues after seek due to calculating the relative
+    // distance traveled in m_samplePosDeltaSum
+    m_seekSamplePos = newPos;
 }

src/engine/positionscratchcontroller.h

@@ -47,17 +47,19 @@ 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;
     double m_rate;
     double m_moveDelay;
-    double m_mouseSampleTime;
+    double m_scratchPosSampleTime;
 
     int m_bufferSize;
 
     double m_dt;
     double m_callsPerDt;
+    double m_callsToStop;
 
     double m_p;
     double m_d;

src/engine/readaheadmanager.cpp

@@ -283,22 +283,8 @@ double ReadAheadManager::getFilePlaypositionFromLog(
     }
 
     double filePlayposition = 0;
-    bool shouldNotifySeek = false;
     while (m_readAheadLog.size() > 0 && numConsumedSamples > 0) {
         ReadLogEntry& entry = m_readAheadLog.front();
-
-        // Notify EngineControls that we have taken a seek.
-        // Every new entry start with a seek
-        // (Not looping control)
-        if (shouldNotifySeek) {
-            if (m_pRateControl) {
-                const auto seekPosition =
-                        mixxx::audio::FramePos::fromEngineSamplePos(
-                                entry.virtualPlaypositionStart);
-                m_pRateControl->notifySeek(seekPosition);
-            }
-        }
-
         // Advance our idea of the current virtual playposition to this
         // ReadLogEntry's start position.
         filePlayposition = entry.advancePlayposition(&numConsumedSamples);
@@ -307,7 +293,6 @@ double ReadAheadManager::getFilePlaypositionFromLog(
             // This entry is empty now.
             m_readAheadLog.pop_front();
         }
-        shouldNotifySeek = true;
     }
 
     return filePlayposition;