Skip to content

Commit

Permalink
PositionScratchCon: fix explicit seeking while scratching, eg. jump t…
Browse files Browse the repository at this point in the history 
…o hotcue

adopt the seek position explicitly in order to avoid crazy seek speeds
  • Loading branch information
@ronso0
ronso0 committed Dec 26, 2024
1 parent bf55c08 commit 6927fdc
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 8 deletions.
7 changes: 7 additions & 0 deletions src/engine/enginebuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,13 @@ void EngineBuffer::setNewPlaypos(mixxx::audio::FramePos position) {

// Must hold the engineLock while using m_engineControls
for (const auto& pControl : std::as_const(m_engineControls)) {
if (pControl == m_pRateControl) {
// qWarning() << "---------------------------------";
// qWarning() << "--------------------EB setNewPlaypos, notify all "
// "engine controls"
// << m_playPos;
// qWarning() << "---------------------------------";
}
pControl->notifySeek(m_playPos);
}

Expand Down
114 changes: 107 additions & 7 deletions src/engine/positionscratchcontroller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ constexpr double kThrowThreshold = 2.5;
// Max velocity we would like to stop in a given time period.
constexpr double kMaxVelocity = 100;
// Seconds to stop a throw at the max velocity.
// TODO make configurable, eg. to customize spinbacks with controllers
constexpr double kTimeToStop = 1.0;
constexpr double kSeekPosTolerance = 50;
constexpr int kMaxSeekWaitCount = 1;

} // anonymous namespace

Expand All @@ -72,6 +75,7 @@ PositionScratchController::PositionScratchController(const QString& group)
m_isScratching(false),
m_inertiaEnabled(false),
m_prevSamplePos(0),
m_seekSamplePos(std::numeric_limits<double>::quiet_NaN()),
m_samplePosDeltaSum(0),
m_scratchTargetDelta(0),
m_scratchStartPos(0),
Expand All @@ -83,7 +87,8 @@ PositionScratchController::PositionScratchController(const QString& group)
m_callsPerDt(1),
m_p(1),
m_d(1),
m_f(0.4) {
m_f(0.4),
m_seekWaitCounter(-1) {
m_pScratchEnable = std::make_unique<ControlObject>(
ConfigKey(group, QStringLiteral("scratch_position_enable")));

Expand Down Expand Up @@ -139,6 +144,47 @@ void PositionScratchController::process(double currentSamplePos,
return;
}

double queuedSeekPos = m_seekSamplePos;
bool adoptSeekPos = false;
if (!util_isnan(queuedSeekPos)) {
if (m_seekWaitCounter >= 0 &&
m_seekWaitCounter <= kMaxSeekWaitCount &&
fabs(currentSamplePos - queuedSeekPos) < kSeekPosTolerance) {
resetSeekPos();
adoptSeekPos = true;

// Alternatively:
// adopt position
// m_prevSamplePos = queuedSeekPos;
// restart scratching.
// works okay (stops) but also previews the hotcue for a fraction of a second,
// presumably because RateControl simply continues playing with the
// desired rate if m_isScratching is false, and doesn't ask for m_rate.
// m_isScratching = false;
// m_inertiaEnabled = false;
// This alone still gives the crazy seeking:
// m_rate = 0;
// return;

// qWarning() <<
// "--------------------------------------------------------match
// queued seek pos after"
// << m_seekWaitCounter << "calls at"
// << qSetRealNumberPrecision(18) << currentSamplePos;
// qWarning() <<
// "------------------------------------------------------------------------------";
} else if (m_seekWaitCounter >= 0 && m_seekWaitCounter < kMaxSeekWaitCount) {
// qWarning() << "--------------------------------------------------------seek queued"
// << m_seekWaitCounter << "calls ago";
m_seekWaitCounter++;
} else {
resetSeekPos();
// qWarning() <<
// "--------------------------------------------------------reset
// queued seek";
}
}

if (bufferSize != m_bufferSize) {
m_bufferSize = bufferSize;
slotSampleRateChanged(m_pMainSampleRate->get());
Expand Down Expand Up @@ -189,7 +235,11 @@ void PositionScratchController::process(double currentSamplePos,
m_inertiaEnabled = false;

double sampleDelta = 0.0;
// Seeks occur if we wrap around due to loop/repeat and when we've
// seeked explicitly, for example when jumping to cue/hotcue.
// We consider it only in the explicit case.
if (wrappedAround > 0) {
// qWarning() << "XXX----wrappedAround" << wrappedAround;
// 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
Expand All @@ -208,13 +258,28 @@ void PositionScratchController::process(double currentSamplePos,
(triggerPos - m_prevSamplePos) +
(currentSamplePos - targetPos);
} else {
// Check if we need to adopt a new position after seek.
if (adoptSeekPos) {
m_prevSamplePos = queuedSeekPos;
// qWarning() << " ----------------------adopt seek pos";
}
sampleDelta = currentSamplePos - m_prevSamplePos;
if (adoptSeekPos) {
// qWarning() <<
// "----------------------------------------------------------------sampleDelta"
// << sampleDelta;
}
}

// Measure the total distance traveled since last call, add it to the
// running total and normalize to one buffer.
// This is required to scratch within loop boundaries.
m_samplePosDeltaSum += (sampleDelta) / (bufferSize * baseSampleRate);
// if (adoptSeekPos) {
// qWarning() <<
// "--------------------------------------------------------m_samplePosDeltaSum"
// << m_samplePosDeltaSum;
// }

// If we may have a new position, calculate scratch parameters and
// eventually the new rate.
Expand All @@ -224,13 +289,24 @@ void PositionScratchController::process(double currentSamplePos,
// and normalize to one buffer
double scratchTargetDelta = (scratchPosition - m_scratchStartPos) /
(bufferSize * baseSampleRate);
// if (adoptSeekPos) {
// qWarning() <<
// "---------------------------------------------------------scratchTargetDelta"
// << scratchTargetDelta;
// }

bool calcRate = true;

if (m_scratchTargetDelta == scratchTargetDelta) {
// We get here if the next mouse position is delayed, the mouse
// is stopped or moves very slowly. Since we don't know the case
// we assume delayed mouse updates for 40 ms.
// if (adoptSeekPos) {
// qWarning() <<
// "--------------------------------------------------------scratchTargetDelta
// == "
// "m_scratchTargetDelta";
// }
// We get here if the next mouse position is delayed, the
// mouse is stopped or moves very slowly. Since we don't
// know the case we assume delayed mouse updates for 40 ms.
// TODO Make threshold configurable for controller use?
m_moveDelay += m_dt * m_callsPerDt;
if (m_moveDelay < kMoveDelayMax) {
Expand All @@ -250,6 +326,10 @@ void PositionScratchController::process(double currentSamplePos,
}
}
} else {
// if (adoptSeekPos) {
// qWarning() << "--------------------------------------------------------"
// "scratchTargetDelta != m_scratchTargetDelta";
// }
m_moveDelay = 0;
m_scratchTargetDelta = scratchTargetDelta;
}
Expand All @@ -259,12 +339,14 @@ void PositionScratchController::process(double currentSamplePos,
scratchTargetDelta - m_samplePosDeltaSum);
m_rate = m_pVelocityController->observation(ctrlError);
m_rate /= ceil(kDefaultSampleInterval / m_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 m_dt. Ramping is disabled when direction changes or rate = 0;
// (determined experimentally)
if (fabs(m_rate) < MIN_SEEK_SPEED) {
// we cannot get closer
// TODO ramp to new rate
m_rate = 0;
}
}
Expand Down Expand Up @@ -297,6 +379,7 @@ void PositionScratchController::process(double currentSamplePos,
m_scratchStartPos = scratchPosition;
// qDebug() << "scratchEnable()" << currentSamplePos;
}

m_prevSamplePos = currentSamplePos;
}

Expand All @@ -311,7 +394,24 @@ double PositionScratchController::getRate() {

void PositionScratchController::notifySeek(mixxx::audio::FramePos position) {
DEBUG_ASSERT(position.isValid());
// scratching continues after seek due to calculating the relative distance traveled
// in m_samplePosDeltaSum
m_prevSamplePos = position.toEngineSamplePos();
double newPos = position.toEngineSamplePos();
if (m_prevSamplePos == newPos) {
// qWarning() << "------------------------------------------------------- "
// "equal --> ignore";
return;
} else if (fabs(m_prevSamplePos - newPos) < 100) {
// qWarning() << "------------------------------------------------------- "
// "very close --> ignore";
return;
}
// ?? Scratching continues after seek due to calculating the relative
// ?? distance traveled in m_samplePosDeltaSum
// Note: it can happen that RateControl is not yet aware of the seek position,
// hence it's possible that we get the new position only in thesecond next
// process() call.
// That's what the wait counter is for: keep the seek position for next to calls,
// then reset it (we might come across the seek position just by playing or scratching
// and then we must process it regularly.
m_seekSamplePos = newPos;
m_seekWaitCounter = 0;
}
8 changes: 8 additions & 0 deletions src/engine/positionscratchcontroller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ class PositionScratchController : public QObject {
void slotSampleRateChanged(double sampleRate);

private:
void resetSeekPos() {
m_seekSamplePos = std::numeric_limits<double>::quiet_NaN();
m_seekWaitCounter = -1;
}

const QString m_group;
std::unique_ptr<ControlObject> m_pScratchEnable;
std::unique_ptr<ControlObject> m_pScratchPos;
Expand All @@ -63,6 +68,7 @@ class PositionScratchController : public QObject {
bool m_isScratching;
bool m_inertiaEnabled;
double m_prevSamplePos;
double m_seekSamplePos;
double m_samplePosDeltaSum;
double m_scratchTargetDelta;
double m_scratchStartPos;
Expand All @@ -78,4 +84,6 @@ class PositionScratchController : public QObject {
double m_p;
double m_d;
double m_f;

int m_seekWaitCounter;
};
5 changes: 4 additions & 1 deletion src/engine/readaheadmanager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,10 @@ double ReadAheadManager::getFilePlaypositionFromLog(
const auto seekPosition =

Check failure on line 295 in src/engine/readaheadmanager.cpp

View workflow job for this annotation

GitHub Actions / clazy 

unused variable 'seekPosition' [-Werror,-Wunused-variable]

Check failure on line 295 in src/engine/readaheadmanager.cpp

View workflow job for this annotation

GitHub Actions / Ubuntu 22.04 

variable ‘seekPosition’ set but not used [-Werror=unused-but-set-variable]

Check failure on line 295 in src/engine/readaheadmanager.cpp

View workflow job for this annotation

GitHub Actions / macOS 13 x64 

unused variable 'seekPosition' [-Werror,-Wunused-variable]

Check failure on line 295 in src/engine/readaheadmanager.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy 

unused variable 'seekPosition' [clang-diagnostic-unused-variable]

Check warning on line 295 in src/engine/readaheadmanager.cpp

View workflow job for this annotation

GitHub Actions / coverage 

variable ‘seekPosition’ set but not used [-Wunused-but-set-variable]

Check failure on line 295 in src/engine/readaheadmanager.cpp

View workflow job for this annotation

GitHub Actions / macOS 13 arm64 

unused variable 'seekPosition' [-Werror,-Wunused-variable]
mixxx::audio::FramePos::fromEngineSamplePos(
entry.virtualPlaypositionStart);
m_pRateControl->notifySeek(seekPosition);
// Note(ronso0) This has been a no-op until now and seeking
// worked fine, so deactivate it in order to not interfere
// with the fix in PositionScratchController.
// m_pRateControl->notifySeek(seekPosition);
}
}

Expand Down

0 comments on commit 6927fdc

Please sign in to comment.