Sync Lock: Differentiate track loading from beats updating

src/engine/controls/bpmcontrol.cpp

@@ -144,6 +144,7 @@ BpmControl::BpmControl(const QString& group,
     // Measures distance from last beat in percentage: 0.5 = half-beat away.
     m_pThisBeatDistance = new ControlProxy(group, "beat_distance", this);
     m_pSyncMode = new ControlProxy(group, "sync_mode", this);
+    m_pSyncEnabled = new ControlProxy(group, "sync_enabled", this);
 }
 
 BpmControl::~BpmControl() {
@@ -278,20 +279,13 @@ void BpmControl::slotControlBeatSyncPhase(double value) {
 }
 
 void BpmControl::slotControlBeatSyncTempo(double value) {
-    if (value == 0) {
-        return;
-    }
-
-    syncTempo();
+    m_pSyncEnabled->set(value != 0);
 }
 
 void BpmControl::slotControlBeatSync(double value) {
-    if (value == 0) {
-        return;
-    }
+    m_pSyncEnabled->set(value != 0);
 
-    if (!syncTempo()) {
-        // syncTempo failed, nothing else to do
+    if (value == 0) {
         return;
     }
 
@@ -303,78 +297,6 @@ void BpmControl::slotControlBeatSync(double value) {
     }
 }
 
-bool BpmControl::syncTempo() {
-    if (getSyncMode() == SyncMode::LeaderExplicit) {
-        return false;
-    }
-    EngineBuffer* pOtherEngineBuffer = pickSyncTarget();
-
-    if (!pOtherEngineBuffer) {
-        return false;
-    }
-
-    const auto thisBpm = getBpm();
-    const auto thisLocalBpm = getLocalBpm();
-
-    const auto otherBpm = pOtherEngineBuffer->getBpm();
-    const auto otherLocalBpm = pOtherEngineBuffer->getLocalBpm();
-
-    //qDebug() << "this" << "bpm" << thisBpm << "filebpm" << thisLocalBpm;
-    //qDebug() << "other" << "bpm" << otherBpm << "filebpm" << otherLocalBpm;
-
-    ////////////////////////////////////////////////////////////////////////////
-    // Rough proof of how syncing works -- rryan 3/2011
-    // ------------------------------------------------
-    //
-    // Let this and other denote this deck versus the sync-target deck.
-    //
-    // The goal is for this deck's effective BPM to equal the other decks.
-    //
-    // thisBpm = otherBpm
-    ///
-    // An effective BPM is the file-bpm times the rate:
-    //
-    // bpm = fileBpm * rate
-    //
-    // So our goal is to tweak thisRate such that this equation is true:
-    //
-    // thisFileBpm * (1.0 + thisRate) = otherFileBpm * (1.0 + otherRate)
-    //
-    // so rearrange this equation in terms of thisRate:
-    //
-    // thisRate = (otherFileBpm * (1.0 + otherRate)) / thisFileBpm - 1.0
-    //
-    // So the new rateScale to set is:
-    //
-    // thisRateScale = ((otherFileBpm * (1.0 + otherRate)) / thisFileBpm - 1.0) / (thisRateDir * thisRateRange)
-
-    if (otherBpm.isValid() && thisBpm.isValid() && thisLocalBpm.isValid()) {
-        // The desired rate is the other decks effective rate divided by this
-        // deck's file BPM. This gives us the playback rate that will produce an
-        // effective BPM equivalent to the other decks.
-        double desiredRate = otherBpm / thisLocalBpm;
-
-        // Test if this buffer's bpm is the double of the other one, and adjust
-        // the rate scale. I believe this is intended to account for our BPM
-        // algorithm sometimes finding double or half BPMs. This avoids drastic
-        // scales.
-
-        const double fileBpmDelta = fabs(thisLocalBpm - otherLocalBpm);
-        if (fabs(thisLocalBpm * 2.0 - otherLocalBpm) < fileBpmDelta) {
-            desiredRate /= 2.0;
-        } else if (fabs(thisLocalBpm - otherLocalBpm * 2.0) < fileBpmDelta) {
-            desiredRate *= 2.0;
-        }
-
-        if (desiredRate < 2.0 && desiredRate > 0.5) {
-            m_pEngineBpm->set(m_pLocalBpm->get() * desiredRate);
-            m_pRateRatio->set(desiredRate);
-            return true;
-        }
-    }
-    return false;
-}
-
 // static
 double BpmControl::shortestPercentageChange(const double& current_percentage,
                                             const double& target_percentage) {
@@ -1031,14 +953,21 @@ mixxx::audio::FrameDiff_t BpmControl::getPhaseOffset(mixxx::audio::FramePos this
 }
 
 void BpmControl::slotUpdateEngineBpm(double value) {
-    Q_UNUSED(value);
     // Adjust playback bpm in response to a rate_ration update
     double dRate = m_pRateRatio->get();
+
+    if (kLogger.traceEnabled()) {
+        kLogger.trace() << getGroup() << "BpmControl::slotUpdateEngineBpm"
+                        << value << m_pLocalBpm->get() << dRate;
+    }
     m_pEngineBpm->set(m_pLocalBpm->get() * dRate);
 }
 
 void BpmControl::slotUpdateRateSlider(double value) {
-    Q_UNUSED(value);
+    if (kLogger.traceEnabled()) {
+        kLogger.trace() << getGroup() << "BpmControl::slotUpdateRateSlider"
+                        << value;
+    }
     // Adjust rate slider position response to a change in rate range or m_pEngineBpm
 
     double localBpm = m_pLocalBpm->get();

src/engine/controls/bpmcontrol.h

@@ -115,7 +115,6 @@ class BpmControl : public EngineControl {
     inline bool isSynchronized() const {
         return toSynchronized(getSyncMode());
     }
-    bool syncTempo();
     double calcSyncAdjustment(bool userTweakingSync);
     void adjustBeatsBpm(double deltaBpm);
 
@@ -169,6 +168,7 @@ class BpmControl : public EngineControl {
     ControlValueAtomic<double> m_dUserOffset;
     QAtomicInt m_resetSyncAdjustment;
     ControlProxy* m_pSyncMode;
+    ControlProxy* m_pSyncEnabled;
 
     TapFilter m_tapFilter; // threadsafe
 

src/engine/enginemaster.cpp

@@ -78,7 +78,7 @@ EngineMaster::EngineMaster(
     m_pAudioLatencyOverload  = new ControlPotmeter(ConfigKey(group, "audio_latency_overload"), 0.0, 1.0);
 
     // Master sync controller
-    m_pMasterSync = new EngineSync(pConfig);
+    m_pEngineSync = new EngineSync(pConfig);
 
     // The last-used bpm value is saved in the destructor of EngineSync.
     double default_bpm = pConfig->getValue(
@@ -217,7 +217,7 @@ EngineMaster::~EngineMaster() {
     delete m_pXFaderCurve;
     delete m_pXFaderMode;
 
-    delete m_pMasterSync;
+    delete m_pEngineSync;
     delete m_pMasterSampleRate;
     delete m_pMasterLatency;
     delete m_pMasterAudioBufferSize;
@@ -271,7 +271,7 @@ const CSAMPLE* EngineMaster::getSidechainBuffer() const {
 
 void EngineMaster::processChannels(int iBufferSize) {
     // Update internal sync lock rate.
-    m_pMasterSync->onCallbackStart(m_sampleRate, m_iBufferSize);
+    m_pEngineSync->onCallbackStart(m_sampleRate, m_iBufferSize);
 
     m_activeBusChannels[EngineChannel::LEFT].clear();
     m_activeBusChannels[EngineChannel::CENTER].clear();
@@ -281,7 +281,7 @@ void EngineMaster::processChannels(int iBufferSize) {
     m_activeChannels.clear();
 
     //ScopedTimer timer("EngineMaster::processChannels");
-    EngineChannel* pMasterChannel = m_pMasterSync->getLeader();
+    EngineChannel* pLeaderChannel = m_pEngineSync->getLeaderChannel();
     // Reserve the first place for the master channel which
     // should be processed first
     m_activeChannels.append(NULL);
@@ -343,7 +343,7 @@ void EngineMaster::processChannels(int iBufferSize) {
         }
 
         // If necessary, add the channel to the list of buffers to process.
-        if (pChannel == pMasterChannel) {
+        if (pChannel == pLeaderChannel) {
             // If this is the sync master, it should be processed first.
             m_activeChannels.replace(0, pChannelInfo);
             activeChannelsStartIndex = 0;
@@ -372,7 +372,7 @@ void EngineMaster::processChannels(int iBufferSize) {
     // Note, because we call this on the internal clock first,
     // it will have an up-to-date beatDistance, whereas the other
     // Syncables will not.
-    m_pMasterSync->onCallbackEnd(m_sampleRate, m_iBufferSize);
+    m_pEngineSync->onCallbackEnd(m_sampleRate, m_iBufferSize);
 
     // After all the engines have been processed, trigger post-processing
     // which ensures that all channels are updating certain values at the

src/engine/enginemaster.h

@@ -90,7 +90,7 @@ class EngineMaster : public QObject, public AudioSource {
 
     // Provide access to the sync lock so enginebuffers can know what their rate controller is.
     EngineSync* getEngineSync() const{
-        return m_pMasterSync;
+        return m_pEngineSync;
     }
 
     // These are really only exposed for tests to use.
@@ -296,7 +296,7 @@ class EngineMaster : public QObject, public AudioSource {
     CSAMPLE* m_pSidechainMix;
 
     EngineWorkerScheduler* m_pWorkerScheduler;
-    EngineSync* m_pMasterSync;
+    EngineSync* m_pEngineSync;
 
     ControlObject* m_pMasterGain;
     ControlObject* m_pBoothGain;

src/engine/sync/enginesync.cpp

@@ -89,9 +89,8 @@ void EngineSync::requestSyncMode(Syncable* pSyncable, SyncMode mode) {
     // Second, figure out what Syncable should be used to initialize the leader
     // parameters, if any. Usually this is the new leader. (Note, that pointer might be null!)
     Syncable* pParamsSyncable = m_pLeaderSyncable;
-    // But if we are newly soft leader, we need to match to some other deck.
-    if (pSyncable == m_pLeaderSyncable && pSyncable != oldLeader &&
-            mode != SyncMode::LeaderExplicit) {
+    // But if we are newly leader, we need to match to some other deck.
+    if (pSyncable == m_pLeaderSyncable && pSyncable != oldLeader) {
         pParamsSyncable = findBpmMatchTarget(pSyncable);
         if (!pParamsSyncable) {
             // We weren't able to find anything to match to, so set ourselves as the
@@ -112,7 +111,6 @@ void EngineSync::requestSyncMode(Syncable* pSyncable, SyncMode mode) {
             pSyncable->requestSync();
         }
     }
-
 }
 
 void EngineSync::activateFollower(Syncable* pSyncable) {
@@ -214,7 +212,7 @@ Syncable* EngineSync::pickLeader(Syncable* enabling_syncable) {
         return m_pLeaderSyncable;
     }
 
-    // First preference: some other sync deck that is not playing.
+    // First preference: some other sync deck that is playing.
     // Note, if we are using PREFER_LOCK_BPM we don't use this option.
     Syncable* first_other_playing_deck = nullptr;
     // Second preference: whatever the first playing sync deck is, even if it's us.
@@ -431,7 +429,8 @@ void EngineSync::requestBpmUpdate(Syncable* pSyncable, mixxx::Bpm bpm) {
 
 void EngineSync::notifyInstantaneousBpmChanged(Syncable* pSyncable, mixxx::Bpm bpm) {
     if (kLogger.traceEnabled()) {
-        kLogger.trace() << "EngineSync::notifyInstantaneousBpmChanged" << pSyncable->getGroup() << bpm;
+        kLogger.trace() << "EngineSync::notifyInstantaneousBpmChanged"
+                        << pSyncable->getGroup() << bpm;
     }
     if (pSyncable != m_pInternalClock) {
         return;
@@ -535,7 +534,7 @@ void EngineSync::onCallbackEnd(mixxx::audio::SampleRate sampleRate, int bufferSi
     m_pInternalClock->onCallbackEnd(sampleRate, bufferSize);
 }
 
-EngineChannel* EngineSync::getLeader() const {
+EngineChannel* EngineSync::getLeaderChannel() const {
     return m_pLeaderSyncable ? m_pLeaderSyncable->getChannel() : nullptr;
 }
 

src/engine/sync/enginesync.h

@@ -57,7 +57,7 @@ class EngineSync : public SyncableListener {
     bool otherSyncedPlaying(const QString& group);
 
     void addSyncableDeck(Syncable* pSyncable);
-    EngineChannel* getLeader() const;
+    EngineChannel* getLeaderChannel() const;
     void onCallbackStart(mixxx::audio::SampleRate sampleRate, int bufferSize);
     void onCallbackEnd(mixxx::audio::SampleRate sampleRate, int bufferSize);
 

src/engine/sync/synccontrol.cpp

@@ -302,7 +302,10 @@ void SyncControl::updateTargetBeatDistance() {
         }
     }
     if (kLogger.traceEnabled()) {
-        kLogger.trace() << getGroup() << "SyncControl::updateTargetBeatDistance, adjusted target is" << targetDistance;
+        kLogger.trace()
+                << getGroup()
+                << "SyncControl::updateTargetBeatDistance, adjusted target is"
+                << targetDistance;
     }
     m_pBpmControl->setTargetBeatDistance(targetDistance);
 }
@@ -340,10 +343,6 @@ void SyncControl::trackLoaded(TrackPointer pNewTrack) {
     if (pNewTrack) {
         pBeats = pNewTrack->getBeats();
     }
-    trackBeatsUpdated(pBeats);
-}
-
-void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) {
     // This slot is fired by a new file is loaded or if the user
     // has adjusted the beatgrid.
     if (kLogger.traceEnabled()) {
@@ -355,6 +354,39 @@ void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) {
         return;
     }
 
+    const bool hadBeats = m_pBeats != nullptr;
+    m_pBeats = pBeats;
+    m_leaderBpmAdjustFactor = kBpmUnity;
+
+    m_pBpmControl->updateLocalBpm();
+    if (isSynchronized()) {
+        if (!m_pBeats) {
+            // If we were soft leader and now we have no beats, go to follower.
+            // This is a bit of "enginesync" logic that has bled into this Syncable,
+            // is there a better way to handle "soft leaders no longer have bpm"?
+            if (getSyncMode() == SyncMode::LeaderSoft) {
+                m_pChannel->getEngineBuffer()->requestSyncMode(SyncMode::Follower);
+            }
+            return;
+        }
+
+        // Re-requesting the existing sync mode will resync us.
+        m_pChannel->getEngineBuffer()->requestSyncMode(getSyncMode());
+        if (!hadBeats) {
+            // There is a chance we were beatless leader before, so we notify a basebpm change
+            // to possibly reinit leader params.
+            m_pChannel->getEngineBuffer()->requestSyncMode(getSyncMode());
+            m_pEngineSync->notifyBaseBpmChanged(this, getBaseBpm());
+        }
+    }
+}
+
+void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) {
+    // This slot is fired by if the user has adjusted the beatgrid.
+    if (true) {
+        qDebug() << getGroup() << "SyncControl::trackBeatsUpdated";
+    }
+
     m_pBeats = pBeats;
     m_leaderBpmAdjustFactor = kBpmUnity;
 
@@ -365,7 +397,7 @@ void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) {
             // be leader.
             m_pChannel->getEngineBuffer()->requestSyncMode(SyncMode::Follower);
         } else {
-            // We are remaining leader, so notify the engine with our update.
+            // We should not change playback speed.
             m_pBpmControl->updateLocalBpm();
             m_pEngineSync->notifyBaseBpmChanged(this, getBaseBpm());
         }