Merge pull request #4071 from Holzhaus/sync-bpm-refactor

Sync: Use mixxx::Bpm class

src/engine/enginebuffer.cpp

@@ -1317,11 +1317,11 @@ void EngineBuffer::postProcess(const int iBufferSize) {
     const mixxx::Bpm localBpm = m_pBpmControl->updateLocalBpm();
     double beatDistance = m_pBpmControl->updateBeatDistance();
     // FIXME: Double check if calling setLocalBpm with an invalid value is correct and intended.
-    double localBpmValue = mixxx::Bpm::kValueUndefined;
+    mixxx::Bpm newLocalBpm;
     if (localBpm.isValid()) {
-        localBpmValue = localBpm.value();
+        newLocalBpm = localBpm;
     }
-    m_pSyncControl->setLocalBpm(localBpmValue);
+    m_pSyncControl->setLocalBpm(newLocalBpm);
     m_pSyncControl->updateAudible();
     SyncMode mode = m_pSyncControl->getSyncMode();
     if (isLeader(mode)) {

src/engine/sync/clock.h

@@ -1,4 +1,5 @@
 #pragma once
+#include "track/bpm.h"
 
 class Clock {
   public:
@@ -7,6 +8,6 @@ class Clock {
     virtual double getBeatDistance() const = 0;
     virtual void updateLeaderBeatDistance(double beatDistance) = 0;
 
-    virtual double getBpm() const = 0;
-    virtual void updateLeaderBpm(double bpm) = 0;
+    virtual mixxx::Bpm getBpm() const = 0;
+    virtual void updateLeaderBpm(mixxx::Bpm bpm) = 0;
 };

src/engine/sync/enginesync.cpp

@@ -12,19 +12,23 @@
 namespace {
 const mixxx::Logger kLogger("EngineSync");
 const QString kInternalClockGroup = QStringLiteral("[InternalClock]");
+constexpr mixxx::Bpm kDefaultBpm = mixxx::Bpm(124.0);
 } // anonymous namespace
 
 EngineSync::EngineSync(UserSettingsPointer pConfig)
         : m_pConfig(pConfig),
           m_pInternalClock(new InternalClock(kInternalClockGroup, this)),
           m_pLeaderSyncable(nullptr) {
     qRegisterMetaType<SyncMode>("SyncMode");
-    m_pInternalClock->updateLeaderBpm(124.0);
+    m_pInternalClock->updateLeaderBpm(kDefaultBpm);
 }
 
 EngineSync::~EngineSync() {
     // We use the slider value because that is never set to 0.0.
-    m_pConfig->set(ConfigKey(kInternalClockGroup, "bpm"), ConfigValue(m_pInternalClock->getBpm()));
+    const mixxx::Bpm bpm = m_pInternalClock->getBpm();
+    m_pConfig->set(ConfigKey(kInternalClockGroup, "bpm"),
+            ConfigValue(
+                    bpm.isValid() ? bpm.value() : mixxx::Bpm::kValueUndefined));
     delete m_pInternalClock;
 }
 
@@ -45,7 +49,7 @@ void EngineSync::requestSyncMode(Syncable* pSyncable, SyncMode mode) {
     switch (mode) {
     case SyncMode::LeaderExplicit:
     case SyncMode::LeaderSoft: {
-        if (pSyncable->getBaseBpm() > 0) {
+        if (pSyncable->getBaseBpm().isValid()) {
             activateLeader(pSyncable, mode);
         } else {
             // Because we don't have a valid bpm, we can't be the leader
@@ -206,7 +210,7 @@ Syncable* EngineSync::pickLeader(Syncable* enabling_syncable) {
     }
     if (m_pLeaderSyncable &&
             m_pLeaderSyncable->getSyncMode() == SyncMode::LeaderExplicit &&
-            m_pLeaderSyncable->getBaseBpm() != 0.0) {
+            m_pLeaderSyncable->getBaseBpm().isValid()) {
         return m_pLeaderSyncable;
     }
 
@@ -223,7 +227,7 @@ Syncable* EngineSync::pickLeader(Syncable* enabling_syncable) {
     int playing_deck_count = 0;
 
     for (const auto& pSyncable : qAsConst(m_syncables)) {
-        if (pSyncable->getBaseBpm() <= 0.0) {
+        if (!pSyncable->getBaseBpm().isValid()) {
             continue;
         }
 
@@ -309,7 +313,7 @@ Syncable* EngineSync::findBpmMatchTarget(Syncable* requester) {
         if (!pOtherSyncable->getChannel()->isPrimaryDeck()) {
             continue;
         }
-        if (pOtherSyncable->getBaseBpm() == 0.0) {
+        if (!pOtherSyncable->getBaseBpm().isValid()) {
             continue;
         }
 
@@ -382,7 +386,7 @@ void EngineSync::notifyScratching(Syncable* pSyncable, bool scratching) {
     Q_UNUSED(scratching);
 }
 
-void EngineSync::notifyBaseBpmChanged(Syncable* pSyncable, double bpm) {
+void EngineSync::notifyBaseBpmChanged(Syncable* pSyncable, mixxx::Bpm bpm) {
     if (kLogger.traceEnabled()) {
         kLogger.trace() << "EngineSync::notifyBaseBpmChanged" << pSyncable->getGroup() << bpm;
     }
@@ -392,40 +396,40 @@ void EngineSync::notifyBaseBpmChanged(Syncable* pSyncable, double bpm) {
     }
 }
 
-void EngineSync::notifyRateChanged(Syncable* pSyncable, double bpm) {
+void EngineSync::notifyRateChanged(Syncable* pSyncable, mixxx::Bpm bpm) {
     if (kLogger.traceEnabled()) {
         kLogger.trace() << "EngineSync::notifyRateChanged" << pSyncable->getGroup() << bpm;
     }
 
     updateLeaderBpm(pSyncable, bpm);
 }
 
-void EngineSync::requestBpmUpdate(Syncable* pSyncable, double bpm) {
+void EngineSync::requestBpmUpdate(Syncable* pSyncable, mixxx::Bpm bpm) {
     if (kLogger.traceEnabled()) {
         kLogger.trace() << "EngineSync::requestBpmUpdate" << pSyncable->getGroup() << bpm;
     }
 
-    double mbaseBpm = 0.0;
-    double mbpm = 0.0;
+    mixxx::Bpm leaderBaseBpm;
+    mixxx::Bpm leaderBpm;
     double beatDistance = 0.0;
     if (m_pLeaderSyncable) {
-        mbaseBpm = m_pLeaderSyncable->getBaseBpm();
-        mbpm = m_pLeaderSyncable->getBpm();
+        leaderBaseBpm = m_pLeaderSyncable->getBaseBpm();
+        leaderBpm = m_pLeaderSyncable->getBpm();
         beatDistance = m_pLeaderSyncable->getBeatDistance();
     }
 
-    if (mbaseBpm != 0.0) {
+    if (leaderBaseBpm.isValid()) {
         // update from current leader
         pSyncable->updateLeaderBeatDistance(beatDistance);
-        pSyncable->updateLeaderBpm(mbpm);
+        pSyncable->updateLeaderBpm(leaderBpm);
     } else {
         // There is no leader, adopt this bpm as leader value
         pSyncable->updateLeaderBeatDistance(0.0);
         pSyncable->updateLeaderBpm(bpm);
     }
 }
 
-void EngineSync::notifyInstantaneousBpmChanged(Syncable* pSyncable, double bpm) {
+void EngineSync::notifyInstantaneousBpmChanged(Syncable* pSyncable, mixxx::Bpm bpm) {
     if (kLogger.traceEnabled()) {
         kLogger.trace() << "EngineSync::notifyInstantaneousBpmChanged" << pSyncable->getGroup() << bpm;
     }
@@ -546,14 +550,14 @@ Syncable* EngineSync::getSyncableForGroup(const QString& group) {
 
 bool EngineSync::syncDeckExists() const {
     for (const auto& pSyncable : qAsConst(m_syncables)) {
-        if (pSyncable->isSynchronized() && pSyncable->getBaseBpm() > 0) {
+        if (pSyncable->isSynchronized() && pSyncable->getBaseBpm().isValid()) {
             return true;
         }
     }
     return false;
 }
 
-double EngineSync::leaderBpm() const {
+mixxx::Bpm EngineSync::leaderBpm() const {
     if (m_pLeaderSyncable) {
         return m_pLeaderSyncable->getBpm();
     }
@@ -567,14 +571,14 @@ double EngineSync::leaderBeatDistance() const {
     return m_pInternalClock->getBeatDistance();
 }
 
-double EngineSync::leaderBaseBpm() const {
+mixxx::Bpm EngineSync::leaderBaseBpm() const {
     if (m_pLeaderSyncable) {
         return m_pLeaderSyncable->getBaseBpm();
     }
     return m_pInternalClock->getBaseBpm();
 }
 
-void EngineSync::updateLeaderBpm(Syncable* pSource, double bpm) {
+void EngineSync::updateLeaderBpm(Syncable* pSource, mixxx::Bpm bpm) {
     //qDebug() << "EngineSync::updateLeaderBpm" << pSource << bpm;
     if (pSource != m_pInternalClock) {
         m_pInternalClock->updateLeaderBpm(bpm);
@@ -588,7 +592,7 @@ void EngineSync::updateLeaderBpm(Syncable* pSource, double bpm) {
     }
 }
 
-void EngineSync::updateLeaderInstantaneousBpm(Syncable* pSource, double bpm) {
+void EngineSync::updateLeaderInstantaneousBpm(Syncable* pSource, mixxx::Bpm bpm) {
     if (pSource != m_pInternalClock) {
         m_pInternalClock->updateInstantaneousBpm(bpm);
     }
@@ -648,9 +652,9 @@ void EngineSync::reinitLeaderParams(Syncable* pSource) {
             beatDistance = m_pInternalClock->getBeatDistance();
         }
     }
-    const double baseBpm = pSource->getBaseBpm();
-    double bpm = pSource->getBpm();
-    if (bpm <= 0) {
+    const mixxx::Bpm baseBpm = pSource->getBaseBpm();
+    mixxx::Bpm bpm = pSource->getBpm();
+    if (!bpm.isValid()) {
         bpm = baseBpm;
     }
     if (kLogger.traceEnabled()) {

src/engine/sync/enginesync.h

@@ -25,13 +25,13 @@ class EngineSync : public SyncableListener {
     /// Syncables notify EngineSync directly about various events. EngineSync
     /// does not have a say in whether these succeed or not, they are simply
     /// notifications.
-    void notifyBaseBpmChanged(Syncable* pSyncable, double bpm) override;
-    void notifyRateChanged(Syncable* pSyncable, double bpm) override;
-    void requestBpmUpdate(Syncable* pSyncable, double bpm) override;
+    void notifyBaseBpmChanged(Syncable* pSyncable, mixxx::Bpm bpm) override;
+    void notifyRateChanged(Syncable* pSyncable, mixxx::Bpm bpm) override;
+    void requestBpmUpdate(Syncable* pSyncable, mixxx::Bpm bpm) override;
 
     /// Instantaneous BPM refers to the actual, honest-to-god speed of playback
     /// at any moment, including any scratching that may be happening.
-    void notifyInstantaneousBpmChanged(Syncable* pSyncable, double bpm) override;
+    void notifyInstantaneousBpmChanged(Syncable* pSyncable, mixxx::Bpm bpm) override;
 
     /// the beat distance is updated on every callback.
     void notifyBeatDistanceChanged(Syncable* pSyncable, double beatDistance) override;
@@ -91,7 +91,7 @@ class EngineSync : public SyncableListener {
 
     /// Return the current BPM of the Leader Syncable. If no Leader syncable is
     /// set then returns the BPM of the internal clock.
-    double leaderBpm() const;
+    mixxx::Bpm leaderBpm() const;
 
     /// Returns the current beat distance of the Leader Syncable. If no Leader
     /// Syncable is set, then returns the beat distance of the internal clock.
@@ -100,14 +100,14 @@ class EngineSync : public SyncableListener {
     /// Returns the overall average BPM of the Leader Syncable if it were playing
     /// at 1.0 rate. This is used to calculate half/double multipliers and whether
     /// the Leader has a bpm at all.
-    double leaderBaseBpm() const;
+    mixxx::Bpm leaderBaseBpm() const;
 
     /// Set the BPM on every sync-enabled Syncable except pSource.
-    void updateLeaderBpm(Syncable* pSource, double bpm);
+    void updateLeaderBpm(Syncable* pSource, mixxx::Bpm bpm);
 
     /// Set the Leader instantaneous BPM on every sync-enabled Syncable except
     /// pSource.
-    void updateLeaderInstantaneousBpm(Syncable* pSource, double bpm);
+    void updateLeaderInstantaneousBpm(Syncable* pSource, mixxx::Bpm bpm);
 
     /// Set the Leader beat distance on every sync-enabled Syncable except
     /// pSource.

src/engine/sync/internalclock.cpp

@@ -12,16 +12,17 @@
 
 namespace {
 const mixxx::Logger kLogger("InternalClock");
+constexpr mixxx::Bpm kDefaultBpm(124.0);
 } // namespace
 
 InternalClock::InternalClock(const QString& group, SyncableListener* pEngineSync)
         : m_group(group),
           m_pEngineSync(pEngineSync),
           m_mode(SyncMode::None),
           m_iOldSampleRate(44100),
-          m_dOldBpm(124.0),
-          m_dBaseBpm(124.0),
-          m_dBeatLength(m_iOldSampleRate * 60.0 / m_dOldBpm),
+          m_oldBpm(kDefaultBpm),
+          m_baseBpm(kDefaultBpm),
+          m_dBeatLength(m_iOldSampleRate * 60.0 / m_oldBpm.value()),
           m_dClockPosition(0) {
     // Pick a wide range (1 to 200) and allow out of bounds sets. This lets you
     // map a soft-takeover MIDI knob to the leader BPM. This also creates bpm_up
@@ -84,7 +85,7 @@ void InternalClock::slotSyncLeaderEnabledChangeRequest(double state) {
             return;
         }
         if (mode == SyncMode::None) {
-            m_dBaseBpm = m_dOldBpm;
+            m_baseBpm = m_oldBpm;
         }
         m_pEngineSync->requestSyncMode(this, SyncMode::LeaderExplicit);
     } else {
@@ -115,26 +116,26 @@ void InternalClock::updateLeaderBeatDistance(double beatDistance) {
     m_pEngineSync->notifyBeatDistanceChanged(this, beatDistance);
 }
 
-double InternalClock::getBaseBpm() const {
-    return m_dBaseBpm;
+mixxx::Bpm InternalClock::getBaseBpm() const {
+    return m_baseBpm;
 }
 
-double InternalClock::getBpm() const {
-    return m_pClockBpm->get();
+mixxx::Bpm InternalClock::getBpm() const {
+    return mixxx::Bpm(m_pClockBpm->get());
 }
 
-void InternalClock::updateLeaderBpm(double bpm) {
+void InternalClock::updateLeaderBpm(mixxx::Bpm bpm) {
     if (kLogger.traceEnabled()) {
         kLogger.trace() << "InternalClock::setBpm" << bpm;
     }
-    if (bpm == 0) {
+    if (!bpm.isValid()) {
         return;
     }
-    m_pClockBpm->set(bpm);
+    m_pClockBpm->set(bpm.value());
     updateBeatLength(m_iOldSampleRate, bpm);
 }
 
-void InternalClock::updateInstantaneousBpm(double bpm) {
+void InternalClock::updateInstantaneousBpm(mixxx::Bpm bpm) {
     if (kLogger.traceEnabled()) {
         kLogger.trace() << "InternalClock::setInstantaneousBpm" << bpm;
     }
@@ -145,28 +146,28 @@ void InternalClock::updateInstantaneousBpm(double bpm) {
 void InternalClock::notifyLeaderParamSource() {
 }
 
-void InternalClock::reinitLeaderParams(double beatDistance, double baseBpm, double bpm) {
+void InternalClock::reinitLeaderParams(double beatDistance, mixxx::Bpm baseBpm, mixxx::Bpm bpm) {
     if (kLogger.traceEnabled()) {
         kLogger.trace() << "InternalClock::reinitLeaderParams" << beatDistance << baseBpm << bpm;
     }
-    if (bpm <= 0.0 || baseBpm <= 0.0) {
+    if (!bpm.isValid() || !baseBpm.isValid()) {
         return;
     }
-    m_dBaseBpm = baseBpm;
+    m_baseBpm = baseBpm;
     updateLeaderBpm(bpm);
     updateLeaderBeatDistance(beatDistance);
 }
 
 void InternalClock::slotBpmChanged(double bpm) {
-    m_dBaseBpm = bpm;
-    updateBeatLength(m_iOldSampleRate, m_dBaseBpm);
+    m_baseBpm = mixxx::Bpm(bpm);
+    updateBeatLength(m_iOldSampleRate, m_baseBpm);
     if (!isSynchronized()) {
         return;
     }
     // The internal clock doesn't have a rate slider, so treat
     // "base" bpm changes as rate changes -- this means the change will be
     // reflected in all synced decks.
-    m_pEngineSync->notifyRateChanged(this, m_dBaseBpm);
+    m_pEngineSync->notifyRateChanged(this, m_baseBpm);
 }
 
 void InternalClock::slotBeatDistanceChanged(double beatDistance) {
@@ -176,8 +177,8 @@ void InternalClock::slotBeatDistanceChanged(double beatDistance) {
     updateLeaderBeatDistance(beatDistance);
 }
 
-void InternalClock::updateBeatLength(int sampleRate, double bpm) {
-    if (m_iOldSampleRate == sampleRate && bpm == m_dOldBpm) {
+void InternalClock::updateBeatLength(int sampleRate, mixxx::Bpm bpm) {
+    if (m_iOldSampleRate == sampleRate && bpm == m_oldBpm) {
         return;
     }
 
@@ -193,13 +194,13 @@ void InternalClock::updateBeatLength(int sampleRate, double bpm) {
 
     // that last term is 1 over bpm.
 
-    if (qFuzzyCompare(bpm, 0)) {
+    if (!bpm.isValid()) {
         qDebug() << "WARNING: Leader bpm reported to be zero, internal clock guessing 124bpm";
         m_dBeatLength = (sampleRate * 60.0) / 124.0;
-        m_dOldBpm = 124.0;
+        m_oldBpm = kDefaultBpm;
     } else {
-        m_dOldBpm = bpm;
-        m_dBeatLength = (sampleRate * 60.0) / bpm;
+        m_oldBpm = bpm;
+        m_dBeatLength = (sampleRate * 60.0) / bpm.value();
         if (m_dBeatLength <= 0) {
             qDebug() << "WARNING: Tried to set samples per beat <=0";
             m_dBeatLength = sampleRate;
@@ -219,7 +220,7 @@ void InternalClock::onCallbackStart(int sampleRate, int bufferSize) {
 }
 
 void InternalClock::onCallbackEnd(int sampleRate, int bufferSize) {
-    updateBeatLength(sampleRate, m_pClockBpm->get());
+    updateBeatLength(sampleRate, getBpm());
 
     // stereo samples, so divide by 2
     m_dClockPosition += bufferSize / 2;