From 00dc727ba320f62e89808601a8d6432ae5b77087 Mon Sep 17 00:00:00 2001 From: 0cwa Date: Sun, 13 Sep 2026 08:04:00 +0200 Subject: [PATCH 1/2] Add an all-or-nothing reader retry contract Expose bounded retry reads for callers that must preserve an absolute input range across cache misses. Keep the generic reader and readahead state independent of any optional stretcher. --- src/engine/cachingreader/cachingreader.cpp | 93 +++++- src/engine/cachingreader/cachingreader.h | 37 +++ src/engine/readaheadmanager.cpp | 339 ++++++++++++--------- src/engine/readaheadmanager.h | 65 ++++ src/test/readaheadmanager_test.cpp | 286 ++++++++++++++++- 5 files changed, 681 insertions(+), 139 deletions(-) diff --git a/src/engine/cachingreader/cachingreader.cpp b/src/engine/cachingreader/cachingreader.cpp index 2859dd96aa0d..69ff0129ae1d 100644 --- a/src/engine/cachingreader/cachingreader.cpp +++ b/src/engine/cachingreader/cachingreader.cpp @@ -41,6 +41,7 @@ CachingReader::CachingReader(const QString& group, UserSettingsPointer config, mixxx::audio::ChannelCount maxSupportedChannel) : m_pConfig(config), + m_retryOnCacheMiss(false), // Limit the number of in-flight requests to the worker. This should // prevent to overload the worker when it is not able to fetch those // requests from the FIFO timely. Otherwise outdated requests pile up @@ -60,6 +61,7 @@ CachingReader::CachingReader(const QString& group, m_lruCachingReaderChunk(nullptr), m_sampleBuffer(CachingReaderChunk::kFrames * maxSupportedChannel * kNumberOfCachedChunksInMemory), + m_retryReadBuffer(CachingReaderChunk::kFrames * maxSupportedChannel), m_worker(group, &m_chunkReadRequestFIFO, &m_readerStatusUpdateFIFO, @@ -305,6 +307,60 @@ CachingReader::ReadResult CachingReader::read(SINT startSample, bool reverse, CSAMPLE* buffer, mixxx::audio::ChannelCount channelCount) { + return readInternal(startSample, + numSamples, + reverse, + buffer, + channelCount, + m_retryOnCacheMiss); +} + +CachingReader::ReadResult CachingReader::readWithRetry(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount) { + VERIFY_OR_DEBUG_ASSERT(numSamples >= 0 && + static_cast(numSamples) <= m_retryReadBuffer.size()) { + return ReadResult::UNAVAILABLE; + } + VERIFY_OR_DEBUG_ASSERT(buffer) { + return ReadResult::UNAVAILABLE; + } + + DEBUG_ASSERT(!m_retryOnCacheMiss); + m_retryOnCacheMiss = true; + const auto retryResult = readWithRetryHook(startSample, + numSamples, + reverse, + m_retryReadBuffer.data(), + channelCount); + m_retryOnCacheMiss = false; + + if (retryResult.retryPending || + retryResult.result == ReadResult::UNAVAILABLE) { + return ReadResult::UNAVAILABLE; + } + SampleUtil::copy(buffer, m_retryReadBuffer.data(), numSamples); + return retryResult.result; +} + +CachingReader::RetryReadResult CachingReader::readWithRetryHook(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount) { + const auto result = read( + startSample, numSamples, reverse, buffer, channelCount); + return {result, result == ReadResult::UNAVAILABLE}; +} + +CachingReader::ReadResult CachingReader::readInternal(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount, + bool retryOnCacheMiss) { // Check for bad inputs // Refuse to read from an invalid position VERIFY_OR_DEBUG_ASSERT(startSample % channelCount == 0) { @@ -332,7 +388,6 @@ CachingReader::ReadResult CachingReader::read(SINT startSample, << "buffer =" << buffer; return ReadResult::UNAVAILABLE; } - // If no track is loaded, don't do anything. if (atomicLoadRelaxed(m_state) != STATE_TRACK_LOADED) { return ReadResult::UNAVAILABLE; @@ -365,6 +420,42 @@ CachingReader::ReadResult CachingReader::read(SINT startSample, CachingReaderChunk::samples2frames(numSamples, channelCount)); DEBUG_ASSERT(!remainingFrameIndexRange.empty()); + if (retryOnCacheMiss) { + auto preflightFrameIndexRange = + intersect(remainingFrameIndexRange, m_readableFrameIndexRange); + if (!preflightFrameIndexRange.empty()) { + const SINT firstChunkIndex = + CachingReaderChunk::indexForFrame(preflightFrameIndexRange.start()); + SINT lastChunkIndex = + CachingReaderChunk::indexForFrame(preflightFrameIndexRange.end() - 1); + for (SINT chunkIndex = firstChunkIndex; + chunkIndex <= lastChunkIndex; + ++chunkIndex) { + process(); + preflightFrameIndexRange = intersect( + preflightFrameIndexRange, m_readableFrameIndexRange); + if (preflightFrameIndexRange.empty()) { + break; + } + lastChunkIndex = CachingReaderChunk::indexForFrame( + preflightFrameIndexRange.end() - 1); + if (lastChunkIndex < chunkIndex) { + break; + } + + const CachingReaderChunkForOwner* const pChunk = + lookupChunkAndFreshen(chunkIndex); + if (!pChunk || + pChunk->getState() != CachingReaderChunkForOwner::READY) { + DEBUG_ASSERT(!pChunk || + pChunk->getState() == + CachingReaderChunkForOwner::READ_PENDING); + return ReadResult::UNAVAILABLE; + } + } + } + } + auto result = ReadResult::AVAILABLE; if (!intersect(remainingFrameIndexRange, m_readableFrameIndexRange).empty()) { // Fill the buffer up to the first readable sample with diff --git a/src/engine/cachingreader/cachingreader.h b/src/engine/cachingreader/cachingreader.h index fc77167b9271..186f8994bfce 100644 --- a/src/engine/cachingreader/cachingreader.h +++ b/src/engine/cachingreader/cachingreader.h @@ -107,6 +107,14 @@ class CachingReader : public QObject { CSAMPLE* buffer, mixxx::audio::ChannelCount channelCount); + // Like read(), but treats a cache miss in any required chunk as an + // all-or-nothing failure. On ReadResult::UNAVAILABLE, buffer is untouched. + ReadResult readWithRetry(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount); + // Issue a list of hints, but check whether any of the hints request a chunk // that is not in the cache. If any hints do request a chunk not in cache, // then wake the reader so that it can process them. Must only be called @@ -126,6 +134,23 @@ class CachingReader : public QObject { m_worker.setScheduler(pScheduler); } + protected: + struct RetryReadResult { + ReadResult result; + bool retryPending; + }; + + // Explicit retry hook. Implementations write only to the provided staging + // buffer and report whether the same absolute range must be retried. The + // default implementation accepts PARTIALLY_AVAILABLE as intentional + // padding from legacy readers. Retry-aware subclasses must override this + // hook to identify partial cache misses. + virtual RetryReadResult readWithRetryHook(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount); + signals: // Emitted once a new track is loaded and ready to be read from. void trackLoading(); @@ -136,7 +161,15 @@ class CachingReader : public QObject { void trackLoadFailed(TrackPointer pTrack, const QString& reason); private: + ReadResult readInternal(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount, + bool retryOnCacheMiss); + const UserSettingsPointer m_pConfig; + bool m_retryOnCacheMiss; // Thread-safe FIFOs for communication between the engine callback and // reader thread. @@ -194,6 +227,10 @@ class CachingReader : public QObject { // The raw memory buffer which is divided up into chunks. mixxx::SampleBuffer m_sampleBuffer; + // Preallocated staging storage that preserves the caller's buffer until a + // retry read has completed atomically. + mixxx::SampleBuffer m_retryReadBuffer; + // The readable frame index range as reported by the worker. mixxx::IndexRange m_readableFrameIndexRange; diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index 0838f6b036c5..53ddd07b5141 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -40,116 +40,214 @@ ReadAheadManager::~ReadAheadManager() { SampleUtil::free(m_pCrossFadeBuffer); } +ReadAheadManager::NextSamplesResult ReadAheadManager::getNextSamplesWithRetry( + double dRate, + CSAMPLE* pOutput, + SINT requested_samples, + mixxx::audio::ChannelCount channelCount) { + return getNextSamplesInternal( + dRate, pOutput, requested_samples, channelCount, true); +} + +void ReadAheadManager::cancelPendingRetry() { + m_pendingRetry.active = false; +} + SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, SINT requested_samples, mixxx::audio::ChannelCount channelCount) { - // qDebug() << "getNextSamples:" << m_currentPosition << requested_samples; + return getNextSamplesInternal( + dRate, pOutput, requested_samples, channelCount, false) + .samplesRead; +} - int modSamples = requested_samples % channelCount; - if (modSamples != 0) { - qDebug() << "ERROR: Non-aligned requested_samples to ReadAheadManager::getNextSamples"; - requested_samples -= modSamples; - } - bool in_reverse = dRate < 0; +ReadAheadManager::ReadPlan ReadAheadManager::makeReadPlan(bool inReverse, + SINT requestSamples, + SINT requestedSamples, + mixxx::audio::ChannelCount channelCount) { + ReadPlan plan; + plan.active = true; + plan.inReverse = inReverse; + plan.requestPosition = m_currentPosition; + plan.requestSamples = requestSamples; + plan.requestedSamples = requestedSamples; + plan.channelCount = channelCount; - mixxx::audio::FramePos targetPosition; // A loop (beat loop or track on repeat) will only limit the amount we // can read in one shot. - const mixxx::audio::FramePos loopTriggerPosition = - m_pLoopingControl->nextTrigger(in_reverse, + plan.loopTriggerPosition = + m_pLoopingControl->nextTrigger(inReverse, mixxx::audio::FramePos::fromSamplePosMaybeInvalid( m_currentPosition, channelCount), - &targetPosition); - const double loop_trigger = loopTriggerPosition.toSamplePosMaybeInvalid(channelCount); - double target = targetPosition.toSamplePosMaybeInvalid(channelCount); - - SINT preseek_samples = 0; - double samplesToSeekTrigger = 0.0; - - bool reachedTrigger = false; - - // By default, we are reading as many sampler as requested - SINT samples_from_reader = requested_samples; - if (loop_trigger != kNoTrigger) { - samplesToSeekTrigger = in_reverse ? m_currentPosition - loop_trigger - : loop_trigger - m_currentPosition; - if (samplesToSeekTrigger >= 0.0) { - // We can only read whole frames from the reader. - // Use ceil here, to be sure to reach the loop trigger. - preseek_samples = SampleUtil::ceilPlayPosToFrameStart( - samplesToSeekTrigger, channelCount); - // clamp requested samples from the caller to the loop trigger point - if (preseek_samples <= requested_samples) { - reachedTrigger = true; - samples_from_reader = preseek_samples; + &plan.loopTargetPosition); + const double loopTrigger = + plan.loopTriggerPosition.toSamplePosMaybeInvalid(channelCount); + plan.target = plan.loopTargetPosition.toSamplePosMaybeInvalid(channelCount); + plan.targetPosition = plan.loopTargetPosition; + + // By default, we are reading as many samples as requested. + plan.samplesFromReader = requestedSamples; + if (loopTrigger != kNoTrigger) { + plan.samplesToSeekTrigger = inReverse + ? m_currentPosition - loopTrigger + : loopTrigger - m_currentPosition; + if (plan.samplesToSeekTrigger >= 0.0) { + // We can only read whole frames from the reader. Use ceil here, to + // be sure to reach the loop trigger. + plan.preseekSamples = SampleUtil::ceilPlayPosToFrameStart( + plan.samplesToSeekTrigger, channelCount); + if (plan.preseekSamples <= requestedSamples) { + plan.reachedTrigger = true; + plan.samplesFromReader = plan.preseekSamples; } } } - mixxx::audio::FramePos jumpTargetPosition; // A saved jump cue will only limit the amount we can read in one shot. - const mixxx::audio::FramePos jumpTriggerPosition = - m_pCueControl->nextTrigger(in_reverse, + plan.jumpTriggerPosition = + m_pCueControl->nextTrigger(inReverse, mixxx::audio::FramePos::fromSamplePosMaybeInvalid( m_currentPosition, channelCount), - &jumpTargetPosition, - static_cast(requested_samples / channelCount)); - double jump_trigger = jumpTriggerPosition.toSamplePosMaybeInvalid(channelCount); + &plan.jumpTargetPosition, + static_cast( + requestedSamples / channelCount)); + double jumpTrigger = + plan.jumpTriggerPosition.toSamplePosMaybeInvalid(channelCount); // If there is both a loop and saved jump that are armed, and they both // cancel each other (Loop from A -> B, jump from A -> B), we no-op the jump - // to prevent an infinite silent play loop - if (jump_trigger != kNoTrigger && loop_trigger != kNoTrigger && - jumpTriggerPosition == targetPosition && - loopTriggerPosition == jumpTargetPosition) { - jump_trigger = kNoTrigger; + // to prevent an infinite silent play loop. + if (jumpTrigger != kNoTrigger && loopTrigger != kNoTrigger && + plan.jumpTriggerPosition == plan.loopTargetPosition && + plan.loopTriggerPosition == plan.jumpTargetPosition) { + jumpTrigger = kNoTrigger; } - SINT prejump_samples = 0; - double samplesToJumpTrigger = 0.0; - - if (jump_trigger != kNoTrigger) { - samplesToJumpTrigger = in_reverse ? m_currentPosition - jump_trigger - : jump_trigger - m_currentPosition; + if (jumpTrigger != kNoTrigger) { + const double samplesToJumpTrigger = inReverse + ? m_currentPosition - jumpTrigger + : jumpTrigger - m_currentPosition; if (samplesToJumpTrigger >= 0.0) { - // We can only read whole frames from the reader. - // Use ceil here, to be sure to reach the jump trigger. - prejump_samples = SampleUtil::ceilPlayPosToFrameStart( + const SINT prejumpSamples = SampleUtil::ceilPlayPosToFrameStart( samplesToJumpTrigger, channelCount); - // clamp requested samples from the caller to the jump trigger point - if (prejump_samples <= requested_samples) { - reachedTrigger = true; + if (prejumpSamples <= requestedSamples) { + plan.reachedTrigger = true; // A loop end may be before the jump. If the jump is first, this - // should be our new target - if (loop_trigger == kNoTrigger || prejump_samples < preseek_samples) { - samples_from_reader = prejump_samples; - preseek_samples = prejump_samples; - samplesToSeekTrigger = samplesToJumpTrigger; - target = jumpTargetPosition.toSamplePosMaybeInvalid(channelCount); - targetPosition = jumpTargetPosition; + // should be our new target. + if (loopTrigger == kNoTrigger || + prejumpSamples < plan.preseekSamples) { + plan.samplesFromReader = prejumpSamples; + plan.preseekSamples = prejumpSamples; + plan.samplesToSeekTrigger = samplesToJumpTrigger; + plan.target = + plan.jumpTargetPosition.toSamplePosMaybeInvalid(channelCount); + plan.targetPosition = plan.jumpTargetPosition; } } } } + plan.startSample = SampleUtil::roundPlayPosToFrameStart( + m_currentPosition, channelCount); + if (plan.reachedTrigger) { + plan.positionAfterTrigger = plan.target; + if (plan.preseekSamples > 0) { + // Compensate for reading up to one frame past the trigger so the + // loop or saved jump retains its intended length. + plan.positionAfterTrigger += + plan.preseekSamples - plan.samplesToSeekTrigger; + } + plan.seekReadPosition = SampleUtil::roundPlayPosToFrameStart( + plan.positionAfterTrigger + + (plan.inReverse + ? plan.preseekSamples + : -plan.preseekSamples), + channelCount); + plan.crossFadeSamples = plan.samplesFromReader; + if (plan.seekReadPosition < 0) { + plan.crossFadeStart = -plan.seekReadPosition; + plan.crossFadeSamples -= plan.crossFadeStart; + } else { + const int trackSamples = static_cast( + m_pLoopingControl->getTrackFrame().toSamplePos(channelCount)); + if (plan.seekReadPosition > trackSamples) { + plan.crossFadeStart = plan.seekReadPosition - trackSamples; + plan.crossFadeSamples -= plan.crossFadeStart; + } + } + plan.crossFadeReadPosition = plan.seekReadPosition + + (plan.inReverse ? plan.crossFadeStart : -plan.crossFadeStart); + } + return plan; +} + +ReadAheadManager::NextSamplesResult ReadAheadManager::getNextSamplesInternal( + double dRate, + CSAMPLE* pOutput, + SINT requested_samples, + mixxx::audio::ChannelCount channelCount, + bool retryOnCacheMiss) { + // qDebug() << "getNextSamples:" << m_currentPosition << requested_samples; + + const SINT requestSamples = requested_samples; + int modSamples = requested_samples % channelCount; + if (modSamples != 0) { + qDebug() << "ERROR: Non-aligned requested_samples to ReadAheadManager::getNextSamples"; + requested_samples -= modSamples; + } + const bool inReverse = dRate < 0; + const bool reusePendingRetry = retryOnCacheMiss && + m_pendingRetry.matches(m_currentPosition, + inReverse, + requestSamples, + channelCount); + if (m_pendingRetry.active && !reusePendingRetry) { + cancelPendingRetry(); + } + + ReadPlan plan = reusePendingRetry + ? m_pendingRetry + : makeReadPlan( + inReverse, requestSamples, requested_samples, channelCount); + // Sanity checks. - VERIFY_OR_DEBUG_ASSERT(samples_from_reader >= 0) { + VERIFY_OR_DEBUG_ASSERT(plan.samplesFromReader >= 0) { qDebug() << "Need negative samples in ReadAheadManager::getNextSamples. Ignoring read"; - return 0; + return {0, false}; } - SINT start_sample = SampleUtil::roundPlayPosToFrameStart( - m_currentPosition, channelCount); + if (retryOnCacheMiss && !reusePendingRetry) { + m_pendingRetry = plan; + } - const auto readResult = m_pReader->read( - start_sample, samples_from_reader, in_reverse, pOutput, channelCount); + const auto readResult = retryOnCacheMiss + ? m_pReader->readWithRetry( + plan.startSample, + plan.samplesFromReader, + plan.inReverse, + pOutput, + channelCount) + : m_pReader->read( + plan.startSample, + plan.samplesFromReader, + plan.inReverse, + pOutput, + channelCount); if (readResult == CachingReader::ReadResult::UNAVAILABLE) { // Cache miss - no samples written - SampleUtil::clear(pOutput, samples_from_reader); + SampleUtil::clear(pOutput, + retryOnCacheMiss ? plan.requestSamples : plan.samplesFromReader); // Set the cache miss flag to decide when to apply ramping // after the following read attempts. m_cacheMissCount++; + if (retryOnCacheMiss) { + // Do not advance the read-ahead cursor when a grain-based scaler + // asks for a retryable read. Advancing here would make its next + // attempt label the newly available chunk with the wrong absolute + // frame position. + return {0, true}; + } } else if (m_cacheMissCount > 0) { // Previous read was a cache miss, but now we got something back. // Apply ramping gain, because the last buffer has unwanted silence @@ -157,7 +255,7 @@ SINT ReadAheadManager::getNextSamples(double dRate, SampleUtil::applyRampingGain(pOutput, CSAMPLE_GAIN_ZERO, CSAMPLE_GAIN_ONE, - samples_from_reader); + plan.samplesFromReader); // Reset the cache miss flag, because we are now back on track. if (!m_cacheMissExpected) { qDebug() << "ReadAheadManager: continue after number cache misses:" << m_cacheMissCount; @@ -169,91 +267,53 @@ SINT ReadAheadManager::getNextSamples(double dRate, // Increment or decrement current read-ahead position // Mixing int and double here is desired, because the fractional frame should // be resist - if (in_reverse) { - addReadLogEntry(m_currentPosition, m_currentPosition - samples_from_reader); - m_currentPosition -= samples_from_reader; + if (plan.inReverse) { + addReadLogEntry( + m_currentPosition, m_currentPosition - plan.samplesFromReader); + m_currentPosition -= plan.samplesFromReader; } else { - addReadLogEntry(m_currentPosition, m_currentPosition + samples_from_reader); - m_currentPosition += samples_from_reader; + addReadLogEntry( + m_currentPosition, m_currentPosition + plan.samplesFromReader); + m_currentPosition += plan.samplesFromReader; } // Activate on this trigger if necessary - if (reachedTrigger) { - DEBUG_ASSERT(target != kNoTrigger); + if (plan.reachedTrigger) { + DEBUG_ASSERT(plan.target != kNoTrigger); if (m_pRateControl) { - m_pRateControl->notifyWrapAround(loopTriggerPosition.isValid() - ? loopTriggerPosition - : jumpTriggerPosition, - targetPosition); + m_pRateControl->notifyWrapAround(plan.loopTriggerPosition.isValid() + ? plan.loopTriggerPosition + : plan.jumpTriggerPosition, + plan.targetPosition); } // TODO probably also useful for hotcue_X_indicator in CueControl::updateIndicators() // Jump to other end of loop or track. - m_currentPosition = target; - if (preseek_samples > 0) { - // we are up to one frame ahead of the loop trigger - double overshoot = preseek_samples - samplesToSeekTrigger; - // start the loop later accordingly to be sure the loop length is as desired - // e.g. exactly one bar. - m_currentPosition += overshoot; - - // Example in frames; - // loop start 1.1 loop end 3.3 loop length 2.2 - // m_currentPosition samplesToLoopTrigger preloop_samples - // 2.0 1.3 2 - // 1.8 1.5 2 - // 1.6 1.7 2 - // 1.4 1.9 2 - // 1.2 2.1 3 - // Average preloop_samples = 2.2 - } - - // start reading before the loop start point or the saved jump, to crossfade these samples - // with the samples we need to the loop end - int seek_read_position = SampleUtil::roundPlayPosToFrameStart( - m_currentPosition + - (in_reverse ? preseek_samples : -preseek_samples), - channelCount); + m_currentPosition = plan.positionAfterTrigger; - int crossFadeStart = 0; - int crossFadeSamples = samples_from_reader; - if (seek_read_position < 0) { - // we start in the pre-role without suitable samples for crossfading - crossFadeStart = -seek_read_position; - crossFadeSamples -= crossFadeStart; - } else { - int trackSamples = static_cast( - m_pLoopingControl->getTrackFrame().toSamplePos( - channelCount)); - if (seek_read_position > trackSamples) { - // looping in reverse overlapping post-roll without samples - crossFadeStart = seek_read_position - trackSamples; - crossFadeSamples -= crossFadeStart; - } - } - - if (crossFadeSamples > 0) { - const auto readResult = m_pReader->read(seek_read_position + - (in_reverse ? crossFadeStart : -crossFadeStart), - crossFadeSamples, - in_reverse, + if (plan.crossFadeSamples > 0) { + const auto readResult = m_pReader->read(plan.crossFadeReadPosition, + plan.crossFadeSamples, + plan.inReverse, m_pCrossFadeBuffer, channelCount); if (readResult == CachingReader::ReadResult::UNAVAILABLE) { qDebug() << "ERROR: Couldn't get all needed samples for crossfade."; // Cache miss - no samples written - SampleUtil::clear(m_pCrossFadeBuffer, samples_from_reader); + SampleUtil::clear( + m_pCrossFadeBuffer, plan.samplesFromReader); // Set the cache miss flag to decide when to apply ramping // after the following read attempts. m_cacheMissCount++; } // do crossfade from the current buffer into the new loop beginning - if (samples_from_reader != 0) { // avoid division by zero - SampleUtil::linearCrossfadeBuffersOut( - pOutput + SampleUtil::ceilPlayPosToFrameStart(crossFadeStart, channelCount), + if (plan.samplesFromReader != 0) { // avoid division by zero + SampleUtil::linearCrossfadeBuffersOut(pOutput + + SampleUtil::ceilPlayPosToFrameStart( + plan.crossFadeStart, channelCount), m_pCrossFadeBuffer, - crossFadeSamples, + plan.crossFadeSamples, channelCount); } } else { @@ -261,12 +321,16 @@ SINT ReadAheadManager::getNextSamples(double dRate, SampleUtil::applyRampingGain(pOutput, CSAMPLE_GAIN_ONE, CSAMPLE_GAIN_ZERO, - samples_from_reader); + plan.samplesFromReader); } } - // qDebug() << "read" << m_currentPosition << samples_from_reader; - return samples_from_reader; + if (retryOnCacheMiss) { + cancelPendingRetry(); + } + + // qDebug() << "read" << m_currentPosition << plan.samplesFromReader; + return {plan.samplesFromReader, false}; } void ReadAheadManager::addRateControl(RateControl* pRateControl) { @@ -275,6 +339,7 @@ void ReadAheadManager::addRateControl(RateControl* pRateControl) { // Not thread-save, call from engine thread only void ReadAheadManager::notifySeek(double seekPosition) { + cancelPendingRetry(); m_currentPosition = seekPosition; m_cacheMissCount = 0; m_cacheMissExpected = true; diff --git a/src/engine/readaheadmanager.h b/src/engine/readaheadmanager.h index 36db44263116..c2174771644d 100644 --- a/src/engine/readaheadmanager.h +++ b/src/engine/readaheadmanager.h @@ -23,6 +23,11 @@ class RateControl; /// point. class ReadAheadManager { public: + struct NextSamplesResult { + SINT samplesRead; + bool retryPending; + }; + ReadAheadManager(); // Only for testing: ReadAheadManagerMock ReadAheadManager(CachingReader* reader, LoopingControl* pLoopingControl, @@ -39,6 +44,20 @@ class ReadAheadManager { SINT requested_samples, mixxx::audio::ChannelCount channelCount); + /// Like getNextSamples(), but leave the read-ahead position unchanged when + /// the reader reports a cache miss. This is used by grain-based scalers + /// that must retry the exact same input range instead of analysing silence + /// as real input. + virtual NextSamplesResult getNextSamplesWithRetry(double dRate, + CSAMPLE* buffer, + SINT requested_samples, + mixxx::audio::ChannelCount channelCount); + + /// Discard a retryable read plan that can no longer be completed by the + /// caller. The next retryable request will query the loop and cue controls + /// again and create a new plan. + virtual void cancelPendingRetry(); + /// Used to add a new EngineControls that ReadAheadManager will use to decide /// which samples to return. void addLoopingControl(); @@ -69,6 +88,51 @@ class ReadAheadManager { mixxx::audio::ChannelCount channelCount); private: + struct ReadPlan { + bool active{false}; + bool inReverse{false}; + bool reachedTrigger{false}; + double requestPosition{0.0}; + double target{0.0}; + double positionAfterTrigger{0.0}; + double samplesToSeekTrigger{0.0}; + SINT requestSamples{0}; + SINT requestedSamples{0}; + SINT samplesFromReader{0}; + SINT preseekSamples{0}; + SINT startSample{0}; + int seekReadPosition{0}; + int crossFadeStart{0}; + int crossFadeSamples{0}; + int crossFadeReadPosition{0}; + mixxx::audio::ChannelCount channelCount; + mixxx::audio::FramePos loopTriggerPosition; + mixxx::audio::FramePos loopTargetPosition; + mixxx::audio::FramePos jumpTriggerPosition; + mixxx::audio::FramePos jumpTargetPosition; + mixxx::audio::FramePos targetPosition; + + bool matches(double position, + bool reverse, + SINT samples, + mixxx::audio::ChannelCount channels) const { + return active && requestPosition == position && + inReverse == reverse && requestSamples == samples && + channelCount.value() == channels.value(); + } + }; + + ReadPlan makeReadPlan(bool inReverse, + SINT requestSamples, + SINT requestedSamples, + mixxx::audio::ChannelCount channelCount); + + NextSamplesResult getNextSamplesInternal(double dRate, + CSAMPLE* buffer, + SINT requested_samples, + mixxx::audio::ChannelCount channelCount, + bool retryOnCacheMiss); + /// An entry in the read log indicates the virtual playposition the read /// began at and the virtual playposition it ended at. struct ReadLogEntry { @@ -133,4 +197,5 @@ class ReadAheadManager { CSAMPLE* m_pCrossFadeBuffer; int m_cacheMissCount; bool m_cacheMissExpected; + ReadPlan m_pendingRetry; }; diff --git a/src/test/readaheadmanager_test.cpp b/src/test/readaheadmanager_test.cpp index ece027526603..c30f0b5d7c7d 100644 --- a/src/test/readaheadmanager_test.cpp +++ b/src/test/readaheadmanager_test.cpp @@ -4,9 +4,12 @@ #include #include +#include +#include #include "control/controlobject.h" #include "engine/cachingreader/cachingreader.h" +#include "engine/cachingreader/cachingreaderchunk.h" #include "engine/controls/cuecontrol.h" #include "engine/controls/loopingcontrol.h" #include "test/mixxxtest.h" @@ -29,14 +32,183 @@ class StubReader : public CachingReader { bool reverse, CSAMPLE* buffer, mixxx::audio::ChannelCount channelCount) override { - Q_UNUSED(startSample); Q_UNUSED(reverse); Q_UNUSED(channelCount); + m_readStartSamples.push_back(startSample); + if (!m_readAvailable) { + return CachingReader::ReadResult::UNAVAILABLE; + } + SampleUtil::clear(buffer, numSamples); + return CachingReader::ReadResult::AVAILABLE; + } + + void setReadAvailable(bool available) { + m_readAvailable = available; + } + + const QList& readStartSamples() const { + return m_readStartSamples; + } + + protected: + RetryReadResult readWithRetryHook(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount) override { + Q_UNUSED(reverse); + Q_UNUSED(channelCount); + m_readStartSamples.push_back(startSample); + if (!m_readAvailable) { + return {CachingReader::ReadResult::UNAVAILABLE, true}; + } SampleUtil::clear(buffer, numSamples); + return {CachingReader::ReadResult::AVAILABLE, false}; + } + + private: + bool m_readAvailable{true}; + QList m_readStartSamples; +}; + +class ChunkBoundaryRetryReader : public CachingReader { + public: + ChunkBoundaryRetryReader() + : CachingReader( + kGroup, UserSettingsPointer(), mixxx::audio::ChannelCount::stereo()) { + } + + CachingReader::ReadResult read(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount) override { + Q_UNUSED(reverse); + Q_UNUSED(channelCount); + m_readStartSamples.push_back(startSample); + + const SINT boundarySample = CachingReaderChunk::frames2samples( + CachingReaderChunk::kFrames, + mixxx::audio::ChannelCount::stereo()); + if (!m_laterChunkReady) { + const SINT prefixSamples = std::clamp( + boundarySample - startSample, SINT{0}, numSamples); + SampleUtil::fill(buffer, 0.25f, prefixSamples); + return CachingReader::ReadResult::PARTIALLY_AVAILABLE; + } + + for (SINT i = 0; i < numSamples; ++i) { + buffer[i] = sampleForAbsolutePosition(startSample + i); + } return CachingReader::ReadResult::AVAILABLE; } + + void setLaterChunkReady() { + m_laterChunkReady = true; + } + + const QList& readStartSamples() const { + return m_readStartSamples; + } + + static CSAMPLE sampleForAbsolutePosition(SINT sample) { + return static_cast((sample % 101) / 100.0f); + } + + protected: + RetryReadResult readWithRetryHook(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount) override { + const auto result = read( + startSample, numSamples, reverse, buffer, channelCount); + return {result, !m_laterChunkReady}; + } + + private: + bool m_laterChunkReady{false}; + QList m_readStartSamples; +}; + +class LegacyPartialReader : public CachingReader { + public: + LegacyPartialReader() + : CachingReader( + kGroup, UserSettingsPointer(), mixxx::audio::ChannelCount::stereo()) { + } + + CachingReader::ReadResult read(SINT startSample, + SINT numSamples, + bool reverse, + CSAMPLE* buffer, + mixxx::audio::ChannelCount channelCount) override { + Q_UNUSED(startSample); + Q_UNUSED(reverse); + Q_UNUSED(channelCount); + SampleUtil::clear(buffer, numSamples); + return CachingReader::ReadResult::PARTIALLY_AVAILABLE; + } }; +TEST(CachingReaderRetryTest, + LateChunkMissAcrossChunkBoundaryLeavesDestinationUntouched) { + constexpr auto kChannelCount = mixxx::audio::ChannelCount::stereo(); + constexpr SINT kStartFrame = CachingReaderChunk::kFrames - 2; + constexpr SINT kFrameCount = 4; + constexpr SINT kSampleCount = kFrameCount * 2; + constexpr CSAMPLE kSentinel = -0.75f; + const SINT startSample = CachingReaderChunk::frames2samples( + kStartFrame, kChannelCount); + + ChunkBoundaryRetryReader reader; + std::array buffer; + buffer.fill(kSentinel); + + EXPECT_EQ(CachingReader::ReadResult::UNAVAILABLE, + reader.readWithRetry(startSample, + kSampleCount, + false, + buffer.data(), + kChannelCount)); + for (const auto sample : buffer) { + EXPECT_FLOAT_EQ(kSentinel, sample); + } + + reader.setLaterChunkReady(); + EXPECT_EQ(CachingReader::ReadResult::AVAILABLE, + reader.readWithRetry(startSample, + kSampleCount, + false, + buffer.data(), + kChannelCount)); + for (SINT i = 0; i < kSampleCount; ++i) { + EXPECT_FLOAT_EQ( + ChunkBoundaryRetryReader::sampleForAbsolutePosition(startSample + i), + buffer[i]); + } + ASSERT_EQ(2, reader.readStartSamples().size()); + EXPECT_EQ(startSample, reader.readStartSamples()[0]); + EXPECT_EQ(startSample, reader.readStartSamples()[1]); +} + +TEST(CachingReaderRetryTest, LegacyPartialReadIsAcceptedAsIntentionalPadding) { + constexpr auto kChannelCount = mixxx::audio::ChannelCount::stereo(); + std::array buffer; + buffer.fill(1.0f); + + LegacyPartialReader reader; + EXPECT_EQ(CachingReader::ReadResult::PARTIALLY_AVAILABLE, + reader.readWithRetry(0, + static_cast(buffer.size()), + false, + buffer.data(), + kChannelCount)); + EXPECT_TRUE(std::all_of(buffer.begin(), buffer.end(), [](CSAMPLE sample) { + return sample == 0.0f; + })); +} + class StubLoopControl : public LoopingControl { public: StubLoopControl() @@ -50,12 +222,21 @@ class StubLoopControl : public LoopingControl { mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(target)); } + int queryCount() const { + return m_queryCount; + } + + int pendingPlanCount() const { + return m_triggerReturnValues.size(); + } + mixxx::audio::FramePos nextTrigger(bool reverse, mixxx::audio::FramePos currentPosition, mixxx::audio::FramePos* pTargetPosition) override { Q_UNUSED(reverse); Q_UNUSED(currentPosition); Q_UNUSED(pTargetPosition); + ++m_queryCount; RELEASE_ASSERT(!m_targetReturnValues.isEmpty()); *pTargetPosition = m_targetReturnValues.takeFirst(); RELEASE_ASSERT(!m_triggerReturnValues.isEmpty()); @@ -65,6 +246,7 @@ class StubLoopControl : public LoopingControl { protected: QList m_triggerReturnValues; QList m_targetReturnValues; + int m_queryCount{0}; }; class StubCueControl : public CueControl { @@ -81,10 +263,19 @@ class StubCueControl : public CueControl { mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(target)); } + int queryCount() const { + return m_queryCount; + } + + int pendingPlanCount() const { + return m_triggerReturnValues.size(); + } + mixxx::audio::FramePos nextTrigger(bool, mixxx::audio::FramePos, mixxx::audio::FramePos* pTargetPosition, mixxx::audio::FrameDiff_t) override { + ++m_queryCount; RELEASE_ASSERT(!m_targetReturnValues.isEmpty()); *pTargetPosition = m_targetReturnValues.takeFirst(); RELEASE_ASSERT(!m_triggerReturnValues.isEmpty()); @@ -94,6 +285,7 @@ class StubCueControl : public CueControl { protected: QList m_triggerReturnValues; QList m_targetReturnValues; + int m_queryCount{0}; }; class ReadAheadManagerTest : public MixxxTest { @@ -169,6 +361,98 @@ TEST_F(ReadAheadManagerTest, SavedJump) { EXPECT_NEAR(86.5, m_pReadAheadManager->getPlaypos(), 1); } +TEST_F(ReadAheadManagerTest, RetryableCacheMissDoesNotAdvanceReadAheadPosition) { + m_pReadAheadManager->notifySeek(0); + m_pReader->setReadAvailable(false); + m_pLoopControl->pushValues(kNoTrigger, kNoTrigger); + m_pCueControl->pushValues(kNoTrigger, kNoTrigger); + + const auto unavailableResult = m_pReadAheadManager->getNextSamplesWithRetry( + 1.0, m_pBuffer, 10, mixxx::audio::ChannelCount::stereo()); + EXPECT_EQ(0, unavailableResult.samplesRead); + EXPECT_TRUE(unavailableResult.retryPending); + EXPECT_DOUBLE_EQ(0.0, m_pReadAheadManager->getPlaypos()); + + m_pReader->setReadAvailable(true); + + const auto availableResult = m_pReadAheadManager->getNextSamplesWithRetry( + 1.0, m_pBuffer, 10, mixxx::audio::ChannelCount::stereo()); + EXPECT_EQ(10, availableResult.samplesRead); + EXPECT_FALSE(availableResult.retryPending); + EXPECT_DOUBLE_EQ(10.0, m_pReadAheadManager->getPlaypos()); + ASSERT_EQ(2, m_pReader->readStartSamples().size()); + EXPECT_EQ(0, m_pReader->readStartSamples()[0]); + EXPECT_EQ(0, m_pReader->readStartSamples()[1]); + EXPECT_EQ(1, m_pLoopControl->queryCount()); + EXPECT_EQ(1, m_pCueControl->queryCount()); +} + +TEST_F(ReadAheadManagerTest, RetryableCacheMissRetainsStatefulTriggerPlan) { + m_pReadAheadManager->notifySeek(0); + m_pReader->setReadAvailable(false); + m_pLoopControl->pushValues(8, 2); + m_pCueControl->pushValues(6, 4); + // These plans must remain untouched while the first request is retried. + m_pLoopControl->pushValues(kNoTrigger, kNoTrigger); + m_pCueControl->pushValues(kNoTrigger, kNoTrigger); + std::array output; + output.fill(1.0f); + + const auto unavailableResult = m_pReadAheadManager->getNextSamplesWithRetry( + 1.0, output.data(), output.size(), mixxx::audio::ChannelCount::stereo()); + EXPECT_EQ(0, unavailableResult.samplesRead); + EXPECT_TRUE(unavailableResult.retryPending); + EXPECT_TRUE(std::all_of(output.begin(), output.end(), [](CSAMPLE sample) { + return sample == 0.0f; + })); + EXPECT_EQ(1, m_pLoopControl->queryCount()); + EXPECT_EQ(1, m_pCueControl->queryCount()); + + m_pReader->setReadAvailable(true); + const auto availableResult = m_pReadAheadManager->getNextSamplesWithRetry( + 1.0, output.data(), output.size(), mixxx::audio::ChannelCount::stereo()); + + EXPECT_EQ(6, availableResult.samplesRead); + EXPECT_FALSE(availableResult.retryPending); + EXPECT_DOUBLE_EQ(4.0, m_pReadAheadManager->getPlaypos()); + EXPECT_EQ(1, m_pLoopControl->queryCount()); + EXPECT_EQ(1, m_pCueControl->queryCount()); + EXPECT_EQ(1, m_pLoopControl->pendingPlanCount()); + EXPECT_EQ(1, m_pCueControl->pendingPlanCount()); + ASSERT_GE(m_pReader->readStartSamples().size(), 2); + EXPECT_EQ(0, m_pReader->readStartSamples()[0]); + EXPECT_EQ(0, m_pReader->readStartSamples()[1]); +} + +TEST_F(ReadAheadManagerTest, NotifySeekCancelsPendingTriggerPlan) { + m_pReadAheadManager->notifySeek(0); + m_pReader->setReadAvailable(false); + m_pLoopControl->pushValues(8, 2); + m_pCueControl->pushValues(6, 4); + m_pLoopControl->pushValues(kNoTrigger, kNoTrigger); + m_pCueControl->pushValues(kNoTrigger, kNoTrigger); + + const auto unavailableResult = m_pReadAheadManager->getNextSamplesWithRetry( + 1.0, m_pBuffer, 10, mixxx::audio::ChannelCount::stereo()); + ASSERT_TRUE(unavailableResult.retryPending); + + m_pReadAheadManager->notifySeek(20); + m_pReader->setReadAvailable(true); + const auto availableResult = m_pReadAheadManager->getNextSamplesWithRetry( + 1.0, m_pBuffer, 10, mixxx::audio::ChannelCount::stereo()); + + EXPECT_EQ(10, availableResult.samplesRead); + EXPECT_FALSE(availableResult.retryPending); + EXPECT_DOUBLE_EQ(30.0, m_pReadAheadManager->getPlaypos()); + EXPECT_EQ(2, m_pLoopControl->queryCount()); + EXPECT_EQ(2, m_pCueControl->queryCount()); + EXPECT_EQ(0, m_pLoopControl->pendingPlanCount()); + EXPECT_EQ(0, m_pCueControl->pendingPlanCount()); + ASSERT_GE(m_pReader->readStartSamples().size(), 2); + EXPECT_EQ(0, m_pReader->readStartSamples()[0]); + EXPECT_EQ(20, m_pReader->readStartSamples()[1]); +} + TEST_F(ReadAheadManagerTest, TriggerOnJumpOrLoop) { m_pReadAheadManager->notifySeek(0); From 8c5febfe346c945564ab01262706f1551ccccdc5 Mon Sep 17 00:00:00 2001 From: 0cwa Date: Thu, 17 Sep 2026 02:54:31 +0200 Subject: [PATCH 2/2] Fix signed retry buffer comparison --- src/engine/cachingreader/cachingreader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/cachingreader/cachingreader.cpp b/src/engine/cachingreader/cachingreader.cpp index 69ff0129ae1d..be245b2e17f7 100644 --- a/src/engine/cachingreader/cachingreader.cpp +++ b/src/engine/cachingreader/cachingreader.cpp @@ -321,7 +321,7 @@ CachingReader::ReadResult CachingReader::readWithRetry(SINT startSample, CSAMPLE* buffer, mixxx::audio::ChannelCount channelCount) { VERIFY_OR_DEBUG_ASSERT(numSamples >= 0 && - static_cast(numSamples) <= m_retryReadBuffer.size()) { + numSamples <= m_retryReadBuffer.size()) { return ReadResult::UNAVAILABLE; } VERIFY_OR_DEBUG_ASSERT(buffer) {