Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SyncLock: Don't recalc half/double multiplier on every callback #3706

Merged
merged 2 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/engine/controls/bpmcontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,7 @@ void BpmControl::setTargetBeatDistance(double beatDistance) {
m_dSyncTargetBeatDistance.setValue(beatDistance);
}

void BpmControl::setInstantaneousBpm(double instantaneousBpm) {
void BpmControl::updateInstantaneousBpm(double instantaneousBpm) {
m_dSyncInstantaneousBpm = instantaneousBpm;
}

Expand Down
2 changes: 1 addition & 1 deletion src/engine/controls/bpmcontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class BpmControl : public EngineControl {
}

void setTargetBeatDistance(double beatDistance);
void setInstantaneousBpm(double instantaneousBpm);
void updateInstantaneousBpm(double instantaneousBpm);
void resetSyncAdjustment();
double updateLocalBpm();
/// updateBeatDistance is adjusted to include the user offset so
Expand Down
4 changes: 2 additions & 2 deletions src/engine/sync/clock.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ class Clock {
virtual ~Clock() = default;

virtual double getBeatDistance() const = 0;
virtual void setMasterBeatDistance(double beatDistance) = 0;
virtual void updateMasterBeatDistance(double beatDistance) = 0;

virtual double getBpm() const = 0;
virtual void setMasterBpm(double bpm) = 0;
virtual void updateMasterBpm(double bpm) = 0;
};
68 changes: 39 additions & 29 deletions src/engine/sync/enginesync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ EngineSync::EngineSync(UserSettingsPointer pConfig)
m_pInternalClock(new InternalClock(kInternalClockGroup, this)),
m_pMasterSyncable(nullptr) {
qRegisterMetaType<SyncMode>("SyncMode");
m_pInternalClock->setMasterBpm(124.0);
m_pInternalClock->updateMasterBpm(124.0);
}

EngineSync::~EngineSync() {
Expand Down Expand Up @@ -90,14 +90,19 @@ void EngineSync::requestSyncMode(Syncable* pSyncable, SyncMode mode) {
pParamsSyncable = findBpmMatchTarget(pSyncable);
if (!pParamsSyncable) {
// We weren't able to find anything to match to, so set ourselves as the
// target. That way we'll use our own params when we setMasterParams below.
// target. That way we'll use our own params when we updateMasterBpm below.
pParamsSyncable = pSyncable;
}
}
// Now that all of the decks have their assignments, reinit master params if needed.
if (pParamsSyncable) {
setMasterParams(pParamsSyncable);
pSyncable->setInstantaneousBpm(pParamsSyncable->getBpm());
if (kLogger.traceEnabled()) {
kLogger.trace()
<< "EngineSync::requestSyncMode setting master params from "
<< pParamsSyncable->getGroup();
}
reinitMasterParams(pParamsSyncable);
pSyncable->updateInstantaneousBpm(pParamsSyncable->getBpm());
if (pParamsSyncable != pSyncable) {
pSyncable->requestSync();
}
Expand Down Expand Up @@ -194,6 +199,9 @@ void EngineSync::deactivateSync(Syncable* pSyncable) {
}

Syncable* EngineSync::pickMaster(Syncable* enabling_syncable) {
if (kLogger.traceEnabled()) {
kLogger.trace() << "EngineSync::pickMaster";
}
if (m_pMasterSyncable &&
m_pMasterSyncable->getSyncMode() == SYNC_MASTER_EXPLICIT &&
m_pMasterSyncable->getBaseBpm() != 0.0) {
Expand Down Expand Up @@ -354,14 +362,14 @@ void EngineSync::notifyPlayingAudible(Syncable* pSyncable, bool playingAudible)

if (newMaster != nullptr && newMaster != m_pMasterSyncable) {
activateMaster(newMaster, SYNC_MASTER_SOFT);
setMasterParams(newMaster);
reinitMasterParams(newMaster);
} else {
Syncable* pOnlyPlayer = getUniquePlayingSyncedDeck();
if (pOnlyPlayer) {
// Even if we didn't change master, if there is only one player (us), then we should
// reinit the beat distance.
// update the beat distance.
pOnlyPlayer->notifyUniquePlaying();
setMasterBeatDistance(pOnlyPlayer, pOnlyPlayer->getBeatDistance());
updateMasterBeatDistance(pOnlyPlayer, pOnlyPlayer->getBeatDistance());
}
}

Expand All @@ -380,7 +388,7 @@ void EngineSync::notifyBaseBpmChanged(Syncable* pSyncable, double bpm) {
}

if (isSyncMaster(pSyncable)) {
setMasterBpm(pSyncable, bpm);
updateMasterBpm(pSyncable, bpm);
}
}

Expand All @@ -389,7 +397,7 @@ void EngineSync::notifyRateChanged(Syncable* pSyncable, double bpm) {
kLogger.trace() << "EngineSync::notifyRateChanged" << pSyncable->getGroup() << bpm;
}

setMasterBpm(pSyncable, bpm);
updateMasterBpm(pSyncable, bpm);
}

void EngineSync::requestBpmUpdate(Syncable* pSyncable, double bpm) {
Expand All @@ -407,11 +415,13 @@ void EngineSync::requestBpmUpdate(Syncable* pSyncable, double bpm) {
}

if (mbaseBpm != 0.0) {
// resync to current master
pSyncable->setMasterParams(beatDistance, mbaseBpm, mbpm);
// update from current master
pSyncable->updateMasterBeatDistance(beatDistance);
pSyncable->updateMasterBpm(mbpm);
Copy link
Member Author

@ywwg ywwg May 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the fix -- we just update the beat distance and bpm, NOT basebpm. Updating basebpm triggers all of the half/double code, and we should not do that on every callback.

} else {
// There is no other master, adopt this bpm as master
pSyncable->setMasterParams(0.0, 0.0, bpm);
// There is no master, adopt this bpm as master value
pSyncable->updateMasterBeatDistance(0.0);
pSyncable->updateMasterBpm(bpm);
}
}

Expand All @@ -425,7 +435,7 @@ void EngineSync::notifyInstantaneousBpmChanged(Syncable* pSyncable, double bpm)

// Do not update the master rate slider because instantaneous changes are
// not user visible.
setMasterInstantaneousBpm(pSyncable, bpm);
updateMasterInstantaneousBpm(pSyncable, bpm);
}

void EngineSync::notifyBeatDistanceChanged(Syncable* pSyncable, double beatDistance) {
Expand All @@ -437,7 +447,7 @@ void EngineSync::notifyBeatDistanceChanged(Syncable* pSyncable, double beatDista
return;
}

setMasterBeatDistance(pSyncable, beatDistance);
updateMasterBeatDistance(pSyncable, beatDistance);
}

Syncable* EngineSync::pickNonSyncSyncTarget(EngineChannel* pDontPick) const {
Expand Down Expand Up @@ -564,52 +574,52 @@ double EngineSync::masterBaseBpm() const {
return m_pInternalClock->getBaseBpm();
}

void EngineSync::setMasterBpm(Syncable* pSource, double bpm) {
//qDebug() << "EngineSync::setMasterBpm" << pSource << bpm;
void EngineSync::updateMasterBpm(Syncable* pSource, double bpm) {
//qDebug() << "EngineSync::updateMasterBpm" << pSource << bpm;
if (pSource != m_pInternalClock) {
m_pInternalClock->setMasterBpm(bpm);
m_pInternalClock->updateMasterBpm(bpm);
}
foreach (Syncable* pSyncable, m_syncables) {
if (pSyncable == pSource ||
!pSyncable->isSynchronized()) {
continue;
}
pSyncable->setMasterBpm(bpm);
pSyncable->updateMasterBpm(bpm);
}
}

void EngineSync::setMasterInstantaneousBpm(Syncable* pSource, double bpm) {
void EngineSync::updateMasterInstantaneousBpm(Syncable* pSource, double bpm) {
if (pSource != m_pInternalClock) {
m_pInternalClock->setInstantaneousBpm(bpm);
m_pInternalClock->updateInstantaneousBpm(bpm);
}
foreach (Syncable* pSyncable, m_syncables) {
if (pSyncable == pSource ||
!pSyncable->isSynchronized()) {
continue;
}
pSyncable->setInstantaneousBpm(bpm);
pSyncable->updateInstantaneousBpm(bpm);
}
}

void EngineSync::setMasterBeatDistance(Syncable* pSource, double beatDistance) {
void EngineSync::updateMasterBeatDistance(Syncable* pSource, double beatDistance) {
if (kLogger.traceEnabled()) {
kLogger.trace() << "EngineSync::setMasterBeatDistance"
<< (pSource ? pSource->getGroup() : "null")
<< beatDistance;
}
if (pSource != m_pInternalClock) {
m_pInternalClock->setMasterBeatDistance(beatDistance);
m_pInternalClock->updateMasterBeatDistance(beatDistance);
}
foreach (Syncable* pSyncable, m_syncables) {
if (pSyncable == pSource ||
!pSyncable->isSynchronized()) {
continue;
}
pSyncable->setMasterBeatDistance(beatDistance);
pSyncable->updateMasterBeatDistance(beatDistance);
}
}

void EngineSync::setMasterParams(Syncable* pSource) {
void EngineSync::reinitMasterParams(Syncable* pSource) {
// Important note! Because of the way sync works, the new master is usually not the same
// as the Syncable setting the master parameters (here, pSource). Notify the proper Syncable
// so it can prepare itself. (This is a hack to undo half/double math so that we initialize
Expand Down Expand Up @@ -644,17 +654,17 @@ void EngineSync::setMasterParams(Syncable* pSource) {
bpm = baseBpm;
}
if (kLogger.traceEnabled()) {
kLogger.trace() << "BaseSyncableListener::setMasterParams, source is"
kLogger.trace() << "BaseSyncableListener::reinitMasterParams, source is"
<< pSource->getGroup() << beatDistance << baseBpm << bpm;
}
if (pSource != m_pInternalClock) {
m_pInternalClock->setMasterParams(beatDistance, baseBpm, bpm);
m_pInternalClock->reinitMasterParams(beatDistance, baseBpm, bpm);
}
foreach (Syncable* pSyncable, m_syncables) {
if (!pSyncable->isSynchronized()) {
continue;
}
pSyncable->setMasterParams(beatDistance, baseBpm, bpm);
pSyncable->reinitMasterParams(beatDistance, baseBpm, bpm);
}
}

Expand Down
11 changes: 7 additions & 4 deletions src/engine/sync/enginesync.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,20 @@ class EngineSync : public SyncableListener {
double masterBaseBpm() const;

/// Set the BPM on every sync-enabled Syncable except pSource.
void setMasterBpm(Syncable* pSource, double bpm);
void updateMasterBpm(Syncable* pSource, double bpm);

/// Set the master instantaneous BPM on every sync-enabled Syncable except
/// pSource.
void setMasterInstantaneousBpm(Syncable* pSource, double bpm);
void updateMasterInstantaneousBpm(Syncable* pSource, double bpm);

/// Set the master beat distance on every sync-enabled Syncable except
/// pSource.
void setMasterBeatDistance(Syncable* pSource, double beatDistance);
void updateMasterBeatDistance(Syncable* pSource, double beatDistance);

void setMasterParams(Syncable* pSource);
/// Initialize the master parameters using the provided syncable as the source.
/// This should only be called for "major" updates, like a new track or change in
/// master. Should not be called on every buffer callback.
void reinitMasterParams(Syncable* pSource);

/// Iff there is a single playing syncable in sync mode, return it.
/// This is used to initialize master params.
Expand Down
18 changes: 9 additions & 9 deletions src/engine/sync/internalclock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ double InternalClock::getBeatDistance() const {
return m_dClockPosition / m_dBeatLength;
}

void InternalClock::setMasterBeatDistance(double beatDistance) {
void InternalClock::updateMasterBeatDistance(double beatDistance) {
if (kLogger.traceEnabled()) {
kLogger.trace() << "InternalClock::setMasterBeatDistance" << beatDistance;
}
Expand All @@ -121,7 +121,7 @@ double InternalClock::getBpm() const {
return m_pClockBpm->get();
}

void InternalClock::setMasterBpm(double bpm) {
void InternalClock::updateMasterBpm(double bpm) {
if (kLogger.traceEnabled()) {
kLogger.trace() << "InternalClock::setBpm" << bpm;
}
Expand All @@ -132,7 +132,7 @@ void InternalClock::setMasterBpm(double bpm) {
updateBeatLength(m_iOldSampleRate, bpm);
}

void InternalClock::setInstantaneousBpm(double bpm) {
void InternalClock::updateInstantaneousBpm(double bpm) {
if (kLogger.traceEnabled()) {
kLogger.trace() << "InternalClock::setInstantaneousBpm" << bpm;
}
Expand All @@ -143,16 +143,16 @@ void InternalClock::setInstantaneousBpm(double bpm) {
void InternalClock::notifyMasterParamSource() {
}

void InternalClock::setMasterParams(double beatDistance, double baseBpm, double bpm) {
void InternalClock::reinitMasterParams(double beatDistance, double baseBpm, double bpm) {
if (kLogger.traceEnabled()) {
kLogger.trace() << "InternalClock::setMasterParams" << beatDistance << baseBpm << bpm;
kLogger.trace() << "InternalClock::reinitMasterParams" << beatDistance << baseBpm << bpm;
}
if (bpm <= 0.0 || baseBpm <= 0.0) {
return;
}
m_dBaseBpm = baseBpm;
setMasterBpm(bpm);
setMasterBeatDistance(beatDistance);
updateMasterBpm(bpm);
updateMasterBeatDistance(beatDistance);
}

void InternalClock::slotBaseBpmChanged(double baseBpm) {
Expand All @@ -168,7 +168,7 @@ void InternalClock::slotBeatDistanceChanged(double beatDistance) {
if (beatDistance < 0.0 || beatDistance > 1.0) {
return;
}
setMasterBeatDistance(beatDistance);
updateMasterBeatDistance(beatDistance);
}

void InternalClock::updateBeatLength(int sampleRate, double bpm) {
Expand Down Expand Up @@ -204,7 +204,7 @@ void InternalClock::updateBeatLength(int sampleRate, double bpm) {
m_iOldSampleRate = sampleRate;

// Restore the old beat distance.
setMasterBeatDistance(oldBeatDistance);
updateMasterBeatDistance(oldBeatDistance);
}

void InternalClock::onCallbackStart(int sampleRate, int bufferSize) {
Expand Down
8 changes: 4 additions & 4 deletions src/engine/sync/internalclock.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ class InternalClock : public QObject, public Clock, public Syncable {
}

double getBeatDistance() const override;
void setMasterBeatDistance(double beatDistance) override;
void updateMasterBeatDistance(double beatDistance) override;

double getBaseBpm() const override;
void setMasterBpm(double bpm) override;
void updateMasterBpm(double bpm) override;
void notifyMasterParamSource() override;
double getBpm() const override;
void setInstantaneousBpm(double bpm) override;
void setMasterParams(double beatDistance, double baseBpm, double bpm) override;
void updateInstantaneousBpm(double bpm) override;
void reinitMasterParams(double beatDistance, double baseBpm, double bpm) override;

void onCallbackStart(int sampleRate, int bufferSize);
void onCallbackEnd(int sampleRate, int bufferSize);
Expand Down
16 changes: 9 additions & 7 deletions src/engine/sync/syncable.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,26 +106,28 @@ class Syncable {
// current Sync Master.
// Must never result in a call to
// SyncableListener::notifyBeatDistanceChanged or signal loops could occur.
virtual void setMasterBeatDistance(double beatDistance) = 0;
virtual void updateMasterBeatDistance(double beatDistance) = 0;

// Update the current playback speed (not including scratch values)
// of the current master.
// Must never result in a call to SyncableListener::notifyBpmChanged or
// signal loops could occur.
virtual void setMasterBpm(double bpm) = 0;
virtual void updateMasterBpm(double bpm) = 0;

// Tells a Syncable that it's going to be used as a source for master
// params. This is a gross hack so that the SyncControl can undo its
// half/double adjustment so bpms are initialized correctly.
virtual void notifyMasterParamSource() = 0;

// Combines the above three calls into one, since they are often set
// simultaneously. Avoids redundant recalculation that would occur by
// using the three calls separately.
virtual void setMasterParams(double beatDistance, double baseBpm, double bpm) = 0;
// Perform a reset of Master parameters. This function also triggers recalculation
// of half-double multiplier.
virtual void reinitMasterParams(double beatDistance, double baseBpm, double bpm) = 0;

// Update the playback speed of the master, including scratch values.
// Must never result in a call to
// SyncableListener::notifyInstantaneousBpmChanged or signal loops could
// occur.
virtual void setInstantaneousBpm(double bpm) = 0;
virtual void updateInstantaneousBpm(double bpm) = 0;
};

/// SyncableListener is an interface class used by EngineSync to receive
Expand Down
Loading