LoopingControl: Use mixxx::audio::FramePos in nextTrigger()

src/engine/controls/loopingcontrol.cpp

@@ -364,12 +364,10 @@ void LoopingControl::process(const double dRate,
     }
 }
 
-double LoopingControl::nextTrigger(bool reverse,
-        const double currentSample,
-        double *pTarget) {
-    *pTarget = kNoTrigger;
-
-    const auto currentPosition = mixxx::audio::FramePos::fromEngineSamplePos(currentSample);
+mixxx::audio::FramePos LoopingControl::nextTrigger(bool reverse,
+        mixxx::audio::FramePos currentPosition,
+        mixxx::audio::FramePos* pTargetPosition) {
+    *pTargetPosition = mixxx::audio::kInvalidFramePos;
 
     LoopInfo loopInfo = m_loopInfo.getValue();
 
@@ -382,8 +380,8 @@ double LoopingControl::nextTrigger(bool reverse,
         // Jumping to the end is then handled when the loop's start is reached later in this function.
         if (reverse && !m_bAdjustingLoopIn && !m_pQuantizeEnabled->toBool()) {
             m_oldLoopInfo = loopInfo;
-            *pTarget = loopInfo.endPosition.toEngineSamplePosMaybeInvalid();
-            return currentSample;
+            *pTargetPosition = loopInfo.endPosition;
+            return currentPosition;
         }
     }
 
@@ -396,8 +394,8 @@ double LoopingControl::nextTrigger(bool reverse,
         // Jumping to the start is then handled when the loop's end is reached later in this function.
         if (!reverse && !m_bAdjustingLoopOut && !m_pQuantizeEnabled->toBool()) {
             m_oldLoopInfo = loopInfo;
-            *pTarget = loopInfo.startPosition.toEngineSamplePosMaybeInvalid();
-            return currentSample;
+            *pTargetPosition = loopInfo.startPosition;
+            return currentPosition;
         }
     }
 
@@ -412,11 +410,10 @@ double LoopingControl::nextTrigger(bool reverse,
             case LoopSeekMode::Changed:
                 // here the loop has changed and the play position
                 // should be moved with it
-                *pTarget = seekInsideAdjustedLoop(currentPosition,
+                *pTargetPosition = seekInsideAdjustedLoop(currentPosition,
                         m_oldLoopInfo.startPosition,
                         loopInfo.startPosition,
-                        loopInfo.endPosition)
-                                   .toEngineSamplePosMaybeInvalid();
+                        loopInfo.endPosition);
                 break;
             case LoopSeekMode::MovedOut: {
                 bool movedOut = false;
@@ -431,11 +428,10 @@ double LoopingControl::nextTrigger(bool reverse,
                     }
                 }
                 if (movedOut) {
-                    *pTarget = seekInsideAdjustedLoop(currentPosition,
+                    *pTargetPosition = seekInsideAdjustedLoop(currentPosition,
                             loopInfo.startPosition,
                             loopInfo.startPosition,
-                            loopInfo.endPosition)
-                                       .toEngineSamplePosMaybeInvalid();
+                            loopInfo.endPosition);
                 }
                 break;
             }
@@ -446,21 +442,21 @@ double LoopingControl::nextTrigger(bool reverse,
                 break;
             }
             m_oldLoopInfo = loopInfo;
-            if (*pTarget != kNoTrigger) {
+            if (pTargetPosition->isValid()) {
                 // jump immediately
-                return currentSample;
+                return currentPosition;
             }
         }
 
         if (reverse) {
-            *pTarget = loopInfo.endPosition.toEngineSamplePosMaybeInvalid();
-            return loopInfo.startPosition.toEngineSamplePosMaybeInvalid();
+            *pTargetPosition = loopInfo.endPosition;
+            return loopInfo.startPosition;
         } else {
-            *pTarget = loopInfo.startPosition.toEngineSamplePosMaybeInvalid();
-            return loopInfo.endPosition.toEngineSamplePosMaybeInvalid();
+            *pTargetPosition = loopInfo.startPosition;
+            return loopInfo.endPosition;
         }
     }
-    return kNoTrigger;
+    return mixxx::audio::kInvalidFramePos;
 }
 
 void LoopingControl::hintReader(HintVector* pHintList) {

src/engine/controls/loopingcontrol.h

@@ -35,9 +35,9 @@ class LoopingControl : public EngineControl {
 
     // nextTrigger returns the sample at which the engine will be triggered to
     // take a loop, given the value of currentSample and dRate.
-    virtual double nextTrigger(bool reverse,
-                       const double currentSample,
-                       double *pTarget);
+    virtual mixxx::audio::FramePos nextTrigger(bool reverse,
+            mixxx::audio::FramePos currentSample,
+            mixxx::audio::FramePos* pTargetPosition);
 
     // hintReader will add to hintList hints both the loop in and loop out
     // sample, if set.

src/engine/readaheadmanager.cpp

@@ -46,10 +46,15 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput,
 
     //qDebug() << "start" << start_sample << requested_samples;
 
-    double target;
+    mixxx::audio::FramePos targetPosition;
     // A loop will only limit the amount we can read in one shot.
-    const double loop_trigger = m_pLoopingControl->nextTrigger(
-            in_reverse, m_currentPosition, &target);
+    const mixxx::audio::FramePos loopTriggerPosition =
+            m_pLoopingControl->nextTrigger(in_reverse,
+                    mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
+                            m_currentPosition),
+                    &targetPosition);
+    const double loop_trigger = loopTriggerPosition.toEngineSamplePosMaybeInvalid();
+    const double target = targetPosition.toEngineSamplePosMaybeInvalid();
 
     SINT preloop_samples = 0;
     double samplesToLoopTrigger = 0.0;

src/test/looping_control_test.cpp

@@ -410,11 +410,12 @@ TEST_F(LoopingControlTest, LoopScale_HalvesLoop) {
     EXPECT_EQ(500, m_pLoopEndPoint->get());
     // Since the current sample was out of range of the new loop,
     // the current sample should reseek based on the new loop size.
-    double target;
-    double trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger(
-            false, 1800, &target);
-    EXPECT_EQ(300, target);
-    EXPECT_EQ(1800, trigger);
+    mixxx::audio::FramePos targetPosition;
+    const mixxx::audio::FramePos triggerPosition =
+            m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger(
+                    false, mixxx::audio::FramePos(900), &targetPosition);
+    EXPECT_EQ(mixxx::audio::FramePos(150), targetPosition);
+    EXPECT_EQ(mixxx::audio::FramePos(900), triggerPosition);
 }
 
 TEST_F(LoopingControlTest, LoopDoubleButton_IgnoresPastTrackEnd) {

src/test/readaheadmanager_test.cpp

@@ -38,28 +38,30 @@ class StubLoopControl : public LoopingControl {
     }
 
     void pushTriggerReturnValue(double value) {
-        m_triggerReturnValues.push_back(value);
+        m_triggerReturnValues.push_back(
+                mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(value));
     }
 
     void pushTargetReturnValue(double value) {
-        m_targetReturnValues.push_back(value);
+        m_targetReturnValues.push_back(
+                mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(value));
     }
 
-    double nextTrigger(bool reverse,
-                       const double currentSample,
-                       double* pTarget) override {
+    mixxx::audio::FramePos nextTrigger(bool reverse,
+            mixxx::audio::FramePos currentPosition,
+            mixxx::audio::FramePos* pTargetPosition) override {
         Q_UNUSED(reverse);
-        Q_UNUSED(currentSample);
-        Q_UNUSED(pTarget);
+        Q_UNUSED(currentPosition);
+        Q_UNUSED(pTargetPosition);
         RELEASE_ASSERT(!m_targetReturnValues.isEmpty());
-        *pTarget = m_targetReturnValues.takeFirst();
+        *pTargetPosition = m_targetReturnValues.takeFirst();
         RELEASE_ASSERT(!m_triggerReturnValues.isEmpty());
         return m_triggerReturnValues.takeFirst();
     }
 
   protected:
-    QList<double> m_triggerReturnValues;
-    QList<double> m_targetReturnValues;
+    QList<mixxx::audio::FramePos> m_triggerReturnValues;
+    QList<mixxx::audio::FramePos> m_targetReturnValues;
 };
 
 class ReadAheadManagerTest : public MixxxTest {