From 5b604e6d3c47c89ccd96a6c1b090a787b58604d4 Mon Sep 17 00:00:00 2001 From: attila Date: Wed, 27 Nov 2024 19:37:28 +0100 Subject: [PATCH] Fix AudioFormatReaderSource::getNextAudioBlock The issue occurred when using looping and reading a block that is larger than the length of the underlying AudioFormatReader. --- .../format/juce_AudioFormatReaderSource.cpp | 278 +++++++++++++++--- 1 file changed, 244 insertions(+), 34 deletions(-) diff --git a/modules/juce_audio_formats/format/juce_AudioFormatReaderSource.cpp b/modules/juce_audio_formats/format/juce_AudioFormatReaderSource.cpp index 93d1564913..be8cbe77f1 100644 --- a/modules/juce_audio_formats/format/juce_AudioFormatReaderSource.cpp +++ b/modules/juce_audio_formats/format/juce_AudioFormatReaderSource.cpp @@ -61,46 +61,256 @@ void AudioFormatReaderSource::releaseResources() {} void AudioFormatReaderSource::getNextAudioBlock (const AudioSourceChannelInfo& info) { - if (info.numSamples > 0) + if (info.numSamples <= 0) + return; + + for (auto destOffset = 0; destOffset < info.numSamples;) { - const int64 start = nextPlayPos; + const auto readFrom = looping ? nextPlayPos % reader->lengthInSamples : nextPlayPos; - if (looping) + const auto numSamplesToRead = jlimit ((int64) 0, + (int64) (info.numSamples - destOffset), + reader->lengthInSamples - readFrom); + + reader->read (info.buffer, info.startSample + destOffset, + (int) numSamplesToRead, readFrom, true, true); + + destOffset += (int) numSamplesToRead; + nextPlayPos += numSamplesToRead; + + if (! looping) { - const int64 newStart = start % reader->lengthInSamples; - const int64 newEnd = (start + info.numSamples) % reader->lengthInSamples; + const auto numSamplesToClear = info.numSamples - destOffset; + info.buffer->clear (info.startSample + destOffset, numSamplesToClear); - if (newEnd > newStart) - { - reader->read (info.buffer, info.startSample, - (int) (newEnd - newStart), newStart, true, true); - } - else - { - const int endSamps = (int) (reader->lengthInSamples - newStart); - - reader->read (info.buffer, info.startSample, - endSamps, newStart, true, true); - - reader->read (info.buffer, info.startSample + endSamps, - (int) newEnd, 0, true, true); - } - - nextPlayPos = newEnd; - } - else - { - const auto samplesToRead = jlimit (int64{}, - (int64) info.numSamples, - reader->lengthInSamples - start); - - reader->read (info.buffer, info.startSample, (int) samplesToRead, start, true, true); - info.buffer->clear ((int) (info.startSample + samplesToRead), - (int) (info.numSamples - samplesToRead)); - - nextPlayPos += info.numSamples; + destOffset += numSamplesToClear; + nextPlayPos += numSamplesToClear; } } } +#if JUCE_UNIT_TESTS + +struct AudioFormatReaderSourceTests : public UnitTest +{ + AudioFormatReaderSourceTests() + : UnitTest ("AudioFormatReaderSource", UnitTestCategories::audio) + {} + + //============================================================================== + struct GetNextAudioBlockTestParams + { + int audioFormatReaderLength; + int readFrom; + int numSamplesToRead; + bool enableLooping; + }; + + static void mockReadSamples (float* dest, int64 audioFormatReaderLength, int64 readFrom, int numSamplesToRead) + { + for (auto i = readFrom; i < readFrom + numSamplesToRead; ++i) + { + *dest = i < audioFormatReaderLength ? 0.001f * (float) i : 0.0f; + ++dest; + } + } + + static void createGetNextAudioBlockExpectedOutput (const GetNextAudioBlockTestParams& params, + std::vector& expected) + { + for (auto i = params.readFrom, end = i + params.numSamplesToRead; i < end; ++i) + { + const auto expectedResult = params.enableLooping || i < params.audioFormatReaderLength + ? 0.001f * (float) (i % params.audioFormatReaderLength) + : 0.0f; + + expected.push_back (expectedResult); + } + } + + //============================================================================== + struct TestAudioFormatReader : public AudioFormatReader + { + explicit TestAudioFormatReader (int audioFormatReaderLength) + : AudioFormatReader { nullptr, "test_format" } + { + jassert (audioFormatReaderLength < 1000); + + lengthInSamples = (int64) audioFormatReaderLength; + numChannels = 1; + usesFloatingPointData = true; + bitsPerSample = 32; + } + + void readMaxLevels (int64, int64, Range*, int) override { jassertfalse; } + void readMaxLevels (int64, int64, float&, float&, float&, float&) override { jassertfalse; } + + AudioChannelSet getChannelLayout() override + { + return AudioChannelSet::mono(); + } + + bool readSamples (int* const* destChannels, + [[maybe_unused]] int numDestChannels, + int startOffsetInDestBuffer, + int64 startSampleInFile, + int numSamples) override + { + jassert (numDestChannels == 1); + mockReadSamples (reinterpret_cast (*destChannels + startOffsetInDestBuffer), + lengthInSamples, + startSampleInFile, + numSamples); + return true; + } + }; + + static auto createTestAudioFormatReaderSource (const GetNextAudioBlockTestParams& params) + { + return AudioFormatReaderSource { new TestAudioFormatReader (params.audioFormatReaderLength), true }; + } + + static void getNextAudioBlock (AudioFormatReaderSource& source, + const GetNextAudioBlockTestParams& params, + std::vector& result) + { + source.setLooping (params.enableLooping); + source.setNextReadPosition (params.readFrom); + + AudioBuffer buffer { 1, params.numSamplesToRead }; + AudioSourceChannelInfo info { &buffer, 0, buffer.getNumSamples() }; + + source.getNextAudioBlock (info); + + result.insert (result.end(), + buffer.getReadPointer (0), + buffer.getReadPointer (0) + buffer.getNumSamples()); + } + + static auto createFailureMessage (const GetNextAudioBlockTestParams& params) + { + return String { "AudioFormatReaderSource::getNextAudioBlock() failed for " + "audioFormatReaderLength=%audioFormatReaderLength%, " + "readFrom=%readFrom%, " + "numSamplesToRead=%numSamplesToRead%, " + "enableLooping=%enableLooping%" } + .replace ("%audioFormatReaderLength%", String { params.audioFormatReaderLength }) + .replace ("%readFrom%", String { params.readFrom }) + .replace ("%numSamplesToRead%", String { params.numSamplesToRead }) + .replace ("%enableLooping%", params.enableLooping ? "true" : "false"); + } + + void runTest() override + { + const auto predicate = [] (auto a, auto b) { return exactlyEqual (a, b); }; + + const auto testGetNextAudioBlock = [this, &predicate] (const GetNextAudioBlockTestParams& params) + { + auto uut = createTestAudioFormatReaderSource (params); + std::vector actual; + getNextAudioBlock (uut, params, actual); + + std::vector expected; + createGetNextAudioBlockExpectedOutput (params, expected); + + expect (std::equal (expected.begin(), expected.end(), actual.begin(), actual.end(), predicate), + createFailureMessage (params)); + }; + + beginTest ("A buffer without looping is played once and followed by silence"); + { + GetNextAudioBlockTestParams testParams { 32, 0, 48, false }; + testGetNextAudioBlock (testParams); + } + + beginTest ("A buffer with looping is played multiple times"); + { + GetNextAudioBlockTestParams params { 32, 0, 24, true }; + + auto uut = createTestAudioFormatReaderSource (params); + std::vector actual; + std::vector expected; + const auto numReads = 4; + + for (auto i = 0; i < numReads; ++i) + { + getNextAudioBlock (uut, params, actual); + createGetNextAudioBlockExpectedOutput (params, expected); + params.readFrom += params.numSamplesToRead; + } + + expect (std::equal (expected.begin(), expected.end(), actual.begin(), actual.end(), predicate), + createFailureMessage (params) + " numReads=" + String { numReads }); + } + + beginTest ("A buffer with looping, loops even if the blockSize is greater than the internal buffer"); + { + GetNextAudioBlockTestParams testParams { 32, 16, 128, true }; + testGetNextAudioBlock (testParams); + } + + beginTest ("Behavioural invariants hold even if we turn on looping after prior reads"); + { + GetNextAudioBlockTestParams params { 32, 0, 24, false }; + + auto uut = createTestAudioFormatReaderSource (params); + std::vector actual; + std::vector expected; + + for (auto i = 0; i < 4; ++i) + { + getNextAudioBlock (uut, params, actual); + createGetNextAudioBlockExpectedOutput (params, expected); + params.readFrom += params.numSamplesToRead; + } + + params.enableLooping = true; + + for (auto i = 0; i < 4; ++i) + { + getNextAudioBlock (uut, params, actual); + createGetNextAudioBlockExpectedOutput (params, expected); + params.readFrom += params.numSamplesToRead; + } + + expect (std::equal (expected.begin(), expected.end(), actual.begin(), actual.end(), predicate), + createFailureMessage (params)); + } + + beginTest ("Fuzzing: getNextAudioBlock() should return correct results for all possible inputs"); + { + for (auto params : { GetNextAudioBlockTestParams { 32, 0, 32, false }, + GetNextAudioBlockTestParams { 32, 16, 32, false }, + GetNextAudioBlockTestParams { 32, 16, 32, true }, + GetNextAudioBlockTestParams { 32, 16, 48, false }, + GetNextAudioBlockTestParams { 32, 16, 128, false }, + GetNextAudioBlockTestParams { 32, 16, 48, true }, + GetNextAudioBlockTestParams { 32, 16, 128, true } }) + { + testGetNextAudioBlock (params); + } + + const Range audioFormatReaderLengthRange { 16, 128 }; + const Range startFromRange { 0, 128 }; + const Range numSamplesRange { 0, 128 }; + + auto r = getRandom(); + + for (int i = 0; i < 100; ++i) + { + GetNextAudioBlockTestParams params { r.nextInt (audioFormatReaderLengthRange), + r.nextInt (startFromRange), + r.nextInt (numSamplesRange), + r.nextBool() }; + + testGetNextAudioBlock (params); + } + } + } +}; + +static AudioFormatReaderSourceTests audioFormatReaderSourceTests; + +#endif + } // namespace juce