From feae5ec9e4fcdad6894ce5f18545944ef9a8c788 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Mon, 14 Jun 2021 14:09:03 -0400 Subject: [PATCH] Sync Lock: make sure to reinit leader params after scratching ends --- src/engine/enginebuffer.cpp | 20 +--------- src/engine/sync/enginesync.cpp | 29 ++++++++++++-- src/engine/sync/internalclock.h | 3 ++ src/engine/sync/syncable.h | 1 + src/engine/sync/synccontrol.cpp | 4 ++ src/engine/sync/synccontrol.h | 1 + src/test/enginesynctest.cpp | 69 +++++++++++++++++++++++++++++++++ 7 files changed, 105 insertions(+), 22 deletions(-) diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index a05819759d59..b08a8be140c2 100644 --- a/src/engine/enginebuffer.cpp +++ b/src/engine/enginebuffer.cpp @@ -943,18 +943,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 @@ -1168,12 +1156,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; } @@ -1322,11 +1304,11 @@ void EngineBuffer::postProcess(const int iBufferSize) { m_pSyncControl->setLocalBpm(localBpmValue); 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 master, 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 f0dead04138d..9039f9aac763 100644 --- a/src/engine/sync/enginesync.cpp +++ b/src/engine/sync/enginesync.cpp @@ -377,9 +377,32 @@ 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; + } + qDebug() << "hello? EngineSync::notifyScratching" << pSyncable->getGroup() + << pSyncable->getSyncMode(); + 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::notifyBaseBpmChanged(Syncable* pSyncable, double bpm) { diff --git a/src/engine/sync/internalclock.h b/src/engine/sync/internalclock.h index d8253899d36e..f200a0f35d2b 100644 --- a/src/engine/sync/internalclock.h +++ b/src/engine/sync/internalclock.h @@ -46,6 +46,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 65d43d3135db..a466498f1571 100644 --- a/src/engine/sync/syncable.h +++ b/src/engine/sync/syncable.h @@ -110,6 +110,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. diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index 74e98d9262e1..64f5b176a605 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -177,6 +177,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 diff --git a/src/engine/sync/synccontrol.h b/src/engine/sync/synccontrol.h index d0856f6c166c..5f7ba1c6eb25 100644 --- a/src/engine/sync/synccontrol.h +++ b/src/engine/sync/synccontrol.h @@ -34,6 +34,7 @@ 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; diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index 8a96a833e289..01922ea3ae98 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -2646,6 +2646,75 @@ TEST_F(EngineSyncTest, SeekStayInPhase) { EXPECT_DOUBLE_EQ(0.18925937554508981, ControlObject::get(ConfigKey(m_sGroup1, "playposition"))); } +TEST_F(EngineSyncTest, ScratchEndOtherPlayingTrackStayInPhase) { + // After scratching, confirm that we are still in phase. + // This version tests with a stopped other track. + mixxx::BeatsPointer pBeats1 = + BeatFactory::makeBeatGrid(m_pTrack1->getSampleRate(), + mixxx::Bpm(130), + mixxx::audio::kStartFramePos); + m_pTrack1->trySetBeats(pBeats1); + ControlObject::set(ConfigKey(m_sGroup1, "quantize"), 1.0); + ControlObject::set(ConfigKey(m_sGroup1, "sync_enabled"), 1); + + mixxx::BeatsPointer pBeats2 = + BeatFactory::makeBeatGrid(m_pTrack2->getSampleRate(), + mixxx::Bpm(125), + mixxx::audio::kStartFramePos); + 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, ScratchEndOtherStoppedTrackStayInPhase) { + // After scratching, confirm that we are still in phase. + // This version tests with a playing other track. + mixxx::BeatsPointer pBeats1 = + BeatFactory::makeBeatGrid(m_pTrack1->getSampleRate(), + mixxx::Bpm(130), + mixxx::audio::kStartFramePos); + m_pTrack1->trySetBeats(pBeats1); + ControlObject::set(ConfigKey(m_sGroup1, "quantize"), 1.0); + ControlObject::set(ConfigKey(m_sGroup1, "sync_enabled"), 1); + + mixxx::BeatsPointer pBeats2 = + BeatFactory::makeBeatGrid(m_pTrack2->getSampleRate(), + mixxx::Bpm(125), + mixxx::audio::kStartFramePos); + 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 = BeatFactory::makeBeatGrid(