diff --git a/src/engine/controls/bpmcontrol.cpp b/src/engine/controls/bpmcontrol.cpp index 76971b93bdc..6fa0617040c 100644 --- a/src/engine/controls/bpmcontrol.cpp +++ b/src/engine/controls/bpmcontrol.cpp @@ -379,14 +379,6 @@ double BpmControl::calcSyncAdjustment(bool userTweakingSync) { const double error = shortestPercentageChange(syncTargetBeatDistance, thisBeatDistance); const double curUserOffset = m_dUserOffset.getValue(); - if (kLogger.traceEnabled()) { - kLogger.trace() << m_group << "****************"; - kLogger.trace() << "target beat distance:" << syncTargetBeatDistance; - kLogger.trace() << "my beat distance:" << thisBeatDistance; - kLogger.trace() << "user offset distance:" << curUserOffset; - kLogger.trace() << "error :" << error; - } - double adjustment = 1.0; if (userTweakingSync) { @@ -429,6 +421,14 @@ double BpmControl::calcSyncAdjustment(bool userTweakingSync) { } } m_dLastSyncAdjustment = adjustment; + if (kLogger.traceEnabled() && adjustment - 1.0 > 0.01) { + kLogger.trace() << m_group << "****************"; + kLogger.trace() << "target beat distance:" << syncTargetBeatDistance; + kLogger.trace() << "my beat distance:" << thisBeatDistance; + kLogger.trace() << "user offset distance:" << curUserOffset; + kLogger.trace() << "error :" << error; + kLogger.trace() << "adjustment :" << adjustment; + } return adjustment; } @@ -966,6 +966,10 @@ void BpmControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) { resetSyncAdjustment(); } +void BpmControl::notifySeek(mixxx::audio::FramePos position) { + updateBeatDistance(position); +} + void BpmControl::slotBeatsTranslate(double v) { if (v <= 0) { return; @@ -1035,7 +1039,11 @@ mixxx::Bpm BpmControl::updateLocalBpm() { } double BpmControl::updateBeatDistance() { - double beatDistance = getBeatDistance(frameInfo().currentPosition); + return updateBeatDistance(frameInfo().currentPosition); +} + +double BpmControl::updateBeatDistance(mixxx::audio::FramePos playpos) { + double beatDistance = getBeatDistance(playpos); m_pThisBeatDistance->set(beatDistance); if (!isSynchronized() && m_dUserOffset.getValue() != 0.0) { m_dUserOffset.setValue(0.0); @@ -1058,11 +1066,11 @@ void BpmControl::updateInstantaneousBpm(double instantaneousBpm) { } void BpmControl::resetSyncAdjustment() { - if (kLogger.traceEnabled()) { - kLogger.trace() << getGroup() << "BpmControl::resetSyncAdjustment"; - } // Immediately edit the beat distance to reflect the new reality. double new_distance = m_pThisBeatDistance->get() + m_dUserOffset.getValue(); + if (kLogger.traceEnabled()) { + kLogger.trace() << getGroup() << "BpmControl::resetSyncAdjustment: " << new_distance; + } m_pThisBeatDistance->set(new_distance); m_dUserOffset.setValue(0.0); m_resetSyncAdjustment = true; diff --git a/src/engine/controls/bpmcontrol.h b/src/engine/controls/bpmcontrol.h index 0cf8cd5ac88..2c7f60e2794 100644 --- a/src/engine/controls/bpmcontrol.h +++ b/src/engine/controls/bpmcontrol.h @@ -59,9 +59,13 @@ class BpmControl : public EngineControl { void updateInstantaneousBpm(double instantaneousBpm); void resetSyncAdjustment(); mixxx::Bpm updateLocalBpm(); - /// updateBeatDistance is adjusted to include the user offset so - /// it's transparent to other decks. + /// Updates the beat distance based on the current play position. + /// This override is called on every engine callback to update the + /// beatposition based on the new current playposition. double updateBeatDistance(); + /// Updates the beat distance based on the provided play position. This + /// override is used for seeks. + double updateBeatDistance(mixxx::audio::FramePos playpos); void collectFeatures(GroupFeatureState* pGroupFeatures) const; @@ -92,6 +96,7 @@ class BpmControl : public EngineControl { double getRateRatio() const; void trackLoaded(TrackPointer pNewTrack) override; void trackBeatsUpdated(mixxx::BeatsPointer pBeats) override; + void notifySeek(mixxx::audio::FramePos position) override; private slots: void slotAdjustBeatsFaster(double); diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index 86fe68bfef7..2344cc61f47 100644 --- a/src/engine/enginebuffer.cpp +++ b/src/engine/enginebuffer.cpp @@ -948,18 +948,6 @@ void EngineBuffer::processTrackLocked( // If pitch ratio and tempo ratio are equal, a linear scaler is used, // otherwise tempo and pitch are processed individual - // If we were scratching, and scratching is over, and we're a follower, - // and we're quantized, and not paused, - // 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() && - isFollower(m_pSyncControl->getSyncMode()) && !paused) { - // TODO() The resulting seek is processed in the following callback - // That is to late - requestSyncPhase(); - } - double rate = 0; // If the baserate, speed, or pitch has changed, we need to update the // scaler. Also, if we have changed scalers then we need to update the @@ -1171,12 +1159,6 @@ void EngineBuffer::process(CSAMPLE* pOutput, const int iBufferSize) { } #endif - if (isLeader(m_pSyncControl->getSyncMode())) { - // Report our speed to SyncControl immediately instead of waiting - // for postProcess so we can broadcast this update to followers. - m_pSyncControl->reportPlayerSpeed(m_speed_old, m_scratching_old); - } - m_iLastBufferSize = iBufferSize; m_bCrossfadeReady = false; } @@ -1330,11 +1312,10 @@ void EngineBuffer::postProcess(const int iBufferSize) { m_pSyncControl->setLocalBpm(newLocalBpm); m_pSyncControl->updateAudible(); SyncMode mode = m_pSyncControl->getSyncMode(); + m_pSyncControl->reportPlayerSpeed(m_speed_old, m_scratching_old); if (isLeader(mode)) { m_pEngineSync->notifyBeatDistanceChanged(m_pSyncControl, beatDistance); } else if (isFollower(mode)) { - // Report our speed to SyncControl. If we are leader, we already did this. - m_pSyncControl->reportPlayerSpeed(m_speed_old, m_scratching_old); m_pSyncControl->updateTargetBeatDistance(); } diff --git a/src/engine/sync/enginesync.cpp b/src/engine/sync/enginesync.cpp index f8c564da523..7049c35499f 100644 --- a/src/engine/sync/enginesync.cpp +++ b/src/engine/sync/enginesync.cpp @@ -378,9 +378,40 @@ void EngineSync::notifyPlayingAudible(Syncable* pSyncable, bool playingAudible) } void EngineSync::notifyScratching(Syncable* pSyncable, bool scratching) { - // No special behavior for now. - Q_UNUSED(pSyncable); - Q_UNUSED(scratching); + if (!pSyncable->isPlaying() || !pSyncable->isQuantized()) { + return; + } + // Only take action if scratching is turning off. + if (scratching) { + return; + } + if (isFollower(pSyncable->getSyncMode())) { + pSyncable->getChannel()->getEngineBuffer()->requestSyncPhase(); + return; + } + if (isLeader(pSyncable->getSyncMode())) { + Syncable* pOnlyPlayer = getUniquePlayingSyncedDeck(); + if (pOnlyPlayer) { + // Even if we didn't change leader, if there is only one player (us), then we should + // reinit the beat distance. + pOnlyPlayer->notifyUniquePlaying(); + updateLeaderBeatDistance(pOnlyPlayer, pOnlyPlayer->getBeatDistance()); + } else { + // If the Leader isn't the only player, then it will need to sync + // phase like followers do. + pSyncable->getChannel()->getEngineBuffer()->requestSyncPhase(); + } + } +} + +void EngineSync::notifySeek(Syncable* pSyncable, mixxx::audio::FramePos position) { + Q_UNUSED(position); + if (isLeader(pSyncable->getSyncMode())) { + // This relies on the bpmcontrol being notified about the seek before + // the sync control, but that's ok because that's intrinsic to how the + // controls are constructed (see the constructor of enginebuffer). + updateLeaderBeatDistance(pSyncable, pSyncable->getBeatDistance()); + } } void EngineSync::notifyBaseBpmChanged(Syncable* pSyncable, mixxx::Bpm bpm) { diff --git a/src/engine/sync/enginesync.h b/src/engine/sync/enginesync.h index 3fb7d3ea8e4..d072d80acb6 100644 --- a/src/engine/sync/enginesync.h +++ b/src/engine/sync/enginesync.h @@ -40,6 +40,7 @@ class EngineSync : public SyncableListener { /// Notify the engine that a syncable has started or stopped playing void notifyPlayingAudible(Syncable* pSyncable, bool playingAudible) override; void notifyScratching(Syncable* pSyncable, bool scratching) override; + void notifySeek(Syncable* pSyncable, mixxx::audio::FramePos position) override; /// Used to pick a sync target for cases where Leader sync mode is not sufficient. /// Guaranteed to pick a Syncable that is a real deck and has an EngineBuffer, diff --git a/src/engine/sync/internalclock.h b/src/engine/sync/internalclock.h index 4dc7db7a435..e43c83b06a9 100644 --- a/src/engine/sync/internalclock.h +++ b/src/engine/sync/internalclock.h @@ -47,6 +47,9 @@ class InternalClock : public QObject, public Clock, public Syncable { bool isAudible() const override { return false; } + bool isQuantized() const override { + return true; + } double getBeatDistance() const override; void updateLeaderBeatDistance(double beatDistance) override; diff --git a/src/engine/sync/syncable.h b/src/engine/sync/syncable.h index 06b04acb44e..ea3bd1906c2 100644 --- a/src/engine/sync/syncable.h +++ b/src/engine/sync/syncable.h @@ -3,6 +3,7 @@ #include #include +#include "audio/frame.h" #include "track/bpm.h" class EngineChannel; @@ -112,6 +113,7 @@ class Syncable { // Only relevant for player Syncables. virtual bool isPlaying() const = 0; virtual bool isAudible() const = 0; + virtual bool isQuantized() const = 0; // Gets the current speed of the syncable in bpm (bpm * rate slider), doesn't // include scratch or FF/REW values. @@ -175,6 +177,9 @@ class SyncableListener { // Notify Syncable that the Syncable's scratching state changed. virtual void notifyScratching(Syncable* pSyncable, bool scratching) = 0; + // Notify that the Syncable has seeked. + virtual void notifySeek(Syncable* pSyncable, mixxx::audio::FramePos position) = 0; + // A Syncable must never call notifyBeatDistanceChanged in response to a // setBeatDistance() call. virtual void notifyBeatDistanceChanged( diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index d42d31cfaaa..47e2e2b0b89 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -198,6 +198,10 @@ bool SyncControl::isAudible() const { return m_audible; } +bool SyncControl::isQuantized() const { + return m_pQuantize->toBool(); +} + double SyncControl::adjustSyncBeatDistance(double beatDistance) const { // Similar to adjusting the target beat distance, when we report our beat // distance we need to adjust it by the leader bpm adjustment factor. If @@ -299,6 +303,10 @@ double SyncControl::determineBpmMultiplier(mixxx::Bpm myBpm, mixxx::Bpm targetBp } void SyncControl::updateTargetBeatDistance() { + updateTargetBeatDistance(frameInfo().currentPosition); +} + +void SyncControl::updateTargetBeatDistance(mixxx::audio::FramePos refPosition) { double targetDistance = m_unmultipliedTargetBeatDistance; if (kLogger.traceEnabled()) { kLogger.trace() @@ -320,8 +328,7 @@ void SyncControl::updateTargetBeatDistance() { } else if (m_leaderBpmAdjustFactor == kBpmHalve) { targetDistance *= kBpmHalve; // Our beat distance CO is still a buffer behind, so take the current value. - if (m_pBpmControl->getBeatDistance( - frameInfo().currentPosition) >= 0.5) { + if (m_pBpmControl->getBeatDistance(refPosition) >= 0.5) { targetDistance += 0.5; } } @@ -370,7 +377,7 @@ void SyncControl::trackLoaded(TrackPointer pNewTrack) { // This slot is fired by a new file is loaded or if the user // has adjusted the beatgrid. if (kLogger.traceEnabled()) { - kLogger.trace() << getGroup() << "SyncControl::trackBeatsUpdated"; + kLogger.trace() << getGroup() << "SyncControl::trackLoaded"; } VERIFY_OR_DEBUG_ASSERT(m_pLocalBpm) { @@ -434,6 +441,10 @@ void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) { } } +void SyncControl::notifySeek(mixxx::audio::FramePos position) { + m_pEngineSync->notifySeek(this, position); +} + void SyncControl::slotControlBeatSyncPhase(double value) { if (value == 0) { return; diff --git a/src/engine/sync/synccontrol.h b/src/engine/sync/synccontrol.h index 3ea97fde856..3af5f561e44 100644 --- a/src/engine/sync/synccontrol.h +++ b/src/engine/sync/synccontrol.h @@ -41,10 +41,17 @@ class SyncControl : public EngineControl, public Syncable { void requestSync() override; bool isPlaying() const override; bool isAudible() const override; + bool isQuantized() const override; double adjustSyncBeatDistance(double beatDistance) const; double getBeatDistance() const override; + /// updateTargetBeatDistance calculates the correct beat distance that + /// we should sync against. This may be different from the leader's + /// distance due to half/double calculation. This override is called once + /// per callback and uses the current playposition as a reference point. void updateTargetBeatDistance(); + /// This override uses the provided position, and is used after seeks. + void updateTargetBeatDistance(mixxx::audio::FramePos position); mixxx::Bpm getBaseBpm() const override; // The local bpm is the base bpm of the track around the current position. @@ -72,6 +79,7 @@ class SyncControl : public EngineControl, public Syncable { void reportPlayerSpeed(double speed, bool scratching); void trackLoaded(TrackPointer pNewTrack) override; void trackBeatsUpdated(mixxx::BeatsPointer pBeats) override; + void notifySeek(mixxx::audio::FramePos position) override; private slots: void slotControlBeatSync(double); diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index 46bf3164585..c2027a7d30e 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -2673,6 +2673,75 @@ TEST_F(EngineSyncTest, SeekStayInPhase) { EXPECT_DOUBLE_EQ(0.18925937554508981, ControlObject::get(ConfigKey(m_sGroup1, "playposition"))); } +TEST_F(EngineSyncTest, ScratchEndOtherStoppedTrackStayInPhase) { + // After scratching, confirm that we are still in phase. + // This version tests with a stopped other track. + mixxx::BeatsPointer pBeats1 = + mixxx::Beats::fromConstTempo(m_pTrack1->getSampleRate(), + mixxx::audio::kStartFramePos, + mixxx::Bpm(130)); + m_pTrack1->trySetBeats(pBeats1); + ControlObject::set(ConfigKey(m_sGroup1, "quantize"), 1.0); + ControlObject::set(ConfigKey(m_sGroup1, "sync_enabled"), 1); + + mixxx::BeatsPointer pBeats2 = + mixxx::Beats::fromConstTempo(m_pTrack2->getSampleRate(), + mixxx::audio::kStartFramePos, + mixxx::Bpm(125)); + m_pTrack2->trySetBeats(pBeats2); + ControlObject::set(ConfigKey(m_sGroup2, "quantize"), 1.0); + ControlObject::set(ConfigKey(m_sGroup2, "sync_enabled"), 1); + + ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0); + ProcessBuffer(); + ProcessBuffer(); + ControlObject::set(ConfigKey(m_sGroup1, "scratch2_enable"), 1.0); + ControlObject::set(ConfigKey(m_sGroup1, "scratch2"), 20.0); + ProcessBuffer(); + ControlObject::set(ConfigKey(m_sGroup1, "scratch2_enable"), 0.0); + ProcessBuffer(); + ProcessBuffer(); + + EXPECT_NEAR(ControlObject::get(ConfigKey(m_sInternalClockGroup, "beat_distance")), + ControlObject::get(ConfigKey(m_sGroup1, "beat_distance")), + 1e-8); +} + +TEST_F(EngineSyncTest, ScratchEndOtherPlayingTrackStayInPhase) { + // After scratching, confirm that we are still in phase. + // This version tests with a playing other track. + mixxx::BeatsPointer pBeats1 = + mixxx::Beats::fromConstTempo(m_pTrack1->getSampleRate(), + mixxx::audio::kStartFramePos, + mixxx::Bpm(130)); + m_pTrack1->trySetBeats(pBeats1); + ControlObject::set(ConfigKey(m_sGroup1, "quantize"), 1.0); + ControlObject::set(ConfigKey(m_sGroup1, "sync_enabled"), 1); + + mixxx::BeatsPointer pBeats2 = + mixxx::Beats::fromConstTempo(m_pTrack2->getSampleRate(), + mixxx::audio::kStartFramePos, + mixxx::Bpm(125)); + m_pTrack2->trySetBeats(pBeats2); + ControlObject::set(ConfigKey(m_sGroup2, "quantize"), 1.0); + ControlObject::set(ConfigKey(m_sGroup2, "sync_enabled"), 1); + + ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0); + ControlObject::set(ConfigKey(m_sGroup2, "play"), 1.0); + ProcessBuffer(); + ProcessBuffer(); + ControlObject::set(ConfigKey(m_sGroup1, "scratch2_enable"), 1.0); + ControlObject::set(ConfigKey(m_sGroup1, "scratch2"), 20.0); + ProcessBuffer(); + ControlObject::set(ConfigKey(m_sGroup1, "scratch2_enable"), 0.0); + ProcessBuffer(); + ProcessBuffer(); + + EXPECT_NEAR(ControlObject::get(ConfigKey(m_sInternalClockGroup, "beat_distance")), + ControlObject::get(ConfigKey(m_sGroup1, "beat_distance")), + 1e-8); +} + TEST_F(EngineSyncTest, SyncWithoutBeatgrid) { // this tests bug lp1783020, notresetting rate when other deck has no beatgrid mixxx::BeatsPointer pBeats1 = mixxx::Beats::fromConstTempo(