From 6e12acf2104bb1a8e5c47b1f40bd09347097bb0b Mon Sep 17 00:00:00 2001 From: ronso0 Date: Tue, 26 Mar 2024 14:06:28 +0100 Subject: [PATCH 1/2] PositionScratchController: improve var names, remove prefixes --- src/engine/controls/ratecontrol.cpp | 6 +- src/engine/enginebuffer.cpp | 20 +-- src/engine/positionscratchcontroller.cpp | 147 +++++++++++------------ src/engine/positionscratchcontroller.h | 25 ++-- 4 files changed, 97 insertions(+), 101 deletions(-) diff --git a/src/engine/controls/ratecontrol.cpp b/src/engine/controls/ratecontrol.cpp index 455e84d15a3..70b61698c4a 100644 --- a/src/engine/controls/ratecontrol.cpp +++ b/src/engine/controls/ratecontrol.cpp @@ -441,10 +441,10 @@ double RateControl::calculateSpeed(double baserate, double speed, bool paused, // The buffer is playing, so calculate the buffer rate. // There are four rate effects we apply: wheel, scratch, jog and temp. - // Wheel: a linear additive effect (no spring-back) + // Wheel: a linear additive effect (no spring-back) // Scratch: a rate multiplier - // Jog: a linear additive effect whose value is filtered (springs back) - // Temp: pitch bend + // Jog: a linear additive effect whose value is filtered (springs back) + // Temp: pitch bend // New scratch behavior - overrides playback speed (and old behavior) if (useScratch2Value) { diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index f56bfb48dda..c7dce1c6056 100644 --- a/src/engine/enginebuffer.cpp +++ b/src/engine/enginebuffer.cpp @@ -843,9 +843,9 @@ void EngineBuffer::processTrackLocked( m_trackSampleRateOld = mixxx::audio::SampleRate::fromDouble(m_pTrackSampleRate->get()); m_trackEndPositionOld = getTrackEndPosition(); - double baserate = 0.0; + double baseSampleRate = 0.0; if (sampleRate.isValid()) { - baserate = m_trackSampleRateOld / sampleRate; + baseSampleRate = m_trackSampleRateOld / sampleRate; } // Sync requests can affect rate, so process those first. @@ -875,7 +875,7 @@ void EngineBuffer::processTrackLocked( // pass for every 1 real second). Depending on whether // keylock is enabled, this is applied to either the rate or the tempo. double speed = m_pRateControl->calculateSpeed( - baserate, + baseSampleRate, tempoRatio, paused, iBufferSize, @@ -980,10 +980,10 @@ void EngineBuffer::processTrackLocked( // otherwise tempo and pitch are processed individual double rate = 0; - // If the baserate, speed, or pitch has changed, we need to update the + // If the base samplerate, speed, or pitch has changed, we need to update the // scaler. Also, if we have changed scalers then we need to update the // scaler. - if (baserate != m_baserate_old || speed != m_speed_old || + if (baseSampleRate != m_baserate_old || speed != m_speed_old || pitchRatio != m_pitch_old || tempoRatio != m_tempo_ratio_old || m_bScalerChanged) { // The rate returned by the scale object can be different from the @@ -1005,7 +1005,7 @@ void EngineBuffer::processTrackLocked( m_pScale->clear(); } - m_baserate_old = baserate; + m_baserate_old = baseSampleRate; m_speed_old = speed; m_pitch_old = pitchRatio; m_tempo_ratio_old = tempoRatio; @@ -1016,16 +1016,16 @@ void EngineBuffer::processTrackLocked( // main samplerate), the deck speed, the pitch shift, and whether // the deck speed should affect the pitch. - m_pScale->setScaleParameters(baserate, - &speed, - &pitchRatio); + m_pScale->setScaleParameters(baseSampleRate, + &speed, + &pitchRatio); // The way we treat rate inside of EngineBuffer is actually a // description of "sample consumption rate" or percentage of samples // consumed relative to playing back the track at its native sample // rate and normal speed. pitch_adjust does not change the playback // rate. - rate = baserate * speed; + rate = baseSampleRate * speed; // Scaler is up to date now. m_bScalerChanged = false; diff --git a/src/engine/positionscratchcontroller.cpp b/src/engine/positionscratchcontroller.cpp index 32353601f50..59d0d649d2f 100644 --- a/src/engine/positionscratchcontroller.cpp +++ b/src/engine/positionscratchcontroller.cpp @@ -66,17 +66,17 @@ class RateIIFilter { PositionScratchController::PositionScratchController(const QString& group) : m_group(group), - m_bScratching(false), - m_bEnableInertia(false), - m_dLastPlaypos(0), - m_dPositionDeltaSum(0), - m_dTargetDelta(0), - m_dStartScratchPosition(0), - m_dRate(0), - m_dMoveDelay(0), - m_dMouseSampeTime(0) { + m_isScratching(false), + m_inertiaEnabled(false), + m_prevSamplePos(0), + m_samplePosDeltaSum(0), + m_scratchTargetDelta(0), + m_scratchStartPos(0), + m_rate(0), + m_moveDelay(0), + m_mouseSampleTime(0) { m_pScratchEnable = new ControlObject(ConfigKey(group, "scratch_position_enable")); - m_pScratchPosition = new ControlObject(ConfigKey(group, "scratch_position")); + m_pScratchPos = new ControlObject(ConfigKey(group, "scratch_position")); m_pMainSampleRate = ControlObject::getControl( ConfigKey(QStringLiteral("[App]"), QStringLiteral("samplerate"))); m_pVelocityController = new VelocityController(); @@ -86,19 +86,17 @@ PositionScratchController::PositionScratchController(const QString& group) PositionScratchController::~PositionScratchController() { delete m_pRateIIFilter; delete m_pVelocityController; - delete m_pScratchPosition; + delete m_pScratchPos; delete m_pScratchEnable; } -//volatile double _p = 0.3; -//volatile double _d = -0.15; -//volatile double _f = 0.5; - -void PositionScratchController::process(double currentSample, double releaseRate, - int iBufferSize, double baserate) { +void PositionScratchController::process(double currentSamplePos, + double releaseRate, + int iBufferSize, + double baseSampleRate) { bool scratchEnable = m_pScratchEnable->get() != 0; - if (!m_bScratching && !scratchEnable) { + if (!m_isScratching && !scratchEnable) { // We were not previously in scratch mode are still not in scratch // mode. Do nothing return; @@ -112,29 +110,27 @@ void PositionScratchController::process(double currentSample, double releaseRate // Normally the Mouse is sampled every 8 ms so with this 16 ms window we // have 0 ... 3 samples. The remaining jitter is ironed by the following IIR // lowpass filter - const double m_dMouseSampeIntervall = 0.016; - const auto callsPerDt = static_cast(ceil(m_dMouseSampeIntervall / dt)); + const double m_mouseSampleInterval = 0.016; + const auto callsPerDt = static_cast(ceil(m_mouseSampleInterval / dt)); double scratchPosition = 0; - m_dMouseSampeTime += dt; - if (m_dMouseSampeTime >= m_dMouseSampeIntervall || !m_bScratching) { - scratchPosition = m_pScratchPosition->get(); - m_dMouseSampeTime = 0; + m_mouseSampleTime += dt; + if (m_mouseSampleTime >= m_mouseSampleInterval || !m_isScratching) { + scratchPosition = m_pScratchPos->get(); + m_mouseSampleTime = 0; } // Tweak PD controller for different latencies double p = 0.3; double d = p/-2; double f = 0.4; - if (dt > m_dMouseSampeIntervall * 2) { + if (dt > m_mouseSampleInterval * 2) { f = 1; } m_pVelocityController->setPD(p, d); m_pRateIIFilter->setFactor(f); - //m_pVelocityController->setPID(_p, _i, _d); - //m_pMouseRateIIFilter->setFactor(_f); - if (m_bScratching) { - if (m_bEnableInertia) { + if (m_isScratching) { + if (m_inertiaEnabled) { // If we got here then we're not scratching and we're in inertia // mode. Take the previous rate that was set and apply a // deceleration. @@ -160,79 +156,79 @@ void PositionScratchController::process(double currentSample, double releaseRate // alpha = (decayThreshold / kMaxVelocity) ^ (dt / kTimeToStop) const double kExponentialDecay = pow(decayThreshold / kMaxVelocity, dt / kTimeToStop); - m_dRate *= kExponentialDecay; + m_rate *= kExponentialDecay; // If the rate has decayed below the threshold, or scratching is // re-enabled then leave inertia mode. - if (fabs(m_dRate) < decayThreshold || scratchEnable) { - m_bEnableInertia = false; - m_bScratching = false; + if (fabs(m_rate) < decayThreshold || scratchEnable) { + m_inertiaEnabled = false; + m_isScratching = false; } - //qDebug() << m_dRate << kExponentialDecay << dt; + // qDebug() << m_rate << kExponentialDecay << dt; } else if (scratchEnable) { // If we're scratching, clear the inertia flag. This case should // have been caught by the 'enable' case below, but just to make // sure. - m_bEnableInertia = false; + m_inertiaEnabled = false; // 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_dPositionDeltaSum += (currentSample - m_dLastPlaypos) / - (iBufferSize * baserate); + m_samplePosDeltaSum += (currentSamplePos - m_prevSamplePos) / + (iBufferSize * baseSampleRate); // Continue with the last rate if we do not have a new // Mouse position - if (m_dMouseSampeTime == 0) { - + if (m_mouseSampleTime == 0) { // Set the scratch target to the current set position // and normalize to one buffer - double targetDelta = (scratchPosition - m_dStartScratchPosition) / - (iBufferSize * baserate); + double scratchTargetDelta = (scratchPosition - m_scratchStartPos) / + (iBufferSize * baseSampleRate); bool calcRate = true; - if (m_dTargetDelta == targetDelta) { + if (m_scratchTargetDelta == scratchTargetDelta) { // we get here, if the next mouse position is delayed // the mouse is stopped or moves slow. Since we don't know the case // we assume delayed mouse updates for 40 ms - m_dMoveDelay += dt * callsPerDt; - if (m_dMoveDelay < 0.04) { + m_moveDelay += dt * callsPerDt; + if (m_moveDelay < 0.04) { // Assume a missing Mouse Update and continue with the // previously calculated rate. calcRate = false; } else { // Mouse has stopped m_pVelocityController->setPD(p, 0); - if (targetDelta == 0) { + if (scratchTargetDelta == 0) { // Mouse was not moved at all // Stop immediately by restarting the controller // in stopped mode m_pVelocityController->reset(0); m_pRateIIFilter->reset(0); - m_dPositionDeltaSum = 0; + m_samplePosDeltaSum = 0; } } } else { - m_dMoveDelay = 0; - m_dTargetDelta = targetDelta; + m_moveDelay = 0; + m_scratchTargetDelta = scratchTargetDelta; } if (calcRate) { - double ctrlError = m_pRateIIFilter->filter(targetDelta - m_dPositionDeltaSum); - m_dRate = m_pVelocityController->observation(ctrlError); - m_dRate /= ceil(m_dMouseSampeIntervall/dt); + double ctrlError = m_pRateIIFilter->filter( + scratchTargetDelta - m_samplePosDeltaSum); + m_rate = m_pVelocityController->observation(ctrlError); + m_rate /= ceil(m_mouseSampleInterval / dt); // Note: The following SoundTouch changes the also rate by a ramp // This looks like average of the new and the old rate independent // from dt. Ramping is disabled when direction changes or rate = 0; // (determined experimentally) - if (fabs(m_dRate) < MIN_SEEK_SPEED) { + if (fabs(m_rate) < MIN_SEEK_SPEED) { // we cannot get closer - m_dRate = 0; + m_rate = 0; } } - //qDebug() << m_dRate << targetDelta << m_dPositionDeltaSum << dt; + // qDebug() << m_rate << scratchTargetDelta << m_samplePosDeltaSum << dt; } } else { // We were previously in scratch mode and are no longer in scratch @@ -243,42 +239,43 @@ void PositionScratchController::process(double currentSample, double releaseRate // an 'inertia' mode. constexpr double kThrowThreshold = 2.5; - if (fabs(m_dRate) > kThrowThreshold) { - m_bEnableInertia = true; + if (fabs(m_rate) > kThrowThreshold) { + m_inertiaEnabled = true; } else { - m_bScratching = false; + m_isScratching = false; } //qDebug() << "disable"; } } else if (scratchEnable) { - // We were not previously in scratch mode but now are in scratch - // mode. Enable scratching. - m_bScratching = true; - m_bEnableInertia = false; - m_dMoveDelay = 0; - // Set up initial values, in a way that the system is settled - m_dRate = releaseRate; - m_dPositionDeltaSum = -(releaseRate / p) * callsPerDt; // Set to the remaining error of a p controller - m_pVelocityController->reset(-m_dPositionDeltaSum); - m_pRateIIFilter->reset(-m_dPositionDeltaSum); - m_dStartScratchPosition = scratchPosition; - //qDebug() << "scratchEnable()" << currentSample; + // We were not previously in scratch mode but now are in scratch + // mode. Enable scratching. + m_isScratching = true; + m_inertiaEnabled = false; + m_moveDelay = 0; + // Set up initial values, in a way that the system is settled + m_rate = releaseRate; + m_samplePosDeltaSum = -(releaseRate / p) * + callsPerDt; // Set to the remaining error of a p controller + m_pVelocityController->reset(-m_samplePosDeltaSum); + m_pRateIIFilter->reset(-m_samplePosDeltaSum); + m_scratchStartPos = scratchPosition; + // qDebug() << "scratchEnable()" << currentSamplePos; } - m_dLastPlaypos = currentSample; + m_prevSamplePos = currentSamplePos; } bool PositionScratchController::isEnabled() { - // return true only if m_dRate is valid. - return m_bScratching; + // return true only if m_rate is valid. + return m_isScratching; } double PositionScratchController::getRate() { - return m_dRate; + return m_rate; } void PositionScratchController::notifySeek(mixxx::audio::FramePos position) { DEBUG_ASSERT(position.isValid()); // scratching continues after seek due to calculating the relative distance traveled - // in m_dPositionDeltaSum - m_dLastPlaypos = position.toEngineSamplePos(); + // in m_samplePosDeltaSum + m_prevSamplePos = position.toEngineSamplePos(); } diff --git a/src/engine/positionscratchcontroller.h b/src/engine/positionscratchcontroller.h index fe4d5034d20..1eb504e9495 100644 --- a/src/engine/positionscratchcontroller.h +++ b/src/engine/positionscratchcontroller.h @@ -6,8 +6,8 @@ #include "audio/frame.h" class ControlObject; -class VelocityController; class RateIIFilter; +class VelocityController; class PositionScratchController : public QObject { Q_OBJECT @@ -15,8 +15,7 @@ class PositionScratchController : public QObject { PositionScratchController(const QString& group); virtual ~PositionScratchController(); - void process(double currentSample, double releaseRate, - int iBufferSize, double baserate); + void process(double currentSample, double releaseRate, int iBufferSize, double baseSampleRate); bool isEnabled(); double getRate(); void notifySeek(mixxx::audio::FramePos position); @@ -24,17 +23,17 @@ class PositionScratchController : public QObject { private: const QString m_group; ControlObject* m_pScratchEnable; - ControlObject* m_pScratchPosition; + ControlObject* m_pScratchPos; ControlObject* m_pMainSampleRate; VelocityController* m_pVelocityController; RateIIFilter* m_pRateIIFilter; - bool m_bScratching; - bool m_bEnableInertia; - double m_dLastPlaypos; - double m_dPositionDeltaSum; - double m_dTargetDelta; - double m_dStartScratchPosition; - double m_dRate; - double m_dMoveDelay; - double m_dMouseSampeTime; + bool m_isScratching; + bool m_inertiaEnabled; + double m_prevSamplePos; + double m_samplePosDeltaSum; + double m_scratchTargetDelta; + double m_scratchStartPos; + double m_rate; + double m_moveDelay; + double m_mouseSampleTime; }; From 50e16c96face8890d7f912385a9ba59f26953ed2 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Wed, 27 Mar 2024 22:28:01 +0100 Subject: [PATCH 2/2] 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;