Skip to content

Commit cd76eba

Browse files
authored
Merge pull request #16054 from daschuer/valrind_firstsound
Fix false positive "First sound has been moved!" warnings
2 parents 6c44d77 + 386bf5c commit cd76eba

1 file changed

Lines changed: 25 additions & 14 deletions

File tree

src/engine/cachingreader/cachingreaderworker.cpp

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,19 @@ ReaderStatusUpdate CachingReaderWorker::processReadRequest(
7272
}
7373
}
7474

75-
// This call here assumes that the caching reader will read the first sound cue at
76-
// one of the first chunks. The check serves as a sanity check to ensure that the
77-
// sample data has not changed since it has ben analyzed. This could happen because
78-
// of a change in actual audio data or because the file was decoded using a different
79-
// decoder
80-
// This is part of a first prove of concept and needs to be replaces with a different
81-
// solution which is still under discussion. This might be also extended
82-
// to further checks whether a automatic offset adjustment is possible or a the
83-
// sample position metadata shall be treated as outdated.
84-
// Failures of the sanity check only result in an entry into the log at the moment.
85-
verifyFirstSound(pChunk);
75+
if (status == CHUNK_READ_SUCCESS) {
76+
// This call here assumes that the caching reader will read the first sound cue at
77+
// one of the first chunks. The check serves as a sanity check to ensure that the
78+
// sample data has not changed since it has ben analyzed. This could happen because
79+
// of a change in actual audio data or because the file was decoded using a different
80+
// decoder
81+
// This is part of a first prove of concept and needs to be replaces with a different
82+
// solution which is still under discussion. This might be also extended
83+
// to further checks whether a automatic offset adjustment is possible or a the
84+
// sample position metadata shall be treated as outdated.
85+
// Failures of the sanity check only result in an entry into the log at the moment.
86+
verifyFirstSound(pChunk);
87+
}
8688

8789
ReaderStatusUpdate result;
8890
result.init(status, pChunk, m_pAudioSource ? m_pAudioSource->frameIndexRange() : mixxx::IndexRange());
@@ -272,9 +274,18 @@ void CachingReaderWorker::verifyFirstSound(const CachingReaderChunk* pChunk) {
272274
.value()));
273275
if (pChunk->getIndex() == firstSoundIndex) {
274276
CSAMPLE sampleBuffer[kNumSoundFrameToVerify * mixxx::kEngineChannelCount];
275-
SINT end = static_cast<SINT>(m_firstSoundFrameToVerify.toLowerFrameBoundary().value());
276-
pChunk->readBufferedSampleFrames(sampleBuffer,
277-
mixxx::IndexRange::forward(end - 1, kNumSoundFrameToVerify));
277+
// We read two frames, the last silence frame and the first non-silence frame from
278+
// m_firstSoundFrameToVerify. end points to one position after them.
279+
SINT end = static_cast<SINT>(m_firstSoundFrameToVerify.toLowerFrameBoundary().value()) + 1;
280+
mixxx::IndexRange probeFrameIndexRange =
281+
mixxx::IndexRange::between(end - kNumSoundFrameToVerify, end);
282+
mixxx::IndexRange bufferedFrameIndexRange =
283+
pChunk->readBufferedSampleFrames(
284+
sampleBuffer, probeFrameIndexRange);
285+
VERIFY_OR_DEBUG_ASSERT(bufferedFrameIndexRange == probeFrameIndexRange) {
286+
qWarning() << "skipping verifyFirstSound()";
287+
return;
288+
}
278289
if (AnalyzerSilence::verifyFirstSound(std::span<const CSAMPLE>(sampleBuffer),
279290
mixxx::audio::FramePos(1))) {
280291
qDebug() << "First sound found at the previously stored position";

0 commit comments

Comments
 (0)