From 08f449d2b017fe34c546b0524dfbd6cfb166e6d4 Mon Sep 17 00:00:00 2001 From: reuk Date: Mon, 2 Jun 2025 17:55:46 +0100 Subject: [PATCH] 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. --- .../gui/juce_AudioVisualiserComponent.cpp | 138 ++++++++++++------ .../gui/juce_AudioVisualiserComponent.h | 5 +- 2 files changed, 94 insertions(+), 49 deletions(-) diff --git a/modules/juce_audio_utils/gui/juce_AudioVisualiserComponent.cpp b/modules/juce_audio_utils/gui/juce_AudioVisualiserComponent.cpp index 4fc7063914..d9adc1e54b 100644 --- a/modules/juce_audio_utils/gui/juce_AudioVisualiserComponent.cpp +++ b/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); - clear(); + fifoStorage.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 samples) { - for (int i = 0; i < num; ++i) - pushSample (inputSamples[i]); + for (const auto& sample : samples) + pushSample (blockSize, sample); } - void pushSample (float newSample) noexcept + void pushSample (int blockSize, float sample) { - if (--subSample <= 0) + if (++counter < blockSize) { - if (++nextSample == levels.size()) - nextSample = 0; - - levels.getReference (nextSample) = value; - subSample = owner.getSamplesPerBlock(); - value = Range (newSample, newSample); + value = value.getUnionWith (sample); + return; } - 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()); - levels.insertMultiple (-1, {}, newSize - levels.size()); - - if (nextSample >= newSize) - nextSample = 0; + fifo.read (fifo.getNumReady()).forEach ([this] (auto index) + { + levels[nextSample] = fifoStorage[(size_t) index]; + nextSample = (nextSample + 1) % levels.size(); + }); } - AudioVisualiserComponent& owner; - Array> levels; Range value; - std::atomic nextSample { 0 }, subSample { 0 }; + int counter = 0; - JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ChannelInfo) + std::vector> fifoStorage; + AbstractFifo fifo { 1 }; + + std::vector> levels; + size_t nextSample = 0; }; //============================================================================== AudioVisualiserComponent::AudioVisualiserComponent (int initialNumChannels) - : numSamples (1024), - inputSamplesPerBlock (256), - backgroundColour (Colours::black), - waveformColour (Colours::white) + : numSamples (1024), + inputSamplesPerBlock (256), + backgroundColour (Colours::black), + 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) - channels.getUnchecked (i)->pushSamples (d[i], num); + for (auto i = 0; i < numChannels; ++i) + channels.getUnchecked (i)->pushSamples (inputSamplesPerBlock, { d[i], (size_t) num }); } void AudioVisualiserComponent::pushBuffer (const AudioBuffer& 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) - channels.getUnchecked (i)->pushSamples (buffer.buffer->getReadPointer (i, buffer.startSample), - buffer.numSamples); + for (auto i = 0; i < numChannels; ++i) + { + 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) - channels.getUnchecked (i)->pushSample (d[i]); + for (auto i = 0; i < numChannels; ++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* levels, @@ -228,4 +255,21 @@ void AudioVisualiserComponent::paintChannel (Graphics& g, Rectangle 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 diff --git a/modules/juce_audio_utils/gui/juce_AudioVisualiserComponent.h b/modules/juce_audio_utils/gui/juce_AudioVisualiserComponent.h index 5f92ebddf3..dd6ad1a1cf 100644 --- a/modules/juce_audio_utils/gui/juce_AudioVisualiserComponent.h +++ b/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* levels, int numLevels, int nextSample); + static void getChannelAsPath (Path& result, const Range* 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) };