Adjust determineBpmMultiplier() that it returns the same decission independent from which deck it is called

2 files changed, 64 insertions, 17 deletions

diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp
index dbbf5c6e7c..0921421653 100644
--- a/src/engine/sync/synccontrol.cpp
+++ b/src/engine/sync/synccontrol.cpp
@@ -242,6 +242,8 @@ void SyncControl::setMasterBpm(double bpm) {
 
 void SyncControl::setMasterParams(
         double beatDistance, double baseBpm, double bpm) {
+    // Calculate the factor for the file bpm. That gives the best
+    // result at any rate slider position.
     double masterBpmAdjustFactor = determineBpmMultiplier(fileBpm(), baseBpm);
     if (isMaster(getSyncMode())) {
         // In Master mode we adjust the incoming Bpm for the initial sync.
@@ -256,24 +258,19 @@ void SyncControl::setMasterParams(
 }
 
 double SyncControl::determineBpmMultiplier(double myBpm, double targetBpm) const {
-    double multiplier = kBpmUnity;
     if (myBpm == 0.0 || targetBpm == 0.0) {
-        return multiplier;
-    }
-    double best_margin = fabs((targetBpm / myBpm) - 1.0);
-
-    double try_margin = fabs((targetBpm * kBpmHalve / myBpm) - 1.0);
-    // We really want to prefer unity, so use a float compare with high tolerance.
-    if (best_margin - try_margin > .0001) {
-        multiplier = kBpmHalve;
-        best_margin = try_margin;
-    }
-
-    try_margin = fabs((targetBpm * kBpmDouble / myBpm) - 1.0);
-    if (best_margin - try_margin > .0001) {
-        multiplier = kBpmDouble;
-    }
-    return multiplier;
+        return kBpmUnity;
+    }
+    double unityRatio = myBpm / targetBpm;
+    // the square root of 2 (1.414) is the
+    // rate threshold that works vice versa for this and the target.
+    double unityRatioSquare = unityRatio * unityRatio;
+    if (unityRatioSquare > kBpmDouble) {
+        return kBpmDouble;
+    } else if (unityRatioSquare < kBpmHalve) {
+        return kBpmHalve;
+    }
+    return kBpmUnity;
 }
 
 void SyncControl::updateTargetBeatDistance() {
diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp
index b34cafc1b2..78615e418d 100644
--- a/src/test/enginesynctest.cpp
+++ b/src/test/enginesynctest.cpp
@@ -1821,6 +1821,56 @@ TEST_F(EngineSyncTest, HalfDoubleConsistency) {
             ControlObject::getControl(ConfigKey(m_sGroup2, "bpm"))->get());
 }
 
+TEST_F(EngineSyncTest, HalfDoubleEachOther) {
+    // Confirm that repeated sync with both decks leads to the same
+    // Half/Double decision.
+    // This test demonstrates https://bugs.launchpad.net/mixxx/+bug/1921962
+    mixxx::BeatsPointer pBeats1 = BeatFactory::makeBeatGrid(m_pTrack1->getSampleRate(), 144, 0.0);
+    m_pTrack1->trySetBeats(pBeats1);
+    mixxx::BeatsPointer pBeats2 = BeatFactory::makeBeatGrid(m_pTrack2->getSampleRate(), 105, 0.0);
+    m_pTrack2->trySetBeats(pBeats2);
+
+    // Threshold 1.414 sqrt(2);
+    // 144 / 105 = 1.37
+    // 105 / 72 = 1.46
+    // expect 144 BPM
+
+    ControlObject::getControl(ConfigKey(m_sGroup2, "sync_enabled"))->set(1.0);
+    ControlObject::getControl(ConfigKey(m_sGroup2, "sync_enabled"))->set(0.0);
+
+    EXPECT_DOUBLE_EQ(144.0,
+            ControlObject::getControl(ConfigKey(m_sGroup2, "bpm"))->get());
+
+    ControlObject::getControl(ConfigKey(m_sGroup1, "sync_enabled"))->set(1.0);
+    ControlObject::getControl(ConfigKey(m_sGroup1, "sync_enabled"))->set(0.0);
+
+    EXPECT_DOUBLE_EQ(144.0,
+            ControlObject::getControl(ConfigKey(m_sGroup1, "bpm"))->get());
+
+    // Threshold 1.414 sqrt(2);
+    // 150 / 105 = 1.43
+    // 105 / 75 = 1.40
+    // expect 75 BPM
+
+    mixxx::BeatsPointer pBeats1b = BeatFactory::makeBeatGrid(m_pTrack1->getSampleRate(), 150, 0.0);
+    m_pTrack1->trySetBeats(pBeats1b);
+
+    EXPECT_DOUBLE_EQ(150.0,
+            ControlObject::getControl(ConfigKey(m_sGroup1, "bpm"))->get());
+
+    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,
+            ControlObject::getControl(ConfigKey(m_sGroup2, "bpm"))->get());
+
+    ControlObject::getControl(ConfigKey(m_sGroup1, "sync_enabled"))->set(1.0);
+    ControlObject::getControl(ConfigKey(m_sGroup1, "sync_enabled"))->set(0.0);
+
+    EXPECT_DOUBLE_EQ(150.0,
+            ControlObject::getControl(ConfigKey(m_sGroup1, "bpm"))->get());
+}
+
 TEST_F(EngineSyncTest, SetFileBpmUpdatesLocalBpm) {
     ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))->set(0.2);
     mixxx::BeatsPointer pBeats1 = BeatFactory::makeBeatGrid(m_pTrack1->getSampleRate(), 130, 0.0);