Merge pull request #13671 from acolombier/fix/use-active-bpm

fix: sync rate using the current BPM instead of the file one
mixxxdj · Sep 23, 2024 · 60b6378 · 60b6378
2 parents 64a4248 + 4b86618
commit 60b6378
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 7 deletions.
diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp
@@ -278,7 +278,7 @@ void SyncControl::reinitLeaderParams(
         kLogger.trace() << "SyncControl::reinitLeaderParams" << getGroup()
                         << beatDistance << baseBpm << bpm;
     }
-    m_leaderBpmAdjustFactor = determineBpmMultiplier(fileBpm(), baseBpm);
+    m_leaderBpmAdjustFactor = determineBpmMultiplier(mixxx::Bpm(m_pBpm->get()), bpm);
     updateLeaderBpm(bpm);
     updateLeaderBeatDistance(beatDistance);
 }

diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp
@@ -671,10 +671,10 @@ TEST_F(EngineSyncTest, RateChangeTest) {
     EXPECT_DOUBLE_EQ(
             120.0, ControlObject::get(ConfigKey(m_sGroup2, "file_bpm")));
 
-    // rate slider for channel 2 should now be 1.6 = 160 * 1.2 / 120.
-    EXPECT_DOUBLE_EQ(getRateSliderValue(1.6),
+    EXPECT_DOUBLE_EQ(getRateSliderValue(0.8),
             ControlObject::get(ConfigKey(m_sGroup2, "rate")));
-    EXPECT_DOUBLE_EQ(192.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")));
+    // Leader is currently 192, BPM before sync is 120, so the closer sync should be 96
+    EXPECT_DOUBLE_EQ(96.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")));
 }
 
 TEST_F(EngineSyncTest, RateChangeTestWeirdOrder) {
@@ -2105,8 +2105,8 @@ TEST_F(EngineSyncTest, HalfDoubleEachOther) {
             ControlObject::getControl(ConfigKey(m_sGroup1, "bpm"))->get());
 
     // Threshold 1.414 sqrt(2);
-    // 150 / 105 = 1.43
-    // 105 / 75 = 1.40
+    // 150 / 144 = 1.04
+    // 144 / 75 = 1.92
     // expect 75 BPM
 
     mixxx::BeatsPointer pBeats1b = mixxx::Beats::fromConstTempo(
@@ -2120,7 +2120,7 @@ TEST_F(EngineSyncTest, HalfDoubleEachOther) {
     ControlObject::getControl(ConfigKey(m_sGroup2, "sync_enabled"))->set(1.0);
     ControlObject::getControl(ConfigKey(m_sGroup2, "sync_enabled"))->set(0.0);
 
-    EXPECT_DOUBLE_EQ(75.0,
+    EXPECT_DOUBLE_EQ(150,
             ControlObject::getControl(ConfigKey(m_sGroup2, "bpm"))->get());
 
     ControlObject::getControl(ConfigKey(m_sGroup1, "sync_enabled"))->set(1.0);
@@ -3114,3 +3114,68 @@ TEST_F(EngineSyncTest, BeatContextRounding) {
             ControlObject::get(ConfigKey(m_sGroup1, "playposition")),
             kMaxFloatingPointErrorHighPrecision);
 }
+
+TEST_F(EngineSyncTest, KeepCorrectFactorUponResync) {
+    /* Usecase
+        - load track @ 174bpm in deck 1
+        - load track @ 87 bpm in deck 2
+        - Sync deck 2 to 1, keep 87 BPM on deck 2
+        - adjust rate slider on deck 1 to 184bpm
+        - Sync deck 2 to 1, get 92 BPM on deck 2
+    */
+    m_pMixerDeck1->loadFakeTrack(false, 174.0);
+    m_pMixerDeck2->loadFakeTrack(false, 87.0);
+    ProcessBuffer();
+
+    EXPECT_DOUBLE_EQ(174.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm")));
+    EXPECT_DOUBLE_EQ(87.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")));
+
+    ControlObject::set(ConfigKey(m_sGroup2, "sync_enabled"), 1.0);
+    ControlObject::set(ConfigKey(m_sGroup1, "sync_leader"), 1.0);
+    ProcessBuffer();
+
+    EXPECT_DOUBLE_EQ(174.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm")));
+    EXPECT_DOUBLE_EQ(87.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")));
+
+    ControlObject::set(ConfigKey(m_sGroup1, "rate"), getRateSliderValue(184.0 / 174));
+    ProcessBuffer();
+    EXPECT_NEAR(184.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm")), 0.001);
+    EXPECT_NEAR(92.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")), 0.001);
+    EXPECT_NEAR(getRateSliderValue(1.0574),
+            ControlObject::get(ConfigKey(m_sGroup2, "rate")),
+            0.005);
+}
+
+// There is a race condition preventing this usecase to work.
+// https://github.com/mixxxdj/mixxx/issues/13689
+TEST_F(EngineSyncTest, DISABLED_KeepCorrectFactorOnLoad) {
+    /* Usecase
+        - load track @ 174bpm in deck 1 and enable sync leader
+        - enable sync follower on the empty second track
+        - load track @ 87 bpm in deck 2
+        - deck 2 should remain at 87 BPM and rate of 1.0
+    */
+    m_pMixerDeck1->loadFakeTrack(false, 174.0);
+    ProcessBuffer();
+
+    EXPECT_DOUBLE_EQ(174.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm")));
+    EXPECT_DOUBLE_EQ(0.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")));
+
+    ControlObject::set(ConfigKey(m_sGroup2, "sync_enabled"), 1.0);
+    ControlObject::set(ConfigKey(m_sGroup1, "sync_leader"), 1.0);
+    ProcessBuffer();
+
+    m_pMixerDeck2->loadFakeTrack(false, 87.0);
+    ProcessBuffer();
+
+    EXPECT_DOUBLE_EQ(174.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm")));
+    EXPECT_DOUBLE_EQ(87.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")));
+
+    ControlObject::set(ConfigKey(m_sGroup1, "rate"), getRateSliderValue(184.0 / 174));
+    ProcessBuffer();
+    EXPECT_NEAR(184.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm")), 0.001);
+    EXPECT_NEAR(92.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")), 0.001);
+    EXPECT_NEAR(getRateSliderValue(1.0574),
+            ControlObject::get(ConfigKey(m_sGroup2, "rate")),
+            0.005);
+}