From c8a23007cbafeecc4a5be748ad9cbb473158fe2e Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Sun, 14 Mar 2021 16:49:48 -0400 Subject: [PATCH] Sync Lock: minor refactoring --- src/engine/controls/bpmcontrol.cpp | 18 +++++++++++----- src/engine/controls/ratecontrol.cpp | 2 +- src/engine/enginebuffer.cpp | 18 ++++++++-------- src/engine/sync/basesyncablelistener.cpp | 16 +++++++-------- src/engine/sync/basesyncablelistener.h | 5 ++--- src/engine/sync/enginesync.cpp | 24 ++++++++++++++-------- src/engine/sync/enginesync.h | 13 ++++++++++-- src/engine/sync/internalclock.cpp | 26 +++++++++++++----------- src/engine/sync/internalclock.h | 4 ++-- src/engine/sync/syncable.h | 6 +++++- src/engine/sync/synccontrol.cpp | 17 ++++++++++++---- src/engine/sync/synccontrol.h | 3 +++ src/test/signalpathtest.h | 1 + 13 files changed, 97 insertions(+), 56 deletions(-) diff --git a/src/engine/controls/bpmcontrol.cpp b/src/engine/controls/bpmcontrol.cpp index 561a9567f4c..340b87a5adf 100644 --- a/src/engine/controls/bpmcontrol.cpp +++ b/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) { diff --git a/src/engine/controls/ratecontrol.cpp b/src/engine/controls/ratecontrol.cpp index 404f55eb8b5..96e13a56cb9 100644 --- a/src/engine/controls/ratecontrol.cpp +++ b/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"; diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index 5c233de12be..b81f1ea2913 100644 --- a/src/engine/enginebuffer.cpp +++ b/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(); diff --git a/src/engine/sync/basesyncablelistener.cpp b/src/engine/sync/basesyncablelistener.cpp index e2c309c6bc4..e36aefa1caf 100644 --- a/src/engine/sync/basesyncablelistener.cpp +++ b/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); } } diff --git a/src/engine/sync/basesyncablelistener.h b/src/engine/sync/basesyncablelistener.h index ca3376ca83f..2a875d4d77e 100644 --- a/src/engine/sync/basesyncablelistener.h +++ b/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(); diff --git a/src/engine/sync/enginesync.cpp b/src/engine/sync/enginesync.cpp index bf27fbd1a72..f4508c4fb4e 100644 --- a/src/engine/sync/enginesync.cpp +++ b/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. } diff --git a/src/engine/sync/enginesync.h b/src/engine/sync/enginesync.h index 86304cd73ed..db23b2b0e3f 100644 --- a/src/engine/sync/enginesync.h +++ b/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,6 +61,8 @@ 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); @@ -61,7 +70,7 @@ class EngineSync : public BaseSyncableListener { 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. diff --git a/src/engine/sync/internalclock.cpp b/src/engine/sync/internalclock.cpp index d12eebfdf49..ba72efda5be 100644 --- a/src/engine/sync/internalclock.cpp +++ b/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); } diff --git a/src/engine/sync/internalclock.h b/src/engine/sync/internalclock.h index a243b5e770c..37be36ba297 100644 --- a/src/engine/sync/internalclock.h +++ b/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: diff --git a/src/engine/sync/syncable.h b/src/engine/sync/syncable.h index 12d0c79ada1..9523c22c737 100644 --- a/src/engine/sync/syncable.h +++ b/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 diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index 25ebc35b1d4..b8f8bf8a7b9 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/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); } } diff --git a/src/engine/sync/synccontrol.h b/src/engine/sync/synccontrol.h index 66e797d7347..548dc455b65 100644 --- a/src/engine/sync/synccontrol.h +++ b/src/engine/sync/synccontrol.h @@ -38,6 +38,9 @@ class SyncControl : public EngineControl, public Syncable { double getBeatDistance() const override; void updateTargetBeatDistance(); double getBaseBpm() const override; + + // The local bpm is the base bpm of the track around the current position. + // For beatmap tracks, this can change with every beat. void setLocalBpm(double local_bpm); // Must never result in a call to diff --git a/src/test/signalpathtest.h b/src/test/signalpathtest.h index bbb057b630d..222158950db 100644 --- a/src/test/signalpathtest.h +++ b/src/test/signalpathtest.h @@ -217,6 +217,7 @@ class BaseSignalPathTest : public MixxxTest { } void ProcessBuffer() { + qDebug() << "------- Process Buffer -------"; m_pEngineMaster->process(kProcessBufferSize); }