Skip to content

Commit

Permalink
Merge pull request #3220 from daschuer/indexrangimprove
Browse files Browse the repository at this point in the history
IndexRange improvements
  • Loading branch information
uklotzde committed Nov 6, 2020
2 parents 0cc2aee + dda4302 commit 18ea1f0
Show file tree
Hide file tree
Showing 13 changed files with 62 additions and 160 deletions.
7 changes: 4 additions & 3 deletions src/analyzer/analyzerthread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ AnalyzerThread::AnalysisResult AnalyzerThread::analyzeAudioSource(
chunkFrameRange,
mixxx::SampleBuffer::WritableSlice(m_sampleBuffer)));
// The returned range fits into the requested range
DEBUG_ASSERT(readableSampleFrames.frameIndexRange() <= chunkFrameRange);
DEBUG_ASSERT(readableSampleFrames.frameIndexRange().isSubrangeOf(chunkFrameRange));

// Sometimes the duration of the audio source is inaccurate and adjusted
// while reading. We need to adjust all frame ranges to reflect this new
Expand All @@ -265,7 +265,7 @@ AnalyzerThread::AnalysisResult AnalyzerThread::analyzeAudioSource(
chunkFrameRange = intersect(chunkFrameRange, audioSourceProxy.frameIndexRange());
// The audio data that has just been read should still fit into the adjusted
// chunk range.
DEBUG_ASSERT(readableSampleFrames.frameIndexRange() <= chunkFrameRange);
DEBUG_ASSERT(readableSampleFrames.frameIndexRange().isSubrangeOf(chunkFrameRange));

// We also need to adjust the remaining frame range for the next requests.
remainingFrameRange = intersect(remainingFrameRange, audioSourceProxy.frameIndexRange());
Expand All @@ -277,7 +277,8 @@ AnalyzerThread::AnalysisResult AnalyzerThread::analyzeAudioSource(
// If we have read an incomplete chunk while the range has grown
// we need to discard the read results and re-read the current
// chunk!
remainingFrameRange = span(remainingFrameRange, chunkFrameRange);

remainingFrameRange.growFront(chunkFrameRange.length());
continue;
}
DEBUG_ASSERT(remainingFrameRange.end() < audioSourceProxy.frameIndexRange().end());
Expand Down
2 changes: 1 addition & 1 deletion src/engine/cachingreader/cachingreader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ CachingReader::ReadResult CachingReader::read(SINT startSample, SINT numSamples,
// finally fill the remaining buffer with silence.
break;
}
DEBUG_ASSERT(bufferedFrameIndexRange <= remainingFrameIndexRange);
DEBUG_ASSERT(bufferedFrameIndexRange.isSubrangeOf(remainingFrameIndexRange));
if (remainingFrameIndexRange.start() < bufferedFrameIndexRange.start()) {
const auto paddingFrameIndexRange =
mixxx::IndexRange::between(
Expand Down
2 changes: 1 addition & 1 deletion src/engine/cachingreader/cachingreaderchunk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ mixxx::IndexRange CachingReaderChunk::bufferSampleFrames(
sourceFrameIndexRange,
mixxx::SampleBuffer::WritableSlice(m_sampleBuffer)));
DEBUG_ASSERT(m_bufferedSampleFrames.frameIndexRange().empty() ||
m_bufferedSampleFrames.frameIndexRange() <= sourceFrameIndexRange);
m_bufferedSampleFrames.frameIndexRange().isSubrangeOf(sourceFrameIndexRange));
return m_bufferedSampleFrames.frameIndexRange();
}

Expand Down
6 changes: 3 additions & 3 deletions src/engine/cachingreader/cachingreaderworker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ ReaderStatusUpdate CachingReaderWorker::processReadRequest(
// actually available.
auto chunkFrameIndexRange = pChunk->frameIndexRange(m_pAudioSource);
DEBUG_ASSERT(!m_pAudioSource ||
chunkFrameIndexRange <= m_pAudioSource->frameIndexRange());
chunkFrameIndexRange.isSubrangeOf(m_pAudioSource->frameIndexRange()));
if (chunkFrameIndexRange.empty()) {
ReaderStatusUpdate result;
result.init(CHUNK_READ_INVALID, pChunk, m_pAudioSource ? m_pAudioSource->frameIndexRange() : mixxx::IndexRange());
Expand All @@ -51,11 +51,11 @@ ReaderStatusUpdate CachingReaderWorker::processReadRequest(
m_pAudioSource,
mixxx::SampleBuffer::WritableSlice(m_tempReadBuffer));
DEBUG_ASSERT(!m_pAudioSource ||
bufferedFrameIndexRange <= m_pAudioSource->frameIndexRange());
bufferedFrameIndexRange.isSubrangeOf(m_pAudioSource->frameIndexRange()));
// The readable frame range might have changed
chunkFrameIndexRange = intersect(chunkFrameIndexRange, m_pAudioSource->frameIndexRange());
DEBUG_ASSERT(bufferedFrameIndexRange.empty() ||
bufferedFrameIndexRange <= chunkFrameIndexRange);
bufferedFrameIndexRange.isSubrangeOf(chunkFrameIndexRange));

ReaderStatus status = bufferedFrameIndexRange.empty() ? CHUNK_READ_EOF : CHUNK_READ_SUCCESS;
if (bufferedFrameIndexRange != chunkFrameIndexRange) {
Expand Down
19 changes: 10 additions & 9 deletions src/sources/audiosource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,10 @@ bool AudioSource::verifyReadable() {
frameIndexRange().splitAndShrinkFront(numSampleFrames),
SampleBuffer::WritableSlice(sampleBuffer));
auto readableSampleFrames = readSampleFrames(writableSampleFrames);
DEBUG_ASSERT(
readableSampleFrames.frameIndexRange() <=
writableSampleFrames.frameIndexRange());
if (readableSampleFrames.frameIndexRange() <
writableSampleFrames.frameIndexRange()) {
DEBUG_ASSERT(readableSampleFrames.frameIndexRange().isSubrangeOf(
writableSampleFrames.frameIndexRange()));
if (readableSampleFrames.frameIndexRange().length() <
writableSampleFrames.frameIndexRange().length()) {
kLogger.warning()
<< "Read test failed:"
<< "expected ="
Expand All @@ -220,7 +219,8 @@ bool AudioSource::verifyReadable() {
std::optional<WritableSampleFrames> AudioSource::clampWritableSampleFrames(
WritableSampleFrames sampleFrames) const {
const auto clampedFrameIndexRange =
clampFrameIndexRange(sampleFrames.frameIndexRange());
intersect2(sampleFrames.frameIndexRange(), frameIndexRange());

if (!clampedFrameIndexRange) {
return std::nullopt;
}
Expand Down Expand Up @@ -293,7 +293,7 @@ ReadableSampleFrames AudioSource::readSampleFrames(
// forward clamped request
ReadableSampleFrames readable = readSampleFramesClamped(writable);
DEBUG_ASSERT(readable.frameIndexRange().empty() ||
readable.frameIndexRange() <= writable.frameIndexRange());
readable.frameIndexRange().isSubrangeOf(writable.frameIndexRange()));
if (readable.frameIndexRange() != writable.frameIndexRange()) {
kLogger.warning()
<< "Failed to read sample frames:"
Expand Down Expand Up @@ -322,7 +322,8 @@ ReadableSampleFrames AudioSource::readSampleFrames(
readable.frameIndexRange().end());
}
}
DEBUG_ASSERT(shrinkedFrameIndexRange < m_frameIndexRange);
DEBUG_ASSERT(shrinkedFrameIndexRange.isSubrangeOf(m_frameIndexRange) &&
shrinkedFrameIndexRange.length() < m_frameIndexRange.length());
kLogger.info()
<< "Shrinking readable frame index range:"
<< "before =" << m_frameIndexRange
Expand All @@ -341,7 +342,7 @@ ReadableSampleFrames AudioSource::readSampleFrames(

void AudioSource::adjustFrameIndexRange(
IndexRange frameIndexRange) {
DEBUG_ASSERT(frameIndexRange <= m_frameIndexRange);
DEBUG_ASSERT(frameIndexRange.isSubrangeOf(m_frameIndexRange));
m_frameIndexRange = frameIndexRange;
}

Expand Down
4 changes: 0 additions & 4 deletions src/sources/audiosource.h
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,6 @@ class AudioSource : public UrlResource, public virtual /*implements*/ IAudioSour

std::optional<WritableSampleFrames> clampWritableSampleFrames(
WritableSampleFrames sampleFrames) const;
std::optional<IndexRange> clampFrameIndexRange(
IndexRange frameIndexRange) const {
return intersect2(frameIndexRange, this->frameIndexRange());
}

audio::SignalInfo m_signalInfo;

Expand Down
3 changes: 1 addition & 2 deletions src/sources/audiosourcestereoproxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ ReadableSampleFrames AudioSourceStereoProxy::readSampleFramesClamped(
return readableSampleFrames;
}
DEBUG_ASSERT(
readableSampleFrames.frameIndexRange() <=
sampleFrames.frameIndexRange());
readableSampleFrames.frameIndexRange().isSubrangeOf(sampleFrames.frameIndexRange()));
DEBUG_ASSERT(
readableSampleFrames.frameIndexRange().start() >=
sampleFrames.frameIndexRange().start());
Expand Down
2 changes: 1 addition & 1 deletion src/sources/readaheadframebuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ WritableSampleFrames ReadAheadFrameBuffer::drainBuffer(
return outputBuffer;
}
const auto consumableRange = intersect2(bufferedRange(), outputRange);
DEBUG_ASSERT(!consumableRange || *consumableRange <= outputRange);
DEBUG_ASSERT(!consumableRange || consumableRange->isSubrangeOf(outputRange));
if (!consumableRange || consumableRange->empty()) {
// No overlap between buffer and requested data
return outputBuffer;
Expand Down
2 changes: 1 addition & 1 deletion src/sources/soundsourceffmpeg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ ReadableSampleFrames SoundSourceFFmpeg::readSampleFramesClamped(

// Skip decoding if all data has been read
auto writableFrameRange = writableSampleFrames.frameIndexRange();
DEBUG_ASSERT(writableFrameRange <= frameIndexRange());
DEBUG_ASSERT(writableFrameRange.isSubrangeOf(frameIndexRange()));
if (writableFrameRange.empty()) {
auto readableRange = IndexRange::between(
readableStartIndex, writableFrameRange.start());
Expand Down
75 changes: 16 additions & 59 deletions src/test/indexrange_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,50 +166,26 @@ TEST_F(IndexRangeTest, equal) {
EXPECT_FALSE(IndexRange::forward(-1, 3) != IndexRange::between(-1, 2));
EXPECT_TRUE(IndexRange::backward(-1, 3) == IndexRange::between(-1, -4));
EXPECT_FALSE(IndexRange::backward(-1, 3) != IndexRange::between(-1, -4));
EXPECT_FALSE(IndexRange::between(-1, 3) == reverse(IndexRange::between(-1, 3)));
EXPECT_TRUE(IndexRange::between(-1, 3) != reverse(IndexRange::between(-1, 3)));
EXPECT_FALSE(IndexRange::between(0, 0) == IndexRange::between(1, 1));
EXPECT_TRUE(IndexRange::between(0, 0) != IndexRange::between(1, 1));
}

TEST_F(IndexRangeTest, lessOrEqual) {
EXPECT_FALSE(IndexRange::between(3, 3) <= IndexRange::between(-2, 2));
EXPECT_TRUE(IndexRange::between(0, 0) <= IndexRange::between(2, -1));
EXPECT_TRUE(IndexRange::between(2, 2) <= IndexRange::between(2, -1));
EXPECT_FALSE(IndexRange::between(-2, -2) <= IndexRange::between(2, -1));
EXPECT_TRUE(IndexRange::between(-2, 1) <= IndexRange::between(-2, 2));
EXPECT_TRUE(IndexRange::between(1, -2) <= IndexRange::between(2, -2));
EXPECT_FALSE(IndexRange::between(-2, 1) <= IndexRange::between(-1, 2));
EXPECT_FALSE(IndexRange::between(1, -2) <= IndexRange::between(2, -1));
EXPECT_FALSE(IndexRange::between(-2, 1) <= IndexRange::between(0, 1));
EXPECT_FALSE(IndexRange::between(-2, 1) <= IndexRange::between(1, 2));
EXPECT_FALSE(IndexRange::between(-2, 1) <= IndexRange::between(1, 1));
EXPECT_FALSE(IndexRange::between(3, 3) <= IndexRange::between(-2, 1));
EXPECT_FALSE(IndexRange::between(3, 3) <= IndexRange::between(1, -2));
EXPECT_TRUE(IndexRange::between(1, 1) <= IndexRange::between(-2, 1));
EXPECT_TRUE(IndexRange::between(1, 1) <= IndexRange::between(1, -2));
}

TEST_F(IndexRangeTest, greaterOrEqual) {
EXPECT_TRUE(IndexRange::between(-2, 2) >= IndexRange());
EXPECT_TRUE(IndexRange::between(-2, 2) >= IndexRange::between(-2, 1));
EXPECT_FALSE(IndexRange::between(-1, 2) >= IndexRange::between(-2, 1));
EXPECT_FALSE(IndexRange::between(2, -1) >= IndexRange::between(1, -2));
EXPECT_FALSE(IndexRange::between(0, 1) >= IndexRange::between(-2, 1));
EXPECT_FALSE(IndexRange::between(1, 2) >= IndexRange::between(-2, 1));
EXPECT_FALSE(IndexRange::between(1, 1) >= IndexRange::between(-2, 1));
EXPECT_FALSE(IndexRange::between(-2, 1) >= IndexRange::between(3, 3));
EXPECT_FALSE(IndexRange::between(1, -2) >= IndexRange::between(3, 3));
EXPECT_TRUE(IndexRange::between(-2, 1) >= IndexRange::between(1, 1));
EXPECT_TRUE(IndexRange::between(1, -2) >= IndexRange::between(1, 1));
}

TEST_F(IndexRangeTest, reverse) {
EXPECT_EQ(IndexRange(), reverse(IndexRange()));
EXPECT_EQ(IndexRange::between(1, 1), reverse(IndexRange::between(1, 1)));
EXPECT_EQ(IndexRange::between(-1, -1), reverse(IndexRange::between(-1, -1)));
EXPECT_EQ(IndexRange::between(8, -5), reverse(IndexRange::between(-4, 9)));
EXPECT_EQ(IndexRange::between(-9, -3), reverse(IndexRange::between(-4, -10)));
TEST_F(IndexRangeTest, isSubrangeOf) {
EXPECT_FALSE(IndexRange::between(3, 3).isSubrangeOf(IndexRange::between(-2, 2)));
EXPECT_TRUE(IndexRange::between(0, 0).isSubrangeOf(IndexRange::between(2, -1)));
EXPECT_TRUE(IndexRange::between(2, 2).isSubrangeOf(IndexRange::between(2, -1)));
EXPECT_FALSE(IndexRange::between(-2, -2).isSubrangeOf(IndexRange::between(2, -1)));
EXPECT_TRUE(IndexRange::between(-2, 1).isSubrangeOf(IndexRange::between(-2, 2)));
EXPECT_TRUE(IndexRange::between(1, -2).isSubrangeOf(IndexRange::between(2, -2)));
EXPECT_FALSE(IndexRange::between(-2, 1).isSubrangeOf(IndexRange::between(-1, 2)));
EXPECT_FALSE(IndexRange::between(1, -2).isSubrangeOf(IndexRange::between(2, -1)));
EXPECT_FALSE(IndexRange::between(-2, 1).isSubrangeOf(IndexRange::between(0, 1)));
EXPECT_FALSE(IndexRange::between(-2, 1).isSubrangeOf(IndexRange::between(1, 2)));
EXPECT_FALSE(IndexRange::between(-2, 1).isSubrangeOf(IndexRange::between(1, 1)));
EXPECT_FALSE(IndexRange::between(3, 3).isSubrangeOf(IndexRange::between(-2, 1)));
EXPECT_FALSE(IndexRange::between(3, 3).isSubrangeOf(IndexRange::between(1, -2)));
EXPECT_TRUE(IndexRange::between(1, 1).isSubrangeOf(IndexRange::between(-2, 1)));
EXPECT_TRUE(IndexRange::between(1, 1).isSubrangeOf(IndexRange::between(1, -2)));
}

TEST_F(IndexRangeTest, intersect2) {
Expand Down Expand Up @@ -247,23 +223,4 @@ TEST_F(IndexRangeTest, intersect2) {
IndexRange::between(0, -1), IndexRange::between(-1, -1)));
}

TEST_F(IndexRangeTest, span) {
EXPECT_EQ(IndexRange::between(-1, 1), span(IndexRange::between(-1, 0), IndexRange::between(0, 1)));
EXPECT_EQ(IndexRange::between(-1, 1), span(IndexRange::between(-1, 1), IndexRange::between(-1, 1)));
EXPECT_EQ(IndexRange::between(-2, 2), span(IndexRange::between(-2, 1), IndexRange::between(-1, 2)));
EXPECT_EQ(IndexRange::between(-2, 2), span(IndexRange::between(-2, -1), IndexRange::between(1, 2)));
EXPECT_EQ(IndexRange::between(1, -1), span(IndexRange::between(0, -1), IndexRange::between(1, 0)));
EXPECT_EQ(IndexRange::between(1, -1), span(IndexRange::between(1, -1), IndexRange::between(1, -1)));
EXPECT_EQ(IndexRange::between(2, -2), span(IndexRange::between(1, -2), IndexRange::between(2, -1)));
EXPECT_EQ(IndexRange::between(2, -2), span(IndexRange::between(-1, -2), IndexRange::between(2, 1)));
EXPECT_EQ(IndexRange::between(-1, 1), span(IndexRange::between(-1, 1), IndexRange::between(0, 0)));
EXPECT_EQ(IndexRange::between(-1, 1), span(IndexRange::between(0, 0), IndexRange::between(-1, 1)));
EXPECT_EQ(IndexRange::between(-1, 1), span(IndexRange::between(-1, 1), IndexRange::between(1, 1)));
EXPECT_EQ(IndexRange::between(-1, 1), span(IndexRange::between(1, 1), IndexRange::between(-1, 1)));
EXPECT_EQ(IndexRange::between(-1, 1), span(IndexRange::between(-1, 1), IndexRange::between(-1, -1)));
EXPECT_EQ(IndexRange::between(-1, 1), span(IndexRange::between(-1, -1), IndexRange::between(-1, 1)));
EXPECT_EQ(IndexRange::between(3, -1), span(IndexRange::between(1, -1), IndexRange::between(3, 3)));
EXPECT_EQ(IndexRange::between(1, -3), span(IndexRange::between(-3, -3), IndexRange::between(1, -1)));
}

} // namespace mixxx
12 changes: 7 additions & 5 deletions src/test/soundproxy_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class SoundSourceProxyTest : public MixxxTest {
skipRange.length() - skippedRange.length(),
pAudioSource->getSignalInfo().samples2frames(m_skipSampleBuffer.size())));
EXPECT_FALSE(nextRange.empty());
EXPECT_TRUE(intersect(nextRange, skipRange) == nextRange);
EXPECT_TRUE(nextRange.isSubrangeOf(skipRange));
const auto readRange = pAudioSource->readSampleFrames(
mixxx::WritableSampleFrames(
nextRange,
Expand All @@ -145,11 +145,12 @@ class SoundSourceProxyTest : public MixxxTest {
return skippedRange;
}
EXPECT_TRUE(readRange.start() == nextRange.start());
EXPECT_TRUE(intersect(readRange, skipRange) == readRange);
EXPECT_TRUE(readRange.isSubrangeOf(skipRange));
if (skippedRange.empty()) {
skippedRange = readRange;
} else {
skippedRange = span(skippedRange, nextRange);
EXPECT_TRUE(skippedRange.end() == nextRange.start());
skippedRange.growBack(nextRange.length());
}
}
return skippedRange;
Expand Down Expand Up @@ -265,7 +266,7 @@ TEST_F(SoundSourceProxyTest, seekForwardBackward) {
readFrameIndexRange,
mixxx::SampleBuffer::WritableSlice(contReadData)));
ASSERT_FALSE(contSampleFrames.frameIndexRange().empty());
ASSERT_LE(contSampleFrames.frameIndexRange(), readFrameIndexRange);
ASSERT_TRUE(contSampleFrames.frameIndexRange().isSubrangeOf(readFrameIndexRange));
ASSERT_EQ(contSampleFrames.frameIndexRange().start(), readFrameIndexRange.start());
contFrameIndex += contSampleFrames.frameLength();

Expand Down Expand Up @@ -387,7 +388,8 @@ TEST_F(SoundSourceProxyTest, skipAndRead) {
readFrameIndexRange,
mixxx::SampleBuffer::WritableSlice(contReadData)));
ASSERT_FALSE(contSampleFrames.frameIndexRange().empty());
ASSERT_LE(contSampleFrames.frameIndexRange(), readFrameIndexRange);
ASSERT_TRUE(contSampleFrames.frameIndexRange()
.isSubrangeOf(readFrameIndexRange));
ASSERT_EQ(contSampleFrames.frameIndexRange().start(),
readFrameIndexRange.start());
contFrameIndex += contSampleFrames.frameLength();
Expand Down

0 comments on commit 18ea1f0

Please sign in to comment.