Sync Lock: make sure to reinit leader params after scratching ends

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;

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);

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();
     }
 

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) {

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,

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;

src/engine/sync/syncable.h

@@ -3,6 +3,7 @@
 #include <QDebug>
 #include <QString>
 
+#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(

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;

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);

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(