LoopingControl: Seek to loop in setLoop() if playpos is behind loop end

mixxxdj · Nov 4, 2020 · e73e44a · e73e44a
1 parent ca24179
commit e73e44a
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 35 deletions.
diff --git a/src/engine/controls/loopingcontrol.cpp b/src/engine/controls/loopingcontrol.cpp
@@ -563,13 +563,20 @@ void LoopingControl::setLoop(double startPosition, double endPosition, bool enab
         m_loopSamples.setValue(loopSamples);
         m_pCOLoopStartPosition->set(loopSamples.start);
         m_pCOLoopEndPosition->set(loopSamples.end);
-        if (enabled) {
-            if (m_currentSample.getValue() > loopSamples.end) {
-                slotLoopInGoto(1);
-            }
-        }
     }
     setLoopingEnabled(enabled);
+
+    // Seek back to loop in position if we're already behind the loop end.
+    //
+    // TODO(Holzhaus): This needs to be reverted as soon as GUI controls for
+    // controlling saved loop behaviour are in place, because this change makes
+    // saved loops very risky to use and might potentially mess up your mix.
+    // See https://github.com/mixxxdj/mixxx/pull/2194#issuecomment-721847833
+    // for details.
+    if (enabled && m_currentSample.getValue() > loopSamples.end) {
+        slotLoopInGoto(1);
+    }
+
     m_pCOBeatLoopSize->setAndConfirm(findBeatloopSizeForLoop(startPosition, endPosition));
 }
 

diff --git a/src/test/hotcuecontrol_test.cpp b/src/test/hotcuecontrol_test.cpp
@@ -917,7 +917,6 @@ TEST_F(HotcueControlTest, SavedLoopToggleDoesNotSeek) {
 
     const double beforeLoopPositionSamples = 0;
     const double loopStartPositionSamples = 8 * beatLengthSamples;
-    const double afterLoopPositionSamples = 16 * beatLengthSamples;
 
     EXPECT_DOUBLE_EQ(static_cast<double>(HotcueControl::Status::Empty), m_pHotcue1Enabled->get());
     EXPECT_DOUBLE_EQ(Cue::kNoPosition, m_pHotcue1Position->get());
@@ -967,33 +966,6 @@ TEST_F(HotcueControlTest, SavedLoopToggleDoesNotSeek) {
     EXPECT_DOUBLE_EQ(static_cast<double>(HotcueControl::Status::Set), m_pHotcue1Enabled->get());
     EXPECT_DOUBLE_EQ(loopStartPositionSamples, m_pHotcue1Position->get());
     EXPECT_DOUBLE_EQ(loopStartPositionSamples + loopLengthSamples, m_pHotcue1EndPosition->get());
-
-    // Seek to position after saved loop
-    setCurrentSamplePosition(afterLoopPositionSamples);
-
-    // Check that the previous seek disabled the loop
-    EXPECT_DOUBLE_EQ(static_cast<double>(HotcueControl::Status::Set), m_pHotcue1Enabled->get());
-    EXPECT_DOUBLE_EQ(loopStartPositionSamples, m_pHotcue1Position->get());
-    EXPECT_DOUBLE_EQ(loopStartPositionSamples + loopLengthSamples, m_pHotcue1EndPosition->get());
-
-    // Re-Enable loop
-    m_pHotcue1Activate->slotSet(1);
-    m_pHotcue1Activate->slotSet(0);
-    ProcessBuffer();
-    EXPECT_DOUBLE_EQ(static_cast<double>(HotcueControl::Status::Active), m_pHotcue1Enabled->get());
-    EXPECT_DOUBLE_EQ(loopStartPositionSamples, m_pHotcue1Position->get());
-    EXPECT_DOUBLE_EQ(loopStartPositionSamples + loopLengthSamples, m_pHotcue1EndPosition->get());
-
-    // Check that re-enabling loop didn't seek
-    EXPECT_NEAR(afterLoopPositionSamples, currentSamplePosition(), 2048);
-
-    // Disable loop
-    m_pHotcue1Activate->slotSet(1);
-    m_pHotcue1Activate->slotSet(0);
-    ProcessBuffer();
-    EXPECT_DOUBLE_EQ(static_cast<double>(HotcueControl::Status::Set), m_pHotcue1Enabled->get());
-    EXPECT_DOUBLE_EQ(loopStartPositionSamples, m_pHotcue1Position->get());
-    EXPECT_DOUBLE_EQ(loopStartPositionSamples + loopLengthSamples, m_pHotcue1EndPosition->get());
 }
 
 TEST_F(HotcueControlTest, SavedLoopActivate) {
@@ -1059,14 +1031,15 @@ TEST_F(HotcueControlTest, SavedLoopActivate) {
 
     positionBeforeActivate = currentSamplePosition();
 
-    // Activate saved loop (does not imply seeking to loop start)
+    // Activate saved loop (usually doesn't imply seeking to loop start, but in this case it does
+    // because the play position is behind the loop end position)
     m_pHotcue1Activate->slotSet(1);
     m_pHotcue1Activate->slotSet(0);
     ProcessBuffer();
     EXPECT_DOUBLE_EQ(static_cast<double>(HotcueControl::Status::Active), m_pHotcue1Enabled->get());
     EXPECT_DOUBLE_EQ(loopStartPositionSamples, m_pHotcue1Position->get());
     EXPECT_DOUBLE_EQ(loopStartPositionSamples + loopLengthSamples, m_pHotcue1EndPosition->get());
-    EXPECT_NEAR(positionBeforeActivate, currentSamplePosition(), 2000);
+    EXPECT_NEAR(loopStartPositionSamples, currentSamplePosition(), 2000);
 }
 
 TEST_F(HotcueControlTest, SavedLoopActivateWhilePlayingTogglesLoop) {