diff --git a/src/engine/controls/bpmcontrol.h b/src/engine/controls/bpmcontrol.h index ad4a1b7f408..f3bcaf3de0f 100644 --- a/src/engine/controls/bpmcontrol.h +++ b/src/engine/controls/bpmcontrol.h @@ -176,4 +176,5 @@ class BpmControl : public EngineControl { FRIEND_TEST(EngineSyncTest, UserTweakPreservedInSeek); FRIEND_TEST(EngineSyncTest, FollowerUserTweakPreservedInLeaderChange); + FRIEND_TEST(EngineSyncTest, FollowerUserTweakPreservedInSyncDisable); }; diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index 408f0094153..2158b165fe7 100644 --- a/src/engine/enginebuffer.cpp +++ b/src/engine/enginebuffer.cpp @@ -1229,6 +1229,7 @@ void EngineBuffer::processSyncRequests() { } void EngineBuffer::processSeek(bool paused) { + m_previousBufferSeek = false; // Check if we are cloning another channel before doing any seeking. EngineChannel* pChannel = m_pChannelToCloneFrom.fetchAndStoreRelaxed(nullptr); if (pChannel) { @@ -1298,6 +1299,7 @@ void EngineBuffer::processSeek(bool paused) { kLogger.trace() << "EngineBuffer::processSeek" << getGroup() << "Seek to" << position; } setNewPlaypos(position); + m_previousBufferSeek = true; } // Reset the m_queuedSeek value after it has been processed in // setNewPlaypos() so that the Engine Controls have always access to the diff --git a/src/engine/enginebuffer.h b/src/engine/enginebuffer.h index 83d8126ed57..21fb4d2ca55 100644 --- a/src/engine/enginebuffer.h +++ b/src/engine/enginebuffer.h @@ -228,7 +228,11 @@ class EngineBuffer : public EngineObject { void processSyncRequests(); void processSeek(bool paused); - + // For debugging / testing -- returns true if the previous buffer call resulted in a seek. + FRIEND_TEST(EngineSyncTest, FollowerUserTweakPreservedInSyncDisable); + bool previousBufferSeek() const { + return m_previousBufferSeek; + } bool updateIndicatorsAndModifyPlay(bool newPlay, bool oldPlay); void verifyPlay(); void notifyTrackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack); @@ -389,6 +393,7 @@ class EngineBuffer : public EngineObject { QAtomicInt m_iEnableSyncQueued; QAtomicInt m_iSyncModeQueued; ControlValueAtomic m_queuedSeek; + bool m_previousBufferSeek = false; /// Indicates that no seek is queued static constexpr QueuedSeek kNoQueuedSeek = {mixxx::audio::kInvalidFramePos, SEEK_NONE}; diff --git a/src/engine/sync/enginesync.cpp b/src/engine/sync/enginesync.cpp index e1be5c11868..d82dfb28668 100644 --- a/src/engine/sync/enginesync.cpp +++ b/src/engine/sync/enginesync.cpp @@ -85,7 +85,7 @@ 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. + // But if we are newly soft leader, we need to match to some other deck. if (pSyncable == m_pLeaderSyncable && pSyncable != oldLeader && mode != SyncMode::LeaderExplicit) { pParamsSyncable = findBpmMatchTarget(pSyncable); @@ -104,7 +104,7 @@ void EngineSync::requestSyncMode(Syncable* pSyncable, SyncMode mode) { } reinitLeaderParams(pParamsSyncable); pSyncable->updateInstantaneousBpm(pParamsSyncable->getBpm()); - if (pParamsSyncable != pSyncable) { + if (pParamsSyncable != pSyncable && mode != SyncMode::None) { pSyncable->requestSync(); } } diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index 53145d08c4b..0ba03ae08b9 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -2409,6 +2409,40 @@ TEST_F(EngineSyncTest, FollowerUserTweakPreservedInLeaderChange) { } } +TEST_F(EngineSyncTest, FollowerUserTweakPreservedInSyncDisable) { + // Ensure that when a follower disables sync, a phase seek is not performed. + // This is about 128 bpm, but results in nice round numbers of samples. + const double kDivisibleBpm = 44100.0 / 344.0; + mixxx::BeatsPointer pBeats1 = BeatFactory::makeBeatGrid( + m_pTrack1->getSampleRate(), mixxx::Bpm(kDivisibleBpm), mixxx::audio::kStartFramePos); + m_pTrack1->trySetBeats(pBeats1); + mixxx::BeatsPointer pBeats2 = BeatFactory::makeBeatGrid( + m_pTrack2->getSampleRate(), mixxx::Bpm(130), mixxx::audio::kStartFramePos); + m_pTrack2->trySetBeats(pBeats2); + + ControlObject::getControl(ConfigKey(m_sGroup1, "sync_leader"))->set(1); + ControlObject::getControl(ConfigKey(m_sGroup2, "sync_enabled"))->set(1); + ControlObject::set(ConfigKey(m_sGroup1, "quantize"), 1.0); + ControlObject::set(ConfigKey(m_sGroup2, "quantize"), 1.0); + ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0); + ControlObject::set(ConfigKey(m_sGroup2, "play"), 1.0); + ProcessBuffer(); + + // Apply user tweak offset to follower + m_pChannel2->getEngineBuffer() + ->m_pBpmControl->m_dUserOffset.setValue(0.3); + + EXPECT_TRUE(isExplicitLeader(m_sGroup1)); + EXPECT_TRUE(isFollower(m_sGroup2)); + + ProcessBuffer(); + + // Disable sync, no seek should occur + ControlObject::getControl(ConfigKey(m_sGroup2, "sync_enabled"))->set(0); + ProcessBuffer(); + EXPECT_FALSE(m_pChannel2->getEngineBuffer()->previousBufferSeek()); +} + TEST_F(EngineSyncTest, LeaderUserTweakPreservedInLeaderChange) { // Ensure that when the leader deck changes, the user offset is accounted for when // reinitializing the leader parameters..