From 50e16c96face8890d7f912385a9ba59f26953ed2 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Wed, 27 Mar 2024 22:28:01 +0100 Subject: [PATCH] Waveform/Spinny scratching: fix loop wrap-around issue PositionScratchController::process() now knows if the play position was wrapped around (and how often) since the last processing. This avoids an infinite wrap-around 'loop', even after the mouse didn't move. Previously, PositionScratchController didn't know if the play position was wrapped around, so it interpreted the raw (huge) sample delta as play pos move and calculated a huge rate, which was then applied by the engine, resulting in a (now true) long sample distance in the next run, which caused another loop wrap-around, and so on... --- src/engine/controls/ratecontrol.cpp | 27 ++++++++++++++++++++- src/engine/controls/ratecontrol.h | 9 +++++++ src/engine/positionscratchcontroller.cpp | 31 +++++++++++++++++++++--- src/engine/positionscratchcontroller.h | 8 +++++- src/engine/readaheadmanager.cpp | 4 +++ 5 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/engine/controls/ratecontrol.cpp b/src/engine/controls/ratecontrol.cpp index 70b61698c4a..8e1c065ccda 100644 --- a/src/engine/controls/ratecontrol.cpp +++ b/src/engine/controls/ratecontrol.cpp @@ -34,6 +34,9 @@ RateControl::RateControl(const QString& group, UserSettingsPointer pConfig) : EngineControl(group, pConfig), m_pBpmControl(nullptr), + m_wrapAroundCount(0), + m_jumpPos(mixxx::audio::FramePos()), + m_targetPos(mixxx::audio::FramePos()), m_bTempStarted(false), m_tempRateRatio(0.0), m_dRateTempRampChange(0.0) { @@ -459,7 +462,17 @@ double RateControl::calculateSpeed(double baserate, double speed, bool paused, } double currentSample = frameInfo().currentPosition.toEngineSamplePos(); - m_pScratchController->process(currentSample, rate, iSamplesPerBuffer, baserate); + // Let PositionScratchController also know if the play pos wrapped around + // (beatloop or track repeat) so it can correctly interpret the sample position delta. + m_pScratchController->process(currentSample, + rate, + iSamplesPerBuffer, + baserate, + m_wrapAroundCount, + m_jumpPos, + m_targetPos); + // Reset count after use. + m_wrapAroundCount = 0; // If waveform scratch is enabled, override all other controls if (m_pScratchController->isEnabled()) { @@ -603,3 +616,15 @@ bool RateControl::isReverseButtonPressed() { } return false; } + +void RateControl::notifyWrapAround(mixxx::audio::FramePos triggerPos, + mixxx::audio::FramePos targetPos) { + VERIFY_OR_DEBUG_ASSERT(triggerPos.isValid() && targetPos.isValid()) { + m_wrapAroundCount = 0; + // no need to reset the position, they're not used if count is 0. + return; + } + m_wrapAroundCount++; + m_jumpPos = triggerPos; + m_targetPos = targetPos; +} diff --git a/src/engine/controls/ratecontrol.h b/src/engine/controls/ratecontrol.h index f3c553e916b..951772f8406 100644 --- a/src/engine/controls/ratecontrol.h +++ b/src/engine/controls/ratecontrol.h @@ -71,6 +71,11 @@ class RateControl : public EngineControl { static void setRateRampSensitivity(int); static int getRateRampSensitivity(); bool isReverseButtonPressed(); + // ReadAheadManager::getNextSamples() notifies us each time the play position + // wrapped around during one buffer process (beatloop or track repeat) so + // PositionScratchController can correctly interpret the sample position delta. + void notifyWrapAround(mixxx::audio::FramePos triggerPos, + mixxx::audio::FramePos targetPos); public slots: void slotRateRangeChanged(double); @@ -147,6 +152,10 @@ public slots: ControlProxy* m_pSyncMode; ControlProxy* m_pSlipEnabled; + int m_wrapAroundCount; + mixxx::audio::FramePos m_jumpPos; + mixxx::audio::FramePos m_targetPos; + // This is true if we've already started to ramp the rate bool m_bTempStarted; // Set the Temporary Rate Change Mode diff --git a/src/engine/positionscratchcontroller.cpp b/src/engine/positionscratchcontroller.cpp index 59d0d649d2f..d4540b7fabc 100644 --- a/src/engine/positionscratchcontroller.cpp +++ b/src/engine/positionscratchcontroller.cpp @@ -93,7 +93,10 @@ PositionScratchController::~PositionScratchController() { void PositionScratchController::process(double currentSamplePos, double releaseRate, int iBufferSize, - double baseSampleRate) { + double baseSampleRate, + int wrappedAround, + mixxx::audio::FramePos trigger, + mixxx::audio::FramePos target) { bool scratchEnable = m_pScratchEnable->get() != 0; if (!m_isScratching && !scratchEnable) { @@ -171,11 +174,33 @@ void PositionScratchController::process(double currentSamplePos, // sure. m_inertiaEnabled = false; + double sampleDelta = 0.0; + if (wrappedAround > 0) { + // If we wrapped around calculate the virtual position like if + // we are not looping, i.e. sum up diffs from loop start/end and + // loop length for each wrap-aound (necessary if the buffer is + // longer than the loop, e.g. when looping at high rates / with short loops. + // This avoids high rate and infinite wrap-around scratching + // even after mouse was stopped. + double triggerPos = trigger.toEngineSamplePos(); + double targetPos = target.toEngineSamplePos(); + bool reverse = triggerPos < targetPos; + double loopLength = reverse ? -1 * (targetPos - triggerPos) + : triggerPos - targetPos; + if (wrappedAround > 2) { + sampleDelta = (wrappedAround - 1) * loopLength; + } + sampleDelta += + (triggerPos - m_prevSamplePos) + + (currentSamplePos - targetPos); + } else { + sampleDelta = currentSamplePos - m_prevSamplePos; + } + // Measure the total distance traveled since last frame and add // it to the running total. This is required to scratch within loop // boundaries. And normalize to one buffer - m_samplePosDeltaSum += (currentSamplePos - m_prevSamplePos) / - (iBufferSize * baseSampleRate); + m_samplePosDeltaSum += (sampleDelta) / (iBufferSize * baseSampleRate); // Continue with the last rate if we do not have a new // Mouse position diff --git a/src/engine/positionscratchcontroller.h b/src/engine/positionscratchcontroller.h index 1eb504e9495..9a15e1f7f4a 100644 --- a/src/engine/positionscratchcontroller.h +++ b/src/engine/positionscratchcontroller.h @@ -15,7 +15,13 @@ class PositionScratchController : public QObject { PositionScratchController(const QString& group); virtual ~PositionScratchController(); - void process(double currentSample, double releaseRate, int iBufferSize, double baseSampleRate); + void process(double currentSample, + double releaseRate, + int iBufferSize, + double baseSampleRate, + int wrappedAround, + mixxx::audio::FramePos trigger, + mixxx::audio::FramePos target); bool isEnabled(); double getRate(); void notifySeek(mixxx::audio::FramePos position); diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index 6af6981f566..09a8dc0030a 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -117,6 +117,10 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, // Activate on this trigger if necessary if (reachedTrigger) { DEBUG_ASSERT(target != kNoTrigger); + if (m_pRateControl) { + m_pRateControl->notifyWrapAround(loopTriggerPosition, targetPosition); + } + // TODO probably also useful for hotcue_X_indicator in CueControl::updateIndicators() // Jump to other end of loop or track. m_currentPosition = target;