AudioVisualiserComponent: Fix potential data races

In the old implementation, because pushSample() could be called on the
audio thread, and updated the levels array and the nextSample value
non-atomically, other threads (e.g. the UI thread) were not guaranteed
to see these updates in a consistent order.

modules/juce_audio_utils/gui/juce_AudioVisualiserComponent.cpp

@@ -37,81 +37,96 @@ namespace juce
 
 struct AudioVisualiserComponent::ChannelInfo
 {
-    ChannelInfo (AudioVisualiserComponent& o, int bufferSize) : owner (o)
+    void setFifoSize (int numBlocks)
     {
-        setBufferSize (bufferSize);
+        fifoStorage.clear();
-        clear();
+        fifoStorage.resize ((size_t) numBlocks);
+        fifo.setTotalSize (numBlocks);
     }
 
-    void clear() noexcept
+    void setBufferSize (int numBlocks)
     {
-        levels.fill ({});
+        levels.clear();
+        levels.resize ((size_t) numBlocks);
+        nextSample = 0;
+    }
+
+    void clear()
+    {
+        for (auto& c : levels)
+            c = {};
+
+        counter = 0;
         value = {};
-        subSample = 0;
     }
 
-    void pushSamples (const float* inputSamples, int num) noexcept
+    void pushSamples (int blockSize, Span<const float> samples)
     {
-        for (int i = 0; i < num; ++i)
+        for (const auto& sample : samples)
-            pushSample (inputSamples[i]);
+            pushSample (blockSize, sample);
     }
 
-    void pushSample (float newSample) noexcept
+    void pushSample (int blockSize, float sample)
     {
-        if (--subSample <= 0)
+        if (++counter < blockSize)
         {
-            if (++nextSample == levels.size())
+            value = value.getUnionWith (sample);
-                nextSample = 0;
+            return;
-
-            levels.getReference (nextSample) = value;
-            subSample = owner.getSamplesPerBlock();
-            value = Range<float> (newSample, newSample);
         }
-        else
+
+        fifo.write (1).forEach ([this] (auto index)
         {
-            value = value.getUnionWith (newSample);
+            fifoStorage[(size_t) index] = value;
-        }
+        });
+
+        counter = 0;
+        value = Range (sample, sample);
     }
 
-    void setBufferSize (int newSize)
+    void popPending()
     {
-        levels.removeRange (newSize, levels.size());
+        fifo.read (fifo.getNumReady()).forEach ([this] (auto index)
-        levels.insertMultiple (-1, {}, newSize - levels.size());
+        {
-
+            levels[nextSample] = fifoStorage[(size_t) index];
-        if (nextSample >= newSize)
+            nextSample = (nextSample + 1) % levels.size();
-            nextSample = 0;
+        });
     }
 
-    AudioVisualiserComponent& owner;
-    Array<Range<float>> levels;
     Range<float> value;
-    std::atomic<int> nextSample { 0 }, subSample { 0 };
+    int counter = 0;
 
-    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ChannelInfo)
+    std::vector<Range<float>> fifoStorage;
+    AbstractFifo fifo { 1 };
+
+    std::vector<Range<float>> levels;
+    size_t nextSample = 0;
 };
 
 //==============================================================================
 AudioVisualiserComponent::AudioVisualiserComponent (int initialNumChannels)
-  : numSamples (1024),
+    : numSamples (1024),
-    inputSamplesPerBlock (256),
+      inputSamplesPerBlock (256),
-    backgroundColour (Colours::black),
+      backgroundColour (Colours::black),
-    waveformColour (Colours::white)
+      waveformColour (Colours::white)
 {
     setOpaque (true);
     setNumChannels (initialNumChannels);
     setRepaintRate (60);
 }
 
-AudioVisualiserComponent::~AudioVisualiserComponent()
+AudioVisualiserComponent::~AudioVisualiserComponent() = default;
-{
-}
 
 void AudioVisualiserComponent::setNumChannels (int numChannels)
 {
     channels.clear();
 
     for (int i = 0; i < numChannels; ++i)
-        channels.add (new ChannelInfo (*this, numSamples));
+        channels.add (new ChannelInfo);
+
+    for (auto* channel : channels)
+        channel->setBufferSize (numSamples);
+
+    updateChannelFifoSizes();
 }
 
 void AudioVisualiserComponent::setBufferSize (int newNumSamples)
@@ -132,8 +147,8 @@ void AudioVisualiserComponent::pushBuffer (const float* const* d, int numChannel
 {
     numChannels = jmin (numChannels, channels.size());
 
-    for (int i = 0; i < numChannels; ++i)
+    for (auto i = 0; i < numChannels; ++i)
-        channels.getUnchecked (i)->pushSamples (d[i], num);
+        channels.getUnchecked (i)->pushSamples (inputSamplesPerBlock, { d[i], (size_t) num });
 }
 
 void AudioVisualiserComponent::pushBuffer (const AudioBuffer<float>& buffer)
@@ -147,31 +162,38 @@ void AudioVisualiserComponent::pushBuffer (const AudioSourceChannelInfo& buffer)
 {
     auto numChannels = jmin (buffer.buffer->getNumChannels(), channels.size());
 
-    for (int i = 0; i < numChannels; ++i)
+    for (auto i = 0; i < numChannels; ++i)
-        channels.getUnchecked (i)->pushSamples (buffer.buffer->getReadPointer (i, buffer.startSample),
+    {
-                                               buffer.numSamples);
+        channels.getUnchecked (i)->pushSamples (inputSamplesPerBlock,
+                                                { buffer.buffer->getReadPointer (i, buffer.startSample), (size_t) buffer.numSamples });
+    }
 }
 
 void AudioVisualiserComponent::pushSample (const float* d, int numChannels)
 {
     numChannels = jmin (numChannels, channels.size());
 
-    for (int i = 0; i < numChannels; ++i)
+    for (auto i = 0; i < numChannels; ++i)
-        channels.getUnchecked (i)->pushSample (d[i]);
+        channels.getUnchecked (i)->pushSample (inputSamplesPerBlock, d[i]);
 }
 
 void AudioVisualiserComponent::setSamplesPerBlock (int newSamplesPerPixel) noexcept
 {
+    jassert (newSamplesPerPixel > 0);
     inputSamplesPerBlock = newSamplesPerPixel;
 }
 
 void AudioVisualiserComponent::setRepaintRate (int frequencyInHz)
 {
     startTimerHz (frequencyInHz);
+    updateChannelFifoSizes();
 }
 
 void AudioVisualiserComponent::timerCallback()
 {
+    for (auto* channel : channels)
+        channel->popPending();
+
     repaint();
 }
 
@@ -192,8 +214,13 @@ void AudioVisualiserComponent::paint (Graphics& g)
     g.setColour (waveformColour);
 
     for (auto* c : channels)
-        paintChannel (g, r.removeFromTop (channelHeight),
+    {
-                      c->levels.begin(), c->levels.size(), c->nextSample);
+        paintChannel (g,
+                      r.removeFromTop (channelHeight),
+                      c->levels.data(),
+                      (int) c->levels.size(),
+                      (int) c->nextSample);
+    }
 }
 
 void AudioVisualiserComponent::getChannelAsPath (Path& path, const Range<float>* levels,
@@ -228,4 +255,21 @@ void AudioVisualiserComponent::paintChannel (Graphics& g, Rectangle<float> area,
                                                       (float) numLevels, -1.0f,  area.getRight(), area.getY()));
 }
 
+void AudioVisualiserComponent::updateChannelFifoSizes()
+{
+    // This is intended to make sure that the fifo for each channel is large enough to store
+    // at least one frame's incoming blocks with some extra padding to avoid dropping too much info
+    // if a frame is delayed.
+
+    const auto maxSampleRate = 192'000;
+    const auto maxBlocksPerSecond = inputSamplesPerBlock > 0
+                                  ? ((maxSampleRate + inputSamplesPerBlock - 1) / inputSamplesPerBlock)
+                                  : 1;
+    const auto maxBlocksPerRepaint = (maxBlocksPerSecond * getTimerInterval() + 999) / 1000;
+    const auto paddedBlocksPerRepaint = 10 + maxBlocksPerRepaint;
+
+    for (auto* channel : channels)
+        channel->setFifoSize (paddedBlocksPerRepaint);
+}
+
 } // namespace juce

modules/juce_audio_utils/gui/juce_AudioVisualiserComponent.h

@@ -104,7 +104,7 @@ public:
     */
     void pushSample (const float* samplesForEachChannel, int numChannels);
 
-    /** Sets the colours used to paint the */
+    /** Sets the colours used to paint the waveform. */
     void setColours (Colour backgroundColour, Colour waveformColour) noexcept;
 
     /** Sets the frequency at which the component repaints itself. */
@@ -121,7 +121,7 @@ public:
         The path is normalised so that -1 and +1 are its upper and lower bounds, and it
         goes from 0 to numLevels on the X axis.
     */
-    void getChannelAsPath (Path& result, const Range<float>* levels, int numLevels, int nextSample);
+    static void getChannelAsPath (Path& result, const Range<float>* levels, int numLevels, int nextSample);
 
     //==============================================================================
     /** @internal */
@@ -135,6 +135,7 @@ private:
     Colour backgroundColour, waveformColour;
 
     void timerCallback() override;
+    void updateChannelFifoSizes();
 
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (AudioVisualiserComponent)
 };