Sync Lock: minor refactoring.

src/engine/controls/bpmcontrol.cpp

@@ -663,7 +663,7 @@ double BpmControl::getNearestPositionInPhase(
     }
 
     double dOtherBeatFraction;
-    if (syncMode == SYNC_FOLLOWER) {
+    if (isFollower(syncMode)) {
         // If we're a follower, it's easy to get the other beat fraction
         dOtherBeatFraction = m_dSyncTargetBeatDistance.getValue();
     } else {
@@ -797,6 +797,14 @@ double BpmControl::getBeatMatchPosition(
     // explicit master always syncs to itself, so keep it null
     if (getSyncMode() != SYNC_MASTER_EXPLICIT) {
         pOtherEngineBuffer = pickSyncTarget();
+        if (kLogger.traceEnabled()) {
+            if (pOtherEngineBuffer) {
+                kLogger.trace() << "BpmControl::getBeatMatchPosition sync target"
+                                << pOtherEngineBuffer->getGroup();
+            } else {
+                kLogger.trace() << "BpmControl::getBeatMatchPosition no sync target found";
+            }
+        }
     }
     if (playing) {
         if (!pOtherEngineBuffer || pOtherEngineBuffer->getSpeed() == 0.0) {
@@ -1068,15 +1076,15 @@ double BpmControl::updateLocalBpm() {
 }
 
 double BpmControl::updateBeatDistance() {
-    double beat_distance = getBeatDistance(getSampleOfTrack().current);
-    m_pThisBeatDistance->set(beat_distance);
+    double beatDistance = getBeatDistance(getSampleOfTrack().current);
+    m_pThisBeatDistance->set(beatDistance);
     if (!isSynchronized()) {
         m_dUserOffset.setValue(0.0);
     }
     if (kLogger.traceEnabled()) {
-        kLogger.trace() << getGroup() << "BpmControl::updateBeatDistance" << beat_distance;
+        kLogger.trace() << getGroup() << "BpmControl::updateBeatDistance" << beatDistance;
     }
-    return beat_distance;
+    return beatDistance;
 }
 
 void BpmControl::setTargetBeatDistance(double beatDistance) {

src/engine/controls/ratecontrol.cpp

@@ -463,7 +463,7 @@ double RateControl::calculateSpeed(double baserate, double speed, bool paused,
             *pReportScratching = true;
         } else {
             // If master sync is on, respond to it -- but vinyl and scratch mode always override.
-            if (getSyncMode() == SYNC_FOLLOWER && !paused &&
+            if (isFollower(getSyncMode()) && !paused &&
                     !bVinylControlEnabled && !useScratch2Value) {
                 if (m_pBpmControl == nullptr) {
                     qDebug() << "ERROR: calculateRate m_pBpmControl is null during master sync";

src/engine/enginebuffer.cpp

@@ -929,8 +929,8 @@ void EngineBuffer::processTrackLocked(
     // we need to sync phase or we'll be totally out of whack and the sync
     // adjuster will kick in and push the track back in to sync with the
     // master.
-    if (m_scratching_old && !is_scratching && m_pQuantize->toBool()
-            && m_pSyncControl->getSyncMode() == SYNC_FOLLOWER && !paused) {
+    if (m_scratching_old && !is_scratching && m_pQuantize->toBool() &&
+            isFollower(m_pSyncControl->getSyncMode()) && !paused) {
         // TODO() The resulting seek is processed in the following callback
         // That is to late
         requestSyncPhase();
@@ -1269,8 +1269,8 @@ void EngineBuffer::processSeek(bool paused) {
         position = m_pLoopingControl->getSyncPositionInsideLoop(requestedPosition, syncPosition);
         if (kLogger.traceEnabled()) {
             kLogger.trace()
-                    << "EngineBuffer::processSeek seek info: " << m_filepos_play
-                    << " -> " << position;
+                    << "EngineBuffer::processSeek" << getGroup() << "seek info:" << m_filepos_play
+                    << "->" << position;
         }
     }
     if (position != m_filepos_play) {
@@ -1289,13 +1289,13 @@ void EngineBuffer::postProcess(const int iBufferSize) {
     if (kLogger.traceEnabled()) {
         kLogger.trace() << getGroup() << "EngineBuffer::postProcess";
     }
-    double local_bpm = m_pBpmControl->updateLocalBpm();
-    double beat_distance = m_pBpmControl->updateBeatDistance();
-    m_pSyncControl->setLocalBpm(local_bpm);
+    double localBpm = m_pBpmControl->updateLocalBpm();
+    double beatDistance = m_pBpmControl->updateBeatDistance();
+    m_pSyncControl->setLocalBpm(localBpm);
     SyncMode mode = m_pSyncControl->getSyncMode();
     if (isMaster(mode)) {
-        m_pEngineSync->notifyBeatDistanceChanged(m_pSyncControl, beat_distance);
-    } else if (mode == SYNC_FOLLOWER) {
+        m_pEngineSync->notifyBeatDistanceChanged(m_pSyncControl, beatDistance);
+    } else if (isFollower(mode)) {
         // Report our speed to SyncControl.  If we are master, we already did this.
         m_pSyncControl->reportPlayerSpeed(m_speed_old, m_scratching_old);
         m_pSyncControl->updateTargetBeatDistance();

src/engine/sync/basesyncablelistener.cpp

@@ -111,31 +111,31 @@ void BaseSyncableListener::setMasterInstantaneousBpm(Syncable* pSource, double b
     }
 }
 
-void BaseSyncableListener::setMasterBeatDistance(Syncable* pSource, double beat_distance) {
+void BaseSyncableListener::setMasterBeatDistance(Syncable* pSource, double beatDistance) {
     if (pSource != m_pInternalClock) {
-        m_pInternalClock->setMasterBeatDistance(beat_distance);
+        m_pInternalClock->setMasterBeatDistance(beatDistance);
     }
     foreach (Syncable* pSyncable, m_syncables) {
         if (pSyncable == pSource ||
                 !pSyncable->isSynchronized()) {
             continue;
         }
-        pSyncable->setMasterBeatDistance(beat_distance);
+        pSyncable->setMasterBeatDistance(beatDistance);
     }
 }
 
-void BaseSyncableListener::setMasterParams(Syncable* pSource, double beat_distance,
-                                           double base_bpm, double bpm) {
-    //qDebug() << "BaseSyncableListener::setMasterParams, source is" << pSource->getGroup() << beat_distance << base_bpm << bpm;
+void BaseSyncableListener::setMasterParams(
+        Syncable* pSource, double beatDistance, double baseBpm, double bpm) {
+    //qDebug() << "BaseSyncableListener::setMasterParams, source is" << pSource->getGroup() << beatDistance << baseBpm << bpm;
     if (pSource != m_pInternalClock) {
-        m_pInternalClock->setMasterParams(beat_distance, base_bpm, bpm);
+        m_pInternalClock->setMasterParams(beatDistance, baseBpm, bpm);
     }
     foreach (Syncable* pSyncable, m_syncables) {
         if (pSyncable == pSource ||
                 !pSyncable->isSynchronized()) {
             continue;
         }
-        pSyncable->setMasterParams(beat_distance, base_bpm, bpm);
+        pSyncable->setMasterParams(beatDistance, baseBpm, bpm);
     }
 }
 

src/engine/sync/basesyncablelistener.h

@@ -53,10 +53,9 @@ class BaseSyncableListener : public SyncableListener {
 
     // Set the master beat distance on every sync-enabled Syncable except
     // pSource.
-    void setMasterBeatDistance(Syncable* pSource, double beat_distance);
+    void setMasterBeatDistance(Syncable* pSource, double beatDistance);
 
-    void setMasterParams(Syncable* pSource, double beat_distance,
-                         double base_bpm, double bpm);
+    void setMasterParams(Syncable* pSource, double beatDistance, double baseBpm, double bpm);
 
     // Check if there is only one playing syncable deck, and notify it if so.
     void checkUniquePlayingSyncable();

src/engine/sync/enginesync.cpp

@@ -88,8 +88,10 @@ void EngineSync::requestSyncMode(Syncable* pSyncable, SyncMode mode) {
         // tempo, the tempo of the old master remains until we know better
         activateMaster(pSyncable, true);
         if (pSyncable->getBaseBpm() > 0) {
-            setMasterParams(pSyncable, pSyncable->getBeatDistance(),
-                            pSyncable->getBaseBpm(), pSyncable->getBpm());
+            setMasterParams(pSyncable,
+                    pSyncable->getBeatDistance(),
+                    pSyncable->getBaseBpm(),
+                    pSyncable->getBpm());
         }
     } else if (mode == SYNC_FOLLOWER ||
             mode == SYNC_MASTER_SOFT ||
@@ -252,7 +254,10 @@ void EngineSync::notifyPlaying(Syncable* pSyncable, bool playing) {
 
     if (newMaster != nullptr && newMaster != m_pMasterSyncable) {
         activateMaster(newMaster, false);
-        setMasterParams(newMaster, newMaster->getBeatDistance(), newMaster->getBaseBpm(), newMaster->getBpm());
+        setMasterParams(newMaster,
+                newMaster->getBeatDistance(),
+                newMaster->getBaseBpm(),
+                newMaster->getBpm());
     }
 
     pSyncable->requestSync();
@@ -264,9 +269,9 @@ void EngineSync::notifyScratching(Syncable* pSyncable, bool scratching) {
     Q_UNUSED(scratching);
 }
 
-void EngineSync::notifyBpmChanged(Syncable* pSyncable, double bpm) {
+void EngineSync::notifyBaseBpmChanged(Syncable* pSyncable, double bpm) {
     if (kLogger.traceEnabled()) {
-        kLogger.trace() << "EngineSync::notifyBpmChanged" << pSyncable->getGroup() << bpm;
+        kLogger.trace() << "EngineSync::notifyBaseBpmChanged" << pSyncable->getGroup() << bpm;
     }
 
     // Master Base BPM shouldn't be updated for every random deck that twiddles
@@ -310,15 +315,16 @@ void EngineSync::notifyInstantaneousBpmChanged(Syncable* pSyncable, double bpm)
     setMasterInstantaneousBpm(pSyncable, bpm);
 }
 
-void EngineSync::notifyBeatDistanceChanged(Syncable* pSyncable, double beat_distance) {
+void EngineSync::notifyBeatDistanceChanged(Syncable* pSyncable, double beatDistance) {
     if (kLogger.traceEnabled()) {
-        kLogger.trace() << "EngineSync::notifyBeatDistanceChanged" << pSyncable->getGroup() << beat_distance;
+        kLogger.trace() << "EngineSync::notifyBeatDistanceChanged"
+                        << pSyncable->getGroup() << beatDistance;
     }
     if (!isMaster(pSyncable->getSyncMode())) {
         return;
     }
 
-    setMasterBeatDistance(pSyncable, beat_distance);
+    setMasterBeatDistance(pSyncable, beatDistance);
 }
 
 void EngineSync::activateFollower(Syncable* pSyncable) {
@@ -380,7 +386,7 @@ void EngineSync::activateMaster(Syncable* pSyncable, bool explicitMaster) {
         activateFollower(m_pInternalClock);
     }
 
-    // It is up to callers of this function to initialize bpm and beat_distance
+    // It is up to callers of this function to initialize bpm and beatDistance
     // if necessary.
 }
 

src/engine/sync/enginesync.h

@@ -25,10 +25,17 @@ class EngineSync : public BaseSyncableListener {
     // Syncables notify EngineSync directly about various events. EngineSync
     // does not have a say in whether these succeed or not, they are simply
     // notifications.
-    void notifyBpmChanged(Syncable* pSyncable, double bpm) override;
+    void notifyBaseBpmChanged(Syncable* pSyncable, double bpm) override;
     void requestBpmUpdate(Syncable* pSyncable, double 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;
+
+    // the beat distance is updated on every callback.
     void notifyBeatDistanceChanged(Syncable* pSyncable, double beatDistance) override;
+
+    // Notify the engine that a syncable has started or stopped playing
     void notifyPlaying(Syncable* pSyncable, bool playing) override;
     void notifyScratching(Syncable* pSyncable, bool scratching) override;
 
@@ -54,14 +61,16 @@ class EngineSync : public BaseSyncableListener {
 
     // Find a deck to match against, used in the case where there is no sync master.
     // Looks first for a playing deck, and falls back to the first non-playing deck.
+    // If the requester is playing, don't match against a non-playing deck because
+    // that would be strange behavior for the user.
     // Returns nullptr if none can be found.
     Syncable* findBpmMatchTarget(Syncable* requester);
 
     // Activate a specific syncable as master.
     void activateMaster(Syncable* pSyncable, bool explicitMaster);
 
     // Activate a specific channel as Follower. Sets the syncable's bpm and
-    // beat_distance to match the master.
+    // beatDistance to match the master.
     void activateFollower(Syncable* pSyncable);
 
     // Unsets all sync state on a Syncable.

src/engine/sync/internalclock.cpp

@@ -30,8 +30,10 @@ InternalClock::InternalClock(const QString& group, SyncableListener* pEngineSync
     // bpm_up_small / bpm_down_small steps by 0.1
     m_pClockBpm.reset(
             new ControlLinPotmeter(ConfigKey(m_group, "bpm"), 1, 200, 1, 0.1, true));
-    connect(m_pClockBpm.data(), &ControlObject::valueChanged,
-            this, &InternalClock::slotBpmChanged,
+    connect(m_pClockBpm.data(),
+            &ControlObject::valueChanged,
+            this,
+            &InternalClock::slotBaseBpmChanged,
             Qt::DirectConnection);
 
     // The relative position between two beats in the range 0.0 ... 1.0
@@ -146,20 +148,20 @@ void InternalClock::setMasterParams(double beatDistance, double baseBpm, double
     setMasterBeatDistance(beatDistance);
 }
 
-void InternalClock::slotBpmChanged(double bpm) {
-    m_dBaseBpm = bpm;
-    updateBeatLength(m_iOldSampleRate, bpm);
+void InternalClock::slotBaseBpmChanged(double baseBpm) {
+    m_dBaseBpm = baseBpm;
+    updateBeatLength(m_iOldSampleRate, m_dBaseBpm);
     if (!isSynchronized()) {
         return;
     }
-    m_pEngineSync->notifyBpmChanged(this, bpm);
+    m_pEngineSync->notifyBaseBpmChanged(this, m_dBaseBpm);
 }
 
-void InternalClock::slotBeatDistanceChanged(double beat_distance) {
-    if (beat_distance < 0.0 || beat_distance > 1.0) {
+void InternalClock::slotBeatDistanceChanged(double beatDistance) {
+    if (beatDistance < 0.0 || beatDistance > 1.0) {
         return;
     }
-    setMasterBeatDistance(beat_distance);
+    setMasterBeatDistance(beatDistance);
 }
 
 void InternalClock::updateBeatLength(int sampleRate, double bpm) {
@@ -221,7 +223,7 @@ void InternalClock::onCallbackEnd(int sampleRate, int bufferSize) {
         m_dClockPosition -= m_dBeatLength;
     }
 
-    double beat_distance = getBeatDistance();
-    m_pClockBeatDistance->set(beat_distance);
-    m_pEngineSync->notifyBeatDistanceChanged(this, beat_distance);
+    double beatDistance = getBeatDistance();
+    m_pClockBeatDistance->set(beatDistance);
+    m_pEngineSync->notifyBeatDistanceChanged(this, beatDistance);
 }

src/engine/sync/internalclock.h

@@ -57,8 +57,8 @@ class InternalClock : public QObject, public Clock, public Syncable {
     void onCallbackEnd(int sampleRate, int bufferSize);
 
   private slots:
-    void slotBpmChanged(double bpm);
-    void slotBeatDistanceChanged(double beat_distance);
+    void slotBaseBpmChanged(double baseBpm);
+    void slotBeatDistanceChanged(double beatDistance);
     void slotSyncMasterEnabledChangeRequest(double state);
 
   private:

src/engine/sync/syncable.h

@@ -31,6 +31,10 @@ inline bool toSynchronized(SyncMode mode) {
     return mode > SYNC_NONE;
 }
 
+inline bool isFollower(SyncMode mode) {
+    return (mode == SYNC_FOLLOWER);
+}
+
 inline bool isMaster(SyncMode mode) {
     return (mode == SYNC_MASTER_SOFT || mode == SYNC_MASTER_EXPLICIT);
 }
@@ -111,7 +115,7 @@ class SyncableListener {
 
     // A Syncable must never call notifyBpmChanged in response to a setMasterBpm()
     // call.
-    virtual void notifyBpmChanged(Syncable* pSyncable, double bpm) = 0;
+    virtual void notifyBaseBpmChanged(Syncable* pSyncable, double bpm) = 0;
     virtual void requestBpmUpdate(Syncable* pSyncable, double bpm) = 0;
 
     // Syncables notify EngineSync directly about various events. EngineSync

src/engine/sync/synccontrol.cpp

@@ -306,7 +306,7 @@ void SyncControl::updateTargetBeatDistance() {
 
 double SyncControl::getBpm() const {
     if (kLogger.traceEnabled()) {
-        kLogger.trace() << getGroup() << "SyncControl::getBpm()" << m_pBpm->get();
+        kLogger.trace() << getGroup() << "SyncControl::getBpm()";
     }
     return m_pBpm->get() / m_masterBpmAdjustFactor;
 }
@@ -360,6 +360,9 @@ void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) {
 }
 
 void SyncControl::slotControlPlay(double play) {
+    if (kLogger.traceEnabled()) {
+        kLogger.trace() << "SyncControl::slotControlPlay" << getSyncMode() << play;
+    }
     m_pEngineSync->notifyPlaying(this, play > 0.0);
 }
 
@@ -398,6 +401,9 @@ void SyncControl::slotSyncModeChangeRequest(double state) {
 }
 
 void SyncControl::slotSyncMasterEnabledChangeRequest(double state) {
+    if (kLogger.traceEnabled()) {
+        kLogger.trace() << "SyncControl::slotSyncMasterEnabledChangeRequest" << getGroup();
+    }
     SyncMode mode = getSyncMode();
     if (state > 0.0) {
         if (mode == SYNC_MASTER_EXPLICIT) {
@@ -425,6 +431,9 @@ void SyncControl::slotSyncMasterEnabledChangeRequest(double state) {
 }
 
 void SyncControl::slotSyncEnabledChangeRequest(double enabled) {
+    if (kLogger.traceEnabled()) {
+        kLogger.trace() << "SyncControl::slotSyncEnabledChangeRequest" << getGroup();
+    }
     bool bEnabled = enabled > 0.0;
 
     // Allow a request for state change even if it's the same as the current
@@ -449,7 +458,7 @@ void SyncControl::setLocalBpm(double local_bpm) {
 
     double bpm = local_bpm * m_pRateRatio->get();
 
-    if (syncMode == SYNC_FOLLOWER) {
+    if (isFollower(syncMode)) {
         // In this case we need an update from the current master to adjust
         // the rate that we continue with the master BPM. If there is no
         // master bpm, our bpm value is adopted and the m_masterBpmAdjustFactor
@@ -459,7 +468,7 @@ void SyncControl::setLocalBpm(double local_bpm) {
         DEBUG_ASSERT(isMaster(syncMode));
         // We might have adopted an adjust factor when becoming master.
         // Keep it when reporting our bpm.
-        m_pEngineSync->notifyBpmChanged(this, bpm / m_masterBpmAdjustFactor);
+        m_pEngineSync->notifyBaseBpmChanged(this, bpm / m_masterBpmAdjustFactor);
     }
 }
 
@@ -471,7 +480,7 @@ void SyncControl::slotRateChanged() {
     if (bpm > 0 && isSynchronized()) {
         // When reporting our bpm, remove the multiplier so the masters all
         // think the followers have the same bpm.
-        m_pEngineSync->notifyBpmChanged(this, bpm / m_masterBpmAdjustFactor);
+        m_pEngineSync->notifyBaseBpmChanged(this, bpm / m_masterBpmAdjustFactor);
     }
 }