Fix AudioFormatReaderSource::getNextAudioBlock

The issue occurred when using looping and reading a block that is larger
than the length of the underlying AudioFormatReader.

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 auto numSamplesToClear = info.numSamples - destOffset;
-            const int64 newEnd = (start + info.numSamples) % reader->lengthInSamples;
+            info.buffer->clear (info.startSample + destOffset, numSamplesToClear);
 
-            if (newEnd > newStart)
+            destOffset += numSamplesToClear;
-            {
+            nextPlayPos += numSamplesToClear;
-                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;
         }
     }
 }
 
+#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<float>& 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<float>*, 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<float*> (*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<float>& result)
+    {
+        source.setLooping (params.enableLooping);
+        source.setNextReadPosition (params.readFrom);
+
+        AudioBuffer<float> 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<float> actual;
+            getNextAudioBlock (uut, params, actual);
+
+            std::vector<float> 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<float> actual;
+            std::vector<float> 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<float> actual;
+            std::vector<float> 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<int> audioFormatReaderLengthRange { 16, 128 };
+            const Range<int> startFromRange { 0, 128 };
+            const Range<int> 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