From 89452a4dd6d4e9ae711b7d3cec564bd2c5df958b Mon Sep 17 00:00:00 2001 From: James Ball Date: Tue, 11 Jul 2023 22:28:54 +0100 Subject: [PATCH] Introduce more locks to prevent concurrency issues --- Source/PluginProcessor.cpp | 9 ++---- Source/PluginProcessor.h | 2 +- Source/audio/SmoothEffect.cpp | 34 +++++----------------- Source/audio/SmoothEffect.h | 6 ++-- Source/components/DraggableListBox.cpp | 8 ++--- Source/components/EffectComponent.cpp | 6 ++-- Source/components/EffectsListComponent.cpp | 28 +++++++++++------- Source/components/EffectsListComponent.h | 1 + Source/parser/FileParser.cpp | 26 ++++++++--------- Source/parser/FileParser.h | 2 ++ Source/parser/FrameProducer.cpp | 7 +++-- Source/parser/FrameProducer.h | 1 + 12 files changed, 60 insertions(+), 70 deletions(-) diff --git a/Source/PluginProcessor.cpp b/Source/PluginProcessor.cpp index eee4305..b9ef53b 100644 --- a/Source/PluginProcessor.cpp +++ b/Source/PluginProcessor.cpp @@ -31,8 +31,7 @@ OscirenderAudioProcessor::OscirenderAudioProcessor() ) #endif { - producer = std::make_unique(*this, std::make_shared()); - producer->startThread(); + producer.startThread(); juce::SpinLock::ScopedLockType lock(effectsLock); @@ -295,12 +294,12 @@ void OscirenderAudioProcessor::openFile(int index) { void OscirenderAudioProcessor::changeCurrentFile(int index) { if (index == -1) { currentFile = -1; - producer->setSource(std::make_shared(), -1); + producer.setSource(std::make_shared(), -1); } if (index < 0 || index >= fileBlocks.size()) { return; } - producer->setSource(parsers[index], index); + producer.setSource(parsers[index], index); currentFile = index; invalidateFrameBuffer = true; updateLuaValues(); @@ -370,8 +369,6 @@ void OscirenderAudioProcessor::updateFrame() { frameLength = Shape::totalLength(frame); } - } else { - DBG("frame not ready!"); } } diff --git a/Source/PluginProcessor.h b/Source/PluginProcessor.h index 580fa6d..5bb02ed 100644 --- a/Source/PluginProcessor.h +++ b/Source/PluginProcessor.h @@ -171,7 +171,7 @@ public: std::vector fileNames; std::atomic currentFile = -1; - std::unique_ptr producer; + FrameProducer producer = FrameProducer(*this, std::make_shared()); BufferProducer audioProducer; diff --git a/Source/audio/SmoothEffect.cpp b/Source/audio/SmoothEffect.cpp index 981bb33..37835b6 100644 --- a/Source/audio/SmoothEffect.cpp +++ b/Source/audio/SmoothEffect.cpp @@ -1,33 +1,15 @@ #include "SmoothEffect.h" -SmoothEffect::SmoothEffect() { - this->window = std::vector(MAX_WINDOW_SIZE); -} +SmoothEffect::SmoothEffect() {} SmoothEffect::~SmoothEffect() {} Vector2 SmoothEffect::apply(int index, Vector2 input, std::vector details, double frequency, double sampleRate) { - double value = details[0].value; - int newWindowSize = (int)(256 * value); - windowSize = std::max(1, std::min(MAX_WINDOW_SIZE, newWindowSize)); - - window[head++] = input; - if (head >= MAX_WINDOW_SIZE) { - head = 0; - } - double totalX = 0; - double totalY = 0; - int newHead = head - 1; - for (int i = 0; i < windowSize; i++) { - if (newHead < 0) { - newHead = MAX_WINDOW_SIZE - 1; - } - - totalX += window[newHead].x; - totalY += window[newHead].y; - - newHead--; - } - - return Vector2(totalX / windowSize, totalY / windowSize); + double weight = details[0].value; + weight *= 0.95; + double strength = 1000000; + weight = std::log(strength * weight + 1) / std::log(strength + 1); + leftAvg = weight * leftAvg + (1 - weight) * input.x; + rightAvg = weight * rightAvg + (1 - weight) * input.y; + return Vector2(leftAvg, rightAvg); } diff --git a/Source/audio/SmoothEffect.h b/Source/audio/SmoothEffect.h index 9d5b05f..e125f23 100644 --- a/Source/audio/SmoothEffect.h +++ b/Source/audio/SmoothEffect.h @@ -9,8 +9,6 @@ public: Vector2 apply(int index, Vector2 input, std::vector details, double frequency, double sampleRate) override; private: - const int MAX_WINDOW_SIZE = 2048; - std::vector window; - int windowSize = 1; - int head = 0; + double leftAvg = 0; + double rightAvg = 0; }; \ No newline at end of file diff --git a/Source/components/DraggableListBox.cpp b/Source/components/DraggableListBox.cpp index c904759..a405e2c 100644 --- a/Source/components/DraggableListBox.cpp +++ b/Source/components/DraggableListBox.cpp @@ -6,13 +6,13 @@ void DraggableListBoxItem::paint(juce::Graphics& g) { if (insertAfter) { - g.setColour(juce::Colours::red); - g.fillRect(0, getHeight() - 3, getWidth(), 3); + g.setColour(juce::Colour(0xff00ff00)); + g.fillRect(0, getHeight() - 4, getWidth(), 4); } else if (insertBefore) { - g.setColour(juce::Colours::red); - g.fillRect(0, 0, getWidth(), 3); + g.setColour(juce::Colour(0xff00ff00)); + g.fillRect(0, 0, getWidth(), 4); } } diff --git a/Source/components/EffectComponent.cpp b/Source/components/EffectComponent.cpp index 10ee897..51b37d3 100644 --- a/Source/components/EffectComponent.cpp +++ b/Source/components/EffectComponent.cpp @@ -54,10 +54,8 @@ void EffectComponent::resized() { } void EffectComponent::paint(juce::Graphics& g) { - g.fillAll(juce::Colours::lightgrey); - g.setColour(juce::Colours::black); - auto bounds = getLocalBounds(); - g.drawRect(bounds); + g.fillAll(juce::Colours::black); + g.setColour(juce::Colours::white); g.drawText(name, textBounds, juce::Justification::left); } diff --git a/Source/components/EffectsListComponent.cpp b/Source/components/EffectsListComponent.cpp index 6b383e8..e1fde69 100644 --- a/Source/components/EffectsListComponent.cpp +++ b/Source/components/EffectsListComponent.cpp @@ -3,7 +3,7 @@ EffectsListComponent::EffectsListComponent(DraggableListBox& lb, AudioEffectListBoxItemData& data, int rn, std::shared_ptr effect) : DraggableListBoxItem(lb, data, rn), effect(effect) { auto details = effect->getDetails(); for (int i = 0; i < details.size(); i++) { - std::shared_ptr effectComponent = std::make_shared(0, 1, 0.01, details[i], i == 0); + std::shared_ptr effectComponent = std::make_shared(0, 1, 0.001, details[i], i == 0); // using weak_ptr to avoid circular reference and memory leak std::weak_ptr weakEffectComponent = effectComponent; effectComponent->slider.setValue(details[i].value, juce::dontSendNotification); @@ -48,21 +48,29 @@ EffectsListComponent::EffectsListComponent(DraggableListBox& lb, AudioEffectList EffectsListComponent::~EffectsListComponent() {} void EffectsListComponent::paint(juce::Graphics& g) { - DraggableListBoxItem::paint(g); auto bounds = getLocalBounds(); + g.fillAll(juce::Colours::darkgrey); + g.setColour(juce::Colours::white); bounds.removeFromLeft(20); // draw drag and drop handle using circles - g.setColour(juce::Colours::white); double size = 4; double leftPad = 4; double spacing = 7; - double topPad = 7; - g.fillEllipse(leftPad, topPad, size, size); - g.fillEllipse(leftPad, topPad + spacing, size, size); - g.fillEllipse(leftPad, topPad + 2 * spacing, size, size); - g.fillEllipse(leftPad + spacing, topPad, size, size); - g.fillEllipse(leftPad + spacing, topPad + spacing, size, size); - g.fillEllipse(leftPad + spacing, topPad + 2 * spacing, size, size); + double topPad = 6; + double y = bounds.getHeight() / 2 - 15; + g.fillEllipse(leftPad, y + topPad, size, size); + g.fillEllipse(leftPad, y + topPad + spacing, size, size); + g.fillEllipse(leftPad, y + topPad + 2 * spacing, size, size); + g.fillEllipse(leftPad + spacing, y + topPad, size, size); + g.fillEllipse(leftPad + spacing, y + topPad + spacing, size, size); + g.fillEllipse(leftPad + spacing, y + topPad + 2 * spacing, size, size); + DraggableListBoxItem::paint(g); +} + +void EffectsListComponent::paintOverChildren(juce::Graphics& g) { + auto bounds = getLocalBounds(); + g.setColour(juce::Colours::white); + g.drawRect(bounds); } void EffectsListComponent::resized() { diff --git a/Source/components/EffectsListComponent.h b/Source/components/EffectsListComponent.h index 8ce8735..8e40446 100644 --- a/Source/components/EffectsListComponent.h +++ b/Source/components/EffectsListComponent.h @@ -93,6 +93,7 @@ public: ~EffectsListComponent(); void paint(juce::Graphics& g) override; + void paintOverChildren(juce::Graphics& g) override; void resized() override; protected: diff --git a/Source/parser/FileParser.cpp b/Source/parser/FileParser.cpp index 386346b..d5e52e8 100644 --- a/Source/parser/FileParser.cpp +++ b/Source/parser/FileParser.cpp @@ -6,6 +6,8 @@ FileParser::FileParser() {} void FileParser::parse(juce::String extension, std::unique_ptr stream) { + juce::SpinLock::ScopedLockType scope(lock); + object = nullptr; camera = nullptr; svg = nullptr; @@ -28,17 +30,14 @@ void FileParser::parse(juce::String extension, std::unique_ptr> FileParser::nextFrame() { - auto tempObject = object; - auto tempCamera = camera; - auto tempSvg = svg; - auto tempText = text; + juce::SpinLock::ScopedLockType scope(lock); - if (tempObject != nullptr && tempCamera != nullptr) { - return tempCamera->draw(*tempObject); - } else if (tempSvg != nullptr) { - return tempSvg->draw(); - } else if (tempText != nullptr) { - return tempText->draw(); + if (object != nullptr && camera != nullptr) { + return camera->draw(*object); + } else if (svg != nullptr) { + return svg->draw(); + } else if (text != nullptr) { + return text->draw(); } auto tempShapes = std::vector>(); tempShapes.push_back(std::make_unique(0, 0, 0.5, 0.5, std::numbers::pi / 4.0, 2 * std::numbers::pi)); @@ -46,9 +45,10 @@ std::vector> FileParser::nextFrame() { } Vector2 FileParser::nextSample() { - auto tempLua = lua; - if (tempLua != nullptr) { - return tempLua->draw(); + juce::SpinLock::ScopedLockType scope(lock); + + if (lua != nullptr) { + return lua->draw(); } } diff --git a/Source/parser/FileParser.h b/Source/parser/FileParser.h index 1764786..28e642b 100644 --- a/Source/parser/FileParser.h +++ b/Source/parser/FileParser.h @@ -30,6 +30,8 @@ private: bool active = true; bool sampleSource = false; + juce::SpinLock lock; + std::shared_ptr object; std::shared_ptr camera; std::shared_ptr svg; diff --git a/Source/parser/FrameProducer.cpp b/Source/parser/FrameProducer.cpp index ddaf82e..30e0bd5 100644 --- a/Source/parser/FrameProducer.cpp +++ b/Source/parser/FrameProducer.cpp @@ -8,13 +8,16 @@ FrameProducer::~FrameProducer() { } void FrameProducer::run() { - while (!threadShouldExit() && frameSource->isActive()) { + while (!threadShouldExit()) { + // this lock is needed so that frameSource isn't deleted whilst nextFrame() is being called + juce::SpinLock::ScopedLockType scope(lock); frameConsumer.addFrame(frameSource->nextFrame(), sourceFileIndex); } } void FrameProducer::setSource(std::shared_ptr source, int fileIndex) { - // TODO: should make this atomic + juce::SpinLock::ScopedLockType scope(lock); + frameSource->disable(); frameSource = source; sourceFileIndex = fileIndex; } diff --git a/Source/parser/FrameProducer.h b/Source/parser/FrameProducer.h index 4ae980f..873c6f4 100644 --- a/Source/parser/FrameProducer.h +++ b/Source/parser/FrameProducer.h @@ -12,6 +12,7 @@ public: void run() override; void setSource(std::shared_ptr, int fileIndex); private: + juce::SpinLock lock; FrameConsumer& frameConsumer; std::shared_ptr frameSource; int sourceFileIndex = -1;