From 0d1eb3fc53e2b77975137b24417f0f39e0d6705d Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Thu, 22 Jul 2021 20:57:41 -0400 Subject: [PATCH 01/12] Sync Lock: Differentiate track loading from beats updating --- src/engine/controls/bpmcontrol.cpp | 3 -- src/engine/enginemaster.cpp | 2 +- src/engine/sync/enginesync.cpp | 10 ++--- src/engine/sync/enginesync.h | 2 +- src/engine/sync/synccontrol.cpp | 40 +++++++++++++++--- src/test/enginesynctest.cpp | 68 +++++++++++++++--------------- 6 files changed, 75 insertions(+), 50 deletions(-) diff --git a/src/engine/controls/bpmcontrol.cpp b/src/engine/controls/bpmcontrol.cpp index 05aafdb4049..984f410ac0e 100644 --- a/src/engine/controls/bpmcontrol.cpp +++ b/src/engine/controls/bpmcontrol.cpp @@ -302,9 +302,6 @@ void BpmControl::slotControlBeatSync(double value) { } bool BpmControl::syncTempo() { - if (getSyncMode() == SyncMode::LeaderExplicit) { - return false; - } EngineBuffer* pOtherEngineBuffer = pickSyncTarget(); if (!pOtherEngineBuffer) { diff --git a/src/engine/enginemaster.cpp b/src/engine/enginemaster.cpp index 34cb9d22e86..0acbaf0e433 100644 --- a/src/engine/enginemaster.cpp +++ b/src/engine/enginemaster.cpp @@ -281,7 +281,7 @@ void EngineMaster::processChannels(int iBufferSize) { m_activeChannels.clear(); //ScopedTimer timer("EngineMaster::processChannels"); - EngineChannel* pMasterChannel = m_pMasterSync->getLeader(); + EngineChannel* pMasterChannel = m_pMasterSync->getLeaderChannel(); // Reserve the first place for the master channel which // should be processed first m_activeChannels.append(NULL); diff --git a/src/engine/sync/enginesync.cpp b/src/engine/sync/enginesync.cpp index 7b8faf9b9c6..5137117bffb 100644 --- a/src/engine/sync/enginesync.cpp +++ b/src/engine/sync/enginesync.cpp @@ -85,9 +85,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 @@ -108,7 +107,6 @@ void EngineSync::requestSyncMode(Syncable* pSyncable, SyncMode mode) { pSyncable->requestSync(); } } - } void EngineSync::activateFollower(Syncable* pSyncable) { @@ -210,7 +208,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. @@ -533,7 +531,7 @@ void EngineSync::onCallbackEnd(int sampleRate, int bufferSize) { m_pInternalClock->onCallbackEnd(sampleRate, bufferSize); } -EngineChannel* EngineSync::getLeader() const { +EngineChannel* EngineSync::getLeaderChannel() const { return m_pLeaderSyncable ? m_pLeaderSyncable->getChannel() : nullptr; } diff --git a/src/engine/sync/enginesync.h b/src/engine/sync/enginesync.h index 22018ae7727..4507d99e68e 100644 --- a/src/engine/sync/enginesync.h +++ b/src/engine/sync/enginesync.h @@ -56,7 +56,7 @@ class EngineSync : public SyncableListener { bool otherSyncedPlaying(const QString& group); void addSyncableDeck(Syncable* pSyncable); - EngineChannel* getLeader() const; + EngineChannel* getLeaderChannel() const; void onCallbackStart(int sampleRate, int bufferSize); void onCallbackEnd(int sampleRate, int bufferSize); diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index 9b863b4cb6c..1536f5b3a9f 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -332,14 +332,10 @@ 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()) { - kLogger.trace() << getGroup() << "SyncControl::trackBeatsUpdated"; + kLogger.trace() << getGroup() << "SyncControl::trackLoaded"; } VERIFY_OR_DEBUG_ASSERT(m_pLocalBpm) { @@ -347,6 +343,38 @@ void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) { return; } + 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; + } + + m_pBpmControl->syncTempo(); + 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 (kLogger.traceEnabled()) { + kLogger.trace() << getGroup() << "SyncControl::trackBeatsUpdated"; + } + m_pBeats = pBeats; m_leaderBpmAdjustFactor = kBpmUnity; @@ -357,7 +385,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()); } diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index f5774e9d19c..c7b9e6cc536 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -89,7 +89,7 @@ class EngineSyncTest : public MockedEngineBackendTest { } void assertNoLeader() { - EXPECT_EQ(NULL, m_pEngineSync->getLeader()); + EXPECT_EQ(NULL, m_pEngineSync->getLeaderChannel()); EXPECT_EQ(NULL, m_pEngineSync->getLeaderSyncable()); } @@ -106,7 +106,7 @@ class EngineSyncTest : public MockedEngineBackendTest { qWarning() << "internal clock sync_leader should be 2.0, is" << leader; return false; } - if (m_pEngineSync->getLeader()) { + if (m_pEngineSync->getLeaderChannel()) { qWarning() << "no current leader"; return false; } @@ -117,28 +117,28 @@ class EngineSyncTest : public MockedEngineBackendTest { return true; } if (group == m_sGroup1) { - if (m_pEngineSync->getLeader() != m_pChannel1) { + if (m_pEngineSync->getLeaderChannel() != m_pChannel1) { qWarning() << "leader pointer should be channel 1, is " - << (m_pEngineSync->getLeader() - ? m_pEngineSync->getLeader() + << (m_pEngineSync->getLeaderChannel() + ? m_pEngineSync->getLeaderChannel() ->getGroup() : "null"); return false; } } else if (group == m_sGroup2) { - if (m_pEngineSync->getLeader() != m_pChannel2) { + if (m_pEngineSync->getLeaderChannel() != m_pChannel2) { qWarning() << "leader pointer should be channel 2, is " - << (m_pEngineSync->getLeader() - ? m_pEngineSync->getLeader() + << (m_pEngineSync->getLeaderChannel() + ? m_pEngineSync->getLeaderChannel() ->getGroup() : "null"); return false; } } else if (group == m_sGroup3) { - if (m_pEngineSync->getLeader() != m_pChannel3) { + if (m_pEngineSync->getLeaderChannel() != m_pChannel3) { qWarning() << "leader pointer should be channel 3, is " - << (m_pEngineSync->getLeader() - ? m_pEngineSync->getLeader() + << (m_pEngineSync->getLeaderChannel() + ? m_pEngineSync->getLeaderChannel() ->getGroup() : "null"); return false; @@ -702,14 +702,13 @@ TEST_F(EngineSyncTest, RateChangeTestOrder3) { EXPECT_DOUBLE_EQ( 120.0, ControlObject::get(ConfigKey(m_sGroup2, "file_bpm"))); - // Turn on Leader. Setting explicit leader causes this track's rate to be adopted instead - // of matching against the other deck. + // Turn on Leader. Even though it is explicit leader, it still matches the other deck. auto pButtonLeaderSync1 = std::make_unique(m_sGroup1, "sync_mode"); pButtonLeaderSync1->set(static_cast(SyncMode::LeaderExplicit)); ProcessBuffer(); EXPECT_TRUE(isExplicitLeader(m_sGroup1)); - EXPECT_DOUBLE_EQ(160.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm"))); + EXPECT_DOUBLE_EQ(120.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm"))); // Turn on follower. auto pButtonLeaderSync2 = @@ -718,12 +717,12 @@ TEST_F(EngineSyncTest, RateChangeTestOrder3) { ProcessBuffer(); // Follower should immediately set its slider. - EXPECT_NEAR(getRateSliderValue(1.3333333333), - ControlObject::get(ConfigKey(m_sGroup2, "rate")), + EXPECT_NEAR(getRateSliderValue(0.75), + ControlObject::get(ConfigKey(m_sGroup1, "rate")), kMaxFloatingPointErrorLowPrecision); - EXPECT_DOUBLE_EQ(160.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm"))); + EXPECT_DOUBLE_EQ(120.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm"))); EXPECT_DOUBLE_EQ( - 160.0, ControlObject::get(ConfigKey(m_sInternalClockGroup, "bpm"))); + 120.0, ControlObject::get(ConfigKey(m_sInternalClockGroup, "bpm"))); } TEST_F(EngineSyncTest, FollowerRateChange) { @@ -846,12 +845,12 @@ TEST_F(EngineSyncTest, LeaderStopSliderCheck) { m_pTrack2->getSampleRate(), mixxx::Bpm(128), mixxx::audio::kStartFramePos); m_pTrack2->trySetBeats(pBeats2); - auto pButtonLeaderSync1 = - std::make_unique(m_sGroup1, "sync_mode"); - pButtonLeaderSync1->set(static_cast(SyncMode::LeaderExplicit)); auto pButtonLeaderSync2 = std::make_unique(m_sGroup2, "sync_mode"); pButtonLeaderSync2->set(static_cast(SyncMode::Follower)); + auto pButtonLeaderSync1 = + std::make_unique(m_sGroup1, "sync_mode"); + pButtonLeaderSync1->set(static_cast(SyncMode::LeaderExplicit)); ProcessBuffer(); //EXPECT_TRUE(isExplicitLeader(m_sGroup1)); @@ -1006,7 +1005,7 @@ TEST_F(EngineSyncTest, LoadTrackInitializesLeader) { pButtonSyncEnabled1->set(1.0); // No leader because this deck has no track. - EXPECT_EQ(NULL, m_pEngineSync->getLeader()); + EXPECT_EQ(NULL, m_pEngineSync->getLeaderChannel()); EXPECT_DOUBLE_EQ( 0.0, ControlObject::getControl(ConfigKey(m_sGroup1, "bpm"))->get()); @@ -1016,13 +1015,12 @@ TEST_F(EngineSyncTest, LoadTrackInitializesLeader) { EXPECT_DOUBLE_EQ(140.0, ControlObject::getControl(ConfigKey(m_sGroup1, "bpm"))->get()); - // But as soon as we play, deck 1 is leader + // Play button doesn't affect leader. ControlObject::getControl(ConfigKey(m_sGroup1, "play"))->set(1.0); EXPECT_TRUE(isSoftLeader(m_sGroup1)); ControlObject::getControl(ConfigKey(m_sGroup1, "play"))->set(0.0); - // If sync is on two decks and we load a track in only one of them, it will be - // leader. + // Ejecting the track has no effect on leader status m_pChannel1->getEngineBuffer()->slotEjectTrack(1.0); EXPECT_TRUE(isFollower(m_sGroup1)); // no relevant tempo available so internal clock is following @@ -1069,7 +1067,8 @@ TEST_F(EngineSyncTest, LoadTrackResetTempoOption) { std::make_unique(m_sGroup2, "sync_enabled"); pButtonSyncEnabled2->set(1.0); - // If sync is on and we load a track, that should initialize leader. + // If sync is on and we load a track, it should initialize leader because + // the other track has no bpm. TrackPointer track1 = m_pMixerDeck1->loadFakeTrack(false, 140.0); EXPECT_DOUBLE_EQ( @@ -1102,12 +1101,13 @@ TEST_F(EngineSyncTest, LoadTrackResetTempoOption) { EXPECT_DOUBLE_EQ(140.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm"))); // Repeat with RESET_NONE + EXPECT_TRUE(isSoftLeader(m_sGroup1)); m_pConfig->set(ConfigKey("[Controls]", "SpeedAutoReset"), ConfigValue(BaseTrackPlayer::RESET_NONE)); ControlObject::set(ConfigKey(m_sGroup1, "play"), 0.0); ControlObject::set(ConfigKey(m_sGroup1, "rate"), getRateSliderValue(1.0)); ControlObject::set(ConfigKey(m_sGroup2, "rate"), getRateSliderValue(1.0)); - track1 = m_pMixerDeck1->loadFakeTrack(false, 140.0); + track1 = m_pMixerDeck1->loadFakeTrack(false, 124.0); ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0); track2 = m_pMixerDeck2->loadFakeTrack(false, 128.0); EXPECT_DOUBLE_EQ( @@ -1225,7 +1225,7 @@ TEST_F(EngineSyncTest, SyncToNonSyncDeck) { ConfigKey(m_sInternalClockGroup, "sync_leader"))); EXPECT_DOUBLE_EQ( 100.0, ControlObject::get(ConfigKey(m_sInternalClockGroup, "bpm"))); - EXPECT_EQ(NULL, m_pEngineSync->getLeader()); + EXPECT_EQ(NULL, m_pEngineSync->getLeaderChannel()); EXPECT_EQ(NULL, m_pEngineSync->getLeaderSyncable()); EXPECT_EQ(static_cast(SyncMode::None), ControlObject::get(ConfigKey(m_sGroup1, "sync_mode"))); @@ -1251,7 +1251,7 @@ TEST_F(EngineSyncTest, SyncToNonSyncDeck) { ConfigKey(m_sInternalClockGroup, "sync_leader"))); EXPECT_DOUBLE_EQ( 100.0, ControlObject::get(ConfigKey(m_sInternalClockGroup, "bpm"))); - EXPECT_EQ(NULL, m_pEngineSync->getLeader()); + EXPECT_EQ(NULL, m_pEngineSync->getLeaderChannel()); EXPECT_EQ(NULL, m_pEngineSync->getLeaderSyncable()); EXPECT_EQ(static_cast(SyncMode::None), ControlObject::get(ConfigKey(m_sGroup2, "sync_mode"))); @@ -1393,7 +1393,7 @@ TEST_F(EngineSyncTest, EjectTrackSyncRemains) { } TEST_F(EngineSyncTest, FileBpmChangesDontAffectLeader) { - // If filebpm changes, don't treat it like a rate change unless it's the leader. + // If filebpm changes, don't treat it like a rate change. mixxx::BeatsPointer pBeats1 = BeatFactory::makeBeatGrid( m_pTrack1->getSampleRate(), mixxx::Bpm(100), mixxx::audio::kStartFramePos); m_pTrack1->trySetBeats(pBeats1); @@ -1417,6 +1417,8 @@ TEST_F(EngineSyncTest, FileBpmChangesDontAffectLeader) { pBeats1 = BeatFactory::makeBeatGrid( m_pTrack1->getSampleRate(), mixxx::Bpm(160), mixxx::audio::kStartFramePos); m_pTrack1->trySetBeats(pBeats1); + EXPECT_DOUBLE_EQ( + 160.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm"))); EXPECT_DOUBLE_EQ( 160.0, ControlObject::get(ConfigKey(m_sInternalClockGroup, "bpm"))); @@ -2365,10 +2367,10 @@ TEST_F(EngineSyncTest, FollowerUserTweakPreservedInLeaderChange) { // 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->getSampleRate(), mixxx::Bpm(130), mixxx::audio::kStartFramePos); m_pTrack1->trySetBeats(pBeats1); mixxx::BeatsPointer pBeats2 = BeatFactory::makeBeatGrid( - m_pTrack2->getSampleRate(), mixxx::Bpm(130), mixxx::audio::kStartFramePos); + m_pTrack2->getSampleRate(), mixxx::Bpm(kDivisibleBpm), mixxx::audio::kStartFramePos); m_pTrack2->trySetBeats(pBeats2); ControlObject::getControl(ConfigKey(m_sGroup1, "sync_leader"))->set(1); @@ -2424,8 +2426,8 @@ TEST_F(EngineSyncTest, LeaderUserTweakPreservedInLeaderChange) { 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::getControl(ConfigKey(m_sGroup1, "sync_leader"))->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); From 0f15a65cd10fa022afbd6323a10e1fe0dc1ad0f2 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Fri, 23 Jul 2021 11:24:53 -0400 Subject: [PATCH 02/12] Update src/test/enginesynctest.cpp Use nullptr instead of NULL (reviewer suggestion) Co-authored-by: Jan Holthuis --- src/test/enginesynctest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index c7b9e6cc536..89cadcf0e67 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -1005,7 +1005,7 @@ TEST_F(EngineSyncTest, LoadTrackInitializesLeader) { pButtonSyncEnabled1->set(1.0); // No leader because this deck has no track. - EXPECT_EQ(NULL, m_pEngineSync->getLeaderChannel()); + EXPECT_EQ(nullptr, m_pEngineSync->getLeaderChannel()); EXPECT_DOUBLE_EQ( 0.0, ControlObject::getControl(ConfigKey(m_sGroup1, "bpm"))->get()); From e506d7933c13aaaf2c3cf41d121769f941022f08 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Fri, 23 Jul 2021 11:25:26 -0400 Subject: [PATCH 03/12] Update src/engine/sync/synccontrol.cpp constify bool (reviewer suggestion) Co-authored-by: Jan Holthuis --- src/engine/sync/synccontrol.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index 1536f5b3a9f..a89c176e976 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -343,7 +343,7 @@ void SyncControl::trackLoaded(TrackPointer pNewTrack) { return; } - bool hadBeats = m_pBeats != nullptr; + const bool hadBeats = m_pBeats != nullptr; m_pBeats = pBeats; m_leaderBpmAdjustFactor = kBpmUnity; From 7ea1c8ca38cc2715d06a00264e7f9e553149c79f Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Wed, 28 Jul 2021 08:48:36 -0400 Subject: [PATCH 04/12] Update src/test/enginesynctest.cpp review note: NULL -> nullptr Co-authored-by: Jan Holthuis --- src/test/enginesynctest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index 89cadcf0e67..4fba485bd8b 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -1225,7 +1225,7 @@ TEST_F(EngineSyncTest, SyncToNonSyncDeck) { ConfigKey(m_sInternalClockGroup, "sync_leader"))); EXPECT_DOUBLE_EQ( 100.0, ControlObject::get(ConfigKey(m_sInternalClockGroup, "bpm"))); - EXPECT_EQ(NULL, m_pEngineSync->getLeaderChannel()); + EXPECT_EQ(nullptr, m_pEngineSync->getLeaderChannel()); EXPECT_EQ(NULL, m_pEngineSync->getLeaderSyncable()); EXPECT_EQ(static_cast(SyncMode::None), ControlObject::get(ConfigKey(m_sGroup1, "sync_mode"))); From 1410ff36a7313ceede05e65aa78417a387248ec4 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Wed, 28 Jul 2021 08:48:43 -0400 Subject: [PATCH 05/12] Update src/test/enginesynctest.cpp review note: NULL -> nullptr Co-authored-by: Jan Holthuis --- src/test/enginesynctest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index 4fba485bd8b..b8e5384421e 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -1251,7 +1251,7 @@ TEST_F(EngineSyncTest, SyncToNonSyncDeck) { ConfigKey(m_sInternalClockGroup, "sync_leader"))); EXPECT_DOUBLE_EQ( 100.0, ControlObject::get(ConfigKey(m_sInternalClockGroup, "bpm"))); - EXPECT_EQ(NULL, m_pEngineSync->getLeaderChannel()); + EXPECT_EQ(nullptr, m_pEngineSync->getLeaderChannel()); EXPECT_EQ(NULL, m_pEngineSync->getLeaderSyncable()); EXPECT_EQ(static_cast(SyncMode::None), ControlObject::get(ConfigKey(m_sGroup2, "sync_mode"))); From f76defea50901ee98177a67b5b1a925433803b6b Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Wed, 28 Jul 2021 11:49:05 -0400 Subject: [PATCH 06/12] Sync Lock: correct naming in enginemaster --- src/engine/enginemaster.cpp | 12 ++++++------ src/engine/enginemaster.h | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/engine/enginemaster.cpp b/src/engine/enginemaster.cpp index 0acbaf0e433..d86a908dce1 100644 --- a/src/engine/enginemaster.cpp +++ b/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_iSampleRate, m_iBufferSize); + m_pEngineSync->onCallbackStart(m_iSampleRate, 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->getLeaderChannel(); + 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_iSampleRate, m_iBufferSize); + m_pEngineSync->onCallbackEnd(m_iSampleRate, m_iBufferSize); // After all the engines have been processed, trigger post-processing // which ensures that all channels are updating certain values at the diff --git a/src/engine/enginemaster.h b/src/engine/enginemaster.h index f85af0b11c1..2c0a050da49 100644 --- a/src/engine/enginemaster.h +++ b/src/engine/enginemaster.h @@ -89,7 +89,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. @@ -295,7 +295,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; From 719bd084277003e06f10917314ce3012ddf4711d Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Sun, 22 Aug 2021 13:20:20 -0400 Subject: [PATCH 07/12] Sync Lock: remove obsolete and broken BpmControler::syncTempo function --- src/engine/controls/bpmcontrol.cpp | 94 +++++------------------------- src/engine/controls/bpmcontrol.h | 2 +- src/engine/sync/enginesync.cpp | 3 +- src/engine/sync/synccontrol.cpp | 14 +++-- src/test/enginesynctest.cpp | 12 ++-- 5 files changed, 31 insertions(+), 94 deletions(-) diff --git a/src/engine/controls/bpmcontrol.cpp b/src/engine/controls/bpmcontrol.cpp index fa9159666d5..bd417ac439f 100644 --- a/src/engine/controls/bpmcontrol.cpp +++ b/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,75 +297,6 @@ void BpmControl::slotControlBeatSync(double value) { } } -bool BpmControl::syncTempo() { - 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) { @@ -1030,14 +955,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(); diff --git a/src/engine/controls/bpmcontrol.h b/src/engine/controls/bpmcontrol.h index f29505db862..d459ec6de2f 100644 --- a/src/engine/controls/bpmcontrol.h +++ b/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 m_dUserOffset; QAtomicInt m_resetSyncAdjustment; ControlProxy* m_pSyncMode; + ControlProxy* m_pSyncEnabled; TapFilter m_tapFilter; // threadsafe diff --git a/src/engine/sync/enginesync.cpp b/src/engine/sync/enginesync.cpp index 90c5c05289c..3bcd6c5c267 100644 --- a/src/engine/sync/enginesync.cpp +++ b/src/engine/sync/enginesync.cpp @@ -429,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; diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index f3f4f8623e7..89e61673b3a 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/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); } @@ -343,7 +346,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::trackLoaded"; + kLogger.trace() << getGroup() << "SyncControl::trackBeatsUpdated"; } VERIFY_OR_DEBUG_ASSERT(m_pLocalBpm) { @@ -367,7 +370,8 @@ void SyncControl::trackLoaded(TrackPointer pNewTrack) { return; } - m_pBpmControl->syncTempo(); + // 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. @@ -379,8 +383,8 @@ void SyncControl::trackLoaded(TrackPointer pNewTrack) { void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) { // This slot is fired by if the user has adjusted the beatgrid. - if (kLogger.traceEnabled()) { - kLogger.trace() << getGroup() << "SyncControl::trackBeatsUpdated"; + if (true) { + qDebug() << getGroup() << "SyncControl::trackBeatsUpdated"; } m_pBeats = pBeats; diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index 23bb774a1d3..440fcce1425 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -1100,20 +1100,20 @@ TEST_F(EngineSyncTest, LoadTrackResetTempoOption) { EXPECT_DOUBLE_EQ(140.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm"))); EXPECT_DOUBLE_EQ(140.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm"))); - // Repeat with RESET_NONE + // Even when RESET_NONE is on, sync lock is more important -- do not change + // the speed of the playing deck. EXPECT_TRUE(isSoftLeader(m_sGroup1)); m_pConfig->set(ConfigKey("[Controls]", "SpeedAutoReset"), ConfigValue(BaseTrackPlayer::RESET_NONE)); ControlObject::set(ConfigKey(m_sGroup1, "play"), 0.0); - ControlObject::set(ConfigKey(m_sGroup1, "rate"), getRateSliderValue(1.0)); - ControlObject::set(ConfigKey(m_sGroup2, "rate"), getRateSliderValue(1.0)); track1 = m_pMixerDeck1->loadFakeTrack(false, 124.0); + ControlObject::set(ConfigKey(m_sGroup1, "rate"), getRateSliderValue(1.0)); ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0); track2 = m_pMixerDeck2->loadFakeTrack(false, 128.0); EXPECT_DOUBLE_EQ( - 128.0, ControlObject::get(ConfigKey(m_sInternalClockGroup, "bpm"))); - EXPECT_DOUBLE_EQ(128.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm"))); - EXPECT_DOUBLE_EQ(128.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm"))); + 124.0, ControlObject::get(ConfigKey(m_sInternalClockGroup, "bpm"))); + EXPECT_DOUBLE_EQ(124.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm"))); + EXPECT_DOUBLE_EQ(124.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm"))); // Load two tracks with sync off and RESET_SPEED m_pConfig->set(ConfigKey("[Controls]", "SpeedAutoReset"), From b8361481eed2006edc23d97f9cd2a275edc1d590 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Sun, 12 Sep 2021 17:18:30 -0400 Subject: [PATCH 08/12] Remove processbuffer calls to keep positions at zero --- src/test/enginesynctest.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index 558006dad46..ce3d1f0c175 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -2860,13 +2860,8 @@ TEST_F(EngineSyncTest, BeatMapQuantizePlay) { ->set(static_cast(SyncMode::Follower)); ControlObject::getControl(ConfigKey(m_sGroup1, "play"))->set(1.0); - - ProcessBuffer(); - ControlObject::getControl(ConfigKey(m_sGroup2, "play"))->set(1.0); - ProcessBuffer(); - // Beat Distance shall be still 0, because we are before the first beat. // This was fixed in https://bugs.launchpad.net/mixxx/+bug/1920084 EXPECT_DOUBLE_EQ(m_pChannel2->getEngineBuffer()->m_pSyncControl->getBeatDistance(), 0); From 1a31030905bd6560dfd870e8552ee237f3e8cc35 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Sat, 25 Sep 2021 14:46:42 -0400 Subject: [PATCH 09/12] Sync Lock: move obsolete beatsync_* controls to synccontrol --- src/engine/controls/bpmcontrol.cpp | 51 +---------------------------- src/engine/controls/bpmcontrol.h | 8 ----- src/engine/sync/synccontrol.cpp | 52 +++++++++++++++++++++++++++++- src/engine/sync/synccontrol.h | 15 +++++++-- 4 files changed, 64 insertions(+), 62 deletions(-) diff --git a/src/engine/controls/bpmcontrol.cpp b/src/engine/controls/bpmcontrol.cpp index 2ab56db0aea..3a9e8a8214b 100644 --- a/src/engine/controls/bpmcontrol.cpp +++ b/src/engine/controls/bpmcontrol.cpp @@ -111,22 +111,6 @@ BpmControl::BpmControl(const QString& group, this, &BpmControl::slotBpmTap, Qt::DirectConnection); - // Beat sync (scale buffer tempo relative to tempo of other buffer) - m_pButtonSync = new ControlPushButton(ConfigKey(group, "beatsync")); - connect(m_pButtonSync, &ControlObject::valueChanged, - this, &BpmControl::slotControlBeatSync, - Qt::DirectConnection); - - m_pButtonSyncPhase = new ControlPushButton(ConfigKey(group, "beatsync_phase")); - connect(m_pButtonSyncPhase, &ControlObject::valueChanged, - this, &BpmControl::slotControlBeatSyncPhase, - Qt::DirectConnection); - - m_pButtonSyncTempo = new ControlPushButton(ConfigKey(group, "beatsync_tempo")); - connect(m_pButtonSyncTempo, &ControlObject::valueChanged, - this, &BpmControl::slotControlBeatSyncTempo, - Qt::DirectConnection); - m_pTranslateBeats = new ControlPushButton(ConfigKey(group, "beats_translate_curpos")); connect(m_pTranslateBeats, &ControlObject::valueChanged, this, &BpmControl::slotBeatsTranslate, @@ -151,9 +135,6 @@ BpmControl::~BpmControl() { delete m_pLocalBpm; delete m_pEngineBpm; delete m_pButtonTap; - delete m_pButtonSync; - delete m_pButtonSyncPhase; - delete m_pButtonSyncTempo; delete m_pTranslateBeats; delete m_pBeatsTranslateMatchAlignment; delete m_pTranslateBeatsEarlier; @@ -267,36 +248,6 @@ void BpmControl::slotTapFilter(double averageLength, int numSamples) { pTrack->trySetBeats(pBeats->setBpm(averageBpm)); } -void BpmControl::slotControlBeatSyncPhase(double value) { - if (value == 0) { - return; - } - - if (isSynchronized()) { - m_dUserOffset.setValue(0.0); - } - getEngineBuffer()->requestSyncPhase(); -} - -void BpmControl::slotControlBeatSyncTempo(double value) { - m_pSyncEnabled->set(value != 0); -} - -void BpmControl::slotControlBeatSync(double value) { - m_pSyncEnabled->set(value != 0); - - if (value == 0) { - return; - } - - // Also sync phase if quantize is enabled. - // this is used from controller scripts, where the latching behaviour of - // the sync_enable CO cannot be used - if (m_pPlayButton->toBool() && m_pQuantize->toBool()) { - slotControlBeatSyncPhase(value); - } -} - // static double BpmControl::shortestPercentageChange(const double& current_percentage, const double& target_percentage) { @@ -953,7 +904,7 @@ mixxx::audio::FrameDiff_t BpmControl::getPhaseOffset(mixxx::audio::FramePos this } void BpmControl::slotUpdateEngineBpm(double value) { - // Adjust playback bpm in response to a rate_ration update + // Adjust playback bpm in response to a rate_ratio update double dRate = m_pRateRatio->get(); if (kLogger.traceEnabled()) { diff --git a/src/engine/controls/bpmcontrol.h b/src/engine/controls/bpmcontrol.h index d459ec6de2f..0cf8cd5ac88 100644 --- a/src/engine/controls/bpmcontrol.h +++ b/src/engine/controls/bpmcontrol.h @@ -98,9 +98,6 @@ class BpmControl : public EngineControl { void slotAdjustBeatsSlower(double); void slotTranslateBeatsEarlier(double); void slotTranslateBeatsLater(double); - void slotControlBeatSync(double); - void slotControlBeatSyncPhase(double); - void slotControlBeatSyncTempo(double); void slotTapFilter(double,int); void slotBpmTap(double); void slotUpdateRateSlider(double v = 0.0); @@ -149,11 +146,6 @@ class BpmControl : public EngineControl { // Used for bpm tapping from GUI and MIDI ControlPushButton* m_pButtonTap; - // Button for sync'ing with the other EngineBuffer - ControlPushButton* m_pButtonSync; - ControlPushButton* m_pButtonSyncPhase; - ControlPushButton* m_pButtonSyncTempo; - // Button that translates the beats so the nearest beat is on the current // playposition. ControlPushButton* m_pTranslateBeats; diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index 89e61673b3a..1536a59be0e 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -25,7 +25,7 @@ const mixxx::Logger kLogger("SyncControl"); SyncControl::SyncControl(const QString& group, UserSettingsPointer pConfig, EngineChannel* pChannel, - SyncableListener* pEngineSync) + EngineSync* pEngineSync) : EngineControl(group, pConfig), m_sGroup(group), m_pChannel(pChannel), @@ -67,6 +67,22 @@ SyncControl::SyncControl(const QString& group, m_pSyncEnabled->connectValueChangeRequest( this, &SyncControl::slotSyncEnabledChangeRequest, Qt::DirectConnection); + // Beat sync (scale buffer tempo relative to tempo of other buffer) + m_pButtonSync = new ControlPushButton(ConfigKey(group, "beatsync")); + connect(m_pButtonSync, &ControlObject::valueChanged, + this, &SyncControl::slotControlBeatSync, + Qt::DirectConnection); + + m_pButtonSyncPhase = new ControlPushButton(ConfigKey(group, "beatsync_phase")); + connect(m_pButtonSyncPhase, &ControlObject::valueChanged, + this, &SyncControl::slotControlBeatSyncPhase, + Qt::DirectConnection); + + m_pButtonSyncTempo = new ControlPushButton(ConfigKey(group, "beatsync_tempo")); + connect(m_pButtonSyncTempo, &ControlObject::valueChanged, + this, &SyncControl::slotControlBeatSyncTempo, + Qt::DirectConnection); + // The relative position between two beats in the range 0.0 ... 1.0 m_pBeatDistance.reset( new ControlObject(ConfigKey(group, "beat_distance"))); @@ -81,6 +97,9 @@ SyncControl::SyncControl(const QString& group, } SyncControl::~SyncControl() { + delete m_pButtonSync; + delete m_pButtonSyncPhase; + delete m_pButtonSyncTempo; } void SyncControl::setEngineControls(RateControl* pRateControl, @@ -410,6 +429,37 @@ void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) { } } +void SyncControl::slotControlBeatSyncPhase(double value) { + if (value == 0) { + return; + } + + m_pChannel->getEngineBuffer()->requestSyncPhase(); +} + +void SyncControl::slotControlBeatSyncTempo(double value) { + if (value == 0) { + return; + } + // This request is a noop if we are already synced. + const auto localBpm = getLocalBpm(); + if (isSynchronized() || !localBpm.isValid()) { + return; + } + + Syncable* target = m_pEngineSync->pickNonSyncSyncTarget(getChannel()); + if (target == nullptr) { + return; + } + + double multiplier = determineBpmMultiplier(fileBpm(), target->getBaseBpm()); + m_pRateRatio->set(target->getBpm() * multiplier / localBpm); +} + +void SyncControl::slotControlBeatSync(double value) { + m_pSyncEnabled->setAndConfirm(value > 0); +} + void SyncControl::slotControlPlay(double play) { if (kLogger.traceEnabled()) { kLogger.trace() << "SyncControl::slotControlPlay" << getSyncMode() << play; diff --git a/src/engine/sync/synccontrol.h b/src/engine/sync/synccontrol.h index c9f2e170f7b..de1130095ea 100644 --- a/src/engine/sync/synccontrol.h +++ b/src/engine/sync/synccontrol.h @@ -4,7 +4,7 @@ #include #include "engine/controls/enginecontrol.h" -#include "engine/sync/syncable.h" +#include "engine/sync/enginesync.h" class EngineChannel; class BpmControl; @@ -21,7 +21,7 @@ class SyncControl : public EngineControl, public Syncable { static const double kBpmHalve; static const double kBpmDouble; SyncControl(const QString& group, UserSettingsPointer pConfig, - EngineChannel* pChannel, SyncableListener* pEngineSync); + EngineChannel* pChannel, EngineSync* pEngineSync); ~SyncControl() override; const QString& getGroup() const override { return m_sGroup; } @@ -67,6 +67,10 @@ class SyncControl : public EngineControl, public Syncable { void trackBeatsUpdated(mixxx::BeatsPointer pBeats) override; private slots: + void slotControlBeatSync(double); + void slotControlBeatSyncPhase(double); + void slotControlBeatSyncTempo(double); + // Fired by changes in play. void slotControlPlay(double v); @@ -99,7 +103,7 @@ class SyncControl : public EngineControl, public Syncable { // EngineSync can ask us what our EngineChannel is. EngineMaster in turn // asks EngineSync what EngineChannel is the "leader" channel. EngineChannel* m_pChannel; - SyncableListener* m_pEngineSync; + EngineSync* m_pEngineSync; BpmControl* m_pBpmControl; RateControl* m_pRateControl; bool m_bOldScratching; @@ -122,6 +126,11 @@ class SyncControl : public EngineControl, public Syncable { QScopedPointer m_pSyncEnabled; QScopedPointer m_pBeatDistance; + // Button for sync'ing with the other EngineBuffer + ControlPushButton* m_pButtonSync; + ControlPushButton* m_pButtonSyncPhase; + ControlPushButton* m_pButtonSyncTempo; + // These ControlProxys are created as parent to this and deleted by // the Qt object tree. This helps that they are deleted by the creating // thread, which is required to avoid segfaults. From 90132fdd2fafadbd768bc71c4ddead2ed3af1fa2 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Sat, 25 Sep 2021 15:31:38 -0400 Subject: [PATCH 10/12] Revert debug edit --- src/engine/sync/synccontrol.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index 1536a59be0e..dd7b2d415f8 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -402,8 +402,8 @@ void SyncControl::trackLoaded(TrackPointer pNewTrack) { void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) { // This slot is fired by if the user has adjusted the beatgrid. - if (true) { - qDebug() << getGroup() << "SyncControl::trackBeatsUpdated"; + if (kLogger.traceEnabled()) { + kLogger.trace() << getGroup() << "SyncControl::trackBeatsUpdated"; } m_pBeats = pBeats; From 665c03d95529ad8e75610abc1785bbeb60b32209 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Wed, 6 Oct 2021 20:05:01 -0400 Subject: [PATCH 11/12] Sync Lock: make sure beatsync control is momentary --- src/engine/sync/synccontrol.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index dd7b2d415f8..76077a43818 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -457,7 +457,10 @@ void SyncControl::slotControlBeatSyncTempo(double value) { } void SyncControl::slotControlBeatSync(double value) { - m_pSyncEnabled->setAndConfirm(value > 0); + if (value > 0) { + m_pChannel->getEngineBuffer()->requestEnableSync(true); + m_pChannel->getEngineBuffer()->requestEnableSync(false); + } } void SyncControl::slotControlPlay(double play) { From 5d6dbc207dfbd23133bd6d52507bb262865de947 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Wed, 6 Oct 2021 20:13:46 -0400 Subject: [PATCH 12/12] SyncLock: clang-format files --- src/engine/sync/synccontrol.cpp | 35 +++++++++++++++++++-------------- src/engine/sync/synccontrol.h | 17 +++++++++++----- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index 76077a43818..ec0bd4100ee 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -43,8 +43,7 @@ SyncControl::SyncControl(const QString& group, // Play button. We only listen to this to disable leader if the deck is // stopped. m_pPlayButton = new ControlProxy(group, "play", this); - m_pPlayButton->connectValueChanged(this, &SyncControl::slotControlPlay, - Qt::DirectConnection); + m_pPlayButton->connectValueChanged(this, &SyncControl::slotControlPlay, Qt::DirectConnection); m_pSyncMode.reset(new ControlPushButton(ConfigKey(group, "sync_mode"))); m_pSyncMode->setButtonMode(ControlPushButton::TOGGLE); @@ -69,18 +68,24 @@ SyncControl::SyncControl(const QString& group, // Beat sync (scale buffer tempo relative to tempo of other buffer) m_pButtonSync = new ControlPushButton(ConfigKey(group, "beatsync")); - connect(m_pButtonSync, &ControlObject::valueChanged, - this, &SyncControl::slotControlBeatSync, + connect(m_pButtonSync, + &ControlObject::valueChanged, + this, + &SyncControl::slotControlBeatSync, Qt::DirectConnection); m_pButtonSyncPhase = new ControlPushButton(ConfigKey(group, "beatsync_phase")); - connect(m_pButtonSyncPhase, &ControlObject::valueChanged, - this, &SyncControl::slotControlBeatSyncPhase, + connect(m_pButtonSyncPhase, + &ControlObject::valueChanged, + this, + &SyncControl::slotControlBeatSyncPhase, Qt::DirectConnection); m_pButtonSyncTempo = new ControlPushButton(ConfigKey(group, "beatsync_tempo")); - connect(m_pButtonSyncTempo, &ControlObject::valueChanged, - this, &SyncControl::slotControlBeatSyncTempo, + connect(m_pButtonSyncTempo, + &ControlObject::valueChanged, + this, + &SyncControl::slotControlBeatSyncTempo, Qt::DirectConnection); // The relative position between two beats in the range 0.0 ... 1.0 @@ -89,7 +94,8 @@ SyncControl::SyncControl(const QString& group, m_pPassthroughEnabled = new ControlProxy(group, "passthrough", this); m_pPassthroughEnabled->connectValueChanged(this, - &SyncControl::slotPassthroughChanged, Qt::DirectConnection); + &SyncControl::slotPassthroughChanged, + Qt::DirectConnection); m_pQuantize = new ControlProxy(group, "quantize", this); @@ -103,7 +109,7 @@ SyncControl::~SyncControl() { } void SyncControl::setEngineControls(RateControl* pRateControl, - BpmControl* pBpmControl) { + BpmControl* pBpmControl) { m_pRateControl = pRateControl; m_pBpmControl = pBpmControl; @@ -115,8 +121,7 @@ void SyncControl::setEngineControls(RateControl* pRateControl, m_pLocalBpm = new ControlProxy(getGroup(), "local_bpm", this); m_pRateRatio = new ControlProxy(getGroup(), "rate_ratio", this); - m_pRateRatio->connectValueChanged(this, &SyncControl::slotRateChanged, - Qt::DirectConnection); + m_pRateRatio->connectValueChanged(this, &SyncControl::slotRateChanged, Qt::DirectConnection); m_pSyncPhaseButton = new ControlProxy(getGroup(), "beatsync_phase", this); @@ -128,8 +133,8 @@ void SyncControl::setEngineControls(RateControl* pRateControl, // control doesn't exist yet. This will blow up immediately, won't go unnoticed. DEBUG_ASSERT(m_pVCEnabled->valid()); - m_pVCEnabled->connectValueChanged(this, &SyncControl::slotVinylControlChanged, - Qt::DirectConnection); + m_pVCEnabled->connectValueChanged( + this, &SyncControl::slotVinylControlChanged, Qt::DirectConnection); #endif } @@ -452,7 +457,7 @@ void SyncControl::slotControlBeatSyncTempo(double value) { return; } - double multiplier = determineBpmMultiplier(fileBpm(), target->getBaseBpm()); + double multiplier = determineBpmMultiplier(fileBpm(), target->getBaseBpm()); m_pRateRatio->set(target->getBpm() * multiplier / localBpm); } diff --git a/src/engine/sync/synccontrol.h b/src/engine/sync/synccontrol.h index de1130095ea..3ea97fde856 100644 --- a/src/engine/sync/synccontrol.h +++ b/src/engine/sync/synccontrol.h @@ -1,8 +1,9 @@ #pragma once -#include #include +#include + #include "engine/controls/enginecontrol.h" #include "engine/sync/enginesync.h" @@ -20,12 +21,18 @@ class SyncControl : public EngineControl, public Syncable { static const double kBpmUnity; static const double kBpmHalve; static const double kBpmDouble; - SyncControl(const QString& group, UserSettingsPointer pConfig, - EngineChannel* pChannel, EngineSync* pEngineSync); + SyncControl(const QString& group, + UserSettingsPointer pConfig, + EngineChannel* pChannel, + EngineSync* pEngineSync); ~SyncControl() override; - const QString& getGroup() const override { return m_sGroup; } - EngineChannel* getChannel() const override { return m_pChannel; } + const QString& getGroup() const override { + return m_sGroup; + } + EngineChannel* getChannel() const override { + return m_pChannel; + } mixxx::Bpm getBpm() const override; SyncMode getSyncMode() const override;