diff --git a/include/element/processor.hpp b/include/element/processor.hpp index 106b07910..d7611cb54 100644 --- a/include/element/processor.hpp +++ b/include/element/processor.hpp @@ -228,6 +228,14 @@ class Processor : public juce::ReferenceCountedObject { /** Suspend processing */ void suspendProcessing (const bool); + /** Upper bound applied to any latency value in samples. + + Plugin-reported latency and delay compensation size per-channel delay + buffers in the graph builder, so a bogus value would otherwise allocate + gigabytes. About 21 seconds at 48 kHz. + */ + static constexpr int maxLatencySamples = 1 << 20; + /** Get latency audio samples */ int getLatencySamples() const; @@ -516,7 +524,7 @@ class Processor : public juce::ReferenceCountedObject { } //========================================================================== - /** Set latency samples */ + /** Set latency samples. Values are clamped to [0, maxLatencySamples]. */ void setLatencySamples (int latency); //========================================================================== @@ -621,6 +629,7 @@ class Processor : public juce::ReferenceCountedObject { double delayCompMillis = 0.0; int delayCompSamples = 0; + void updateDelayCompensationSamples(); juce::AudioPlayHead* _playhead { nullptr }; diff --git a/src/engine/graphnode.cpp b/src/engine/graphnode.cpp index d9e973763..3eca20622 100644 --- a/src/engine/graphnode.cpp +++ b/src/engine/graphnode.cpp @@ -457,7 +457,7 @@ void GraphNode::buildRenderingSequence() // swap over to the new rendering sequence.. { const ScopedLock sl (getPropertyLock()); - renderingBuffers.setSize (numRenderingBuffersNeeded, 4096); + renderingBuffers.setSize (numRenderingBuffersNeeded, jmax (4096, getBlockSize())); renderingBuffers.clear(); for (int i = midiBuffers.size(); --i >= 0;) diff --git a/src/engine/processor.cpp b/src/engine/processor.cpp index 27e27b9de..08b104beb 100644 --- a/src/engine/processor.cpp +++ b/src/engine/processor.cpp @@ -337,6 +337,7 @@ void Processor::prepare (const double newSampleRate, sampleRate = newSampleRate; blockSize = newBlockSize; parent = parentGraph; + updateDelayCompensationSamples(); if ((willBeEnabled || enabled.get() == 1) && ! isPrepared) { @@ -891,13 +892,29 @@ int Processor::getOversamplingFactor() } //============================================================================== +static int clampLatencySamples (const Processor& proc, int samples, const char* what) +{ + if (samples >= 0 && samples <= Processor::maxLatencySamples) + return samples; + Logger::writeToLog (String ("[element] ") + proc.getName() + ": clamping " + what + + " of " + String (samples) + " samples"); + return jlimit (0, Processor::maxLatencySamples, samples); +} + void Processor::setDelayCompensation (double delayMs) { - if (delayCompMillis == delayMs) - return; delayCompMillis = delayMs; - jassert (sampleRate > 0.0); - delayCompSamples = roundToInt (delayCompMillis * 0.001 * sampleRate); + updateDelayCompensationSamples(); +} + +void Processor::updateDelayCompensationSamples() +{ + // The sample rate may not be known yet (e.g. state restored before + // prepare); prepare() calls this again once it is. + if (sampleRate <= 0.0) + return; + delayCompSamples = clampLatencySamples ( + *this, roundToInt (delayCompMillis * 0.001 * sampleRate), "delay compensation"); } double Processor::getDelayCompensation() const { return delayCompMillis; } @@ -987,9 +1004,7 @@ int Processor::getLatencySamples() const void Processor::setLatencySamples (int latency) { - if (latency == latencySamples) - return; - latencySamples = latency; + latencySamples = clampLatencySamples (*this, latency, "latency"); } //========================================================================= diff --git a/src/engine/rootgraph.cpp b/src/engine/rootgraph.cpp index 59cc660c5..a1188db9e 100644 --- a/src/engine/rootgraph.cpp +++ b/src/engine/rootgraph.cpp @@ -22,8 +22,8 @@ void RootGraph::setPlayConfigFor (DeviceManager& devices) void RootGraph::setPlayConfigFor (AudioIODevice* device) { jassert (device != nullptr); - setRenderDetails (device->getCurrentBufferSizeSamples(), - device->getCurrentSampleRate()); + setRenderDetails (device->getCurrentSampleRate(), + device->getCurrentBufferSizeSamples()); } void RootGraph::setPlayConfigFor (const DeviceManager::AudioDeviceSetup& setup) diff --git a/src/nodes/audiofileplayer.cpp b/src/nodes/audiofileplayer.cpp index dafa8626f..aa6b32cf3 100644 --- a/src/nodes/audiofileplayer.cpp +++ b/src/nodes/audiofileplayer.cpp @@ -415,6 +415,9 @@ AudioFilePlayerNode::AudioFilePlayerNode() addLegacyParameter (volume = new AudioParameterFloat (juce::ParameterID ("volume", 1), "Volume", -60.f, 12.f, 0.f)); addLegacyParameter (looping = new AudioParameterBool (juce::ParameterID ("loop", 1), "Loop", false)); + // Needed before prepareToPlay: state restore opens the file. + formats.registerBasicFormats(); + for (auto* const param : getParameters()) param->addListener (this); } @@ -494,7 +497,6 @@ void AudioFilePlayerNode::openFile (const File& file) void AudioFilePlayerNode::prepareToPlay (double sampleRate, int maximumExpectedSamplesPerBlock) { thread.startThread(); - formats.registerBasicFormats(); player.prepareToPlay (maximumExpectedSamplesPerBlock, sampleRate); if (reader) diff --git a/src/nodes/mediaplayer.cpp b/src/nodes/mediaplayer.cpp index b465698d2..85bf5f59d 100644 --- a/src/nodes/mediaplayer.cpp +++ b/src/nodes/mediaplayer.cpp @@ -163,6 +163,10 @@ MediaPlayerProcessor::MediaPlayerProcessor() addLegacyParameter (playing = new AudioParameterBool (juce::ParameterID ("playing", 1), "Playing", false)); addLegacyParameter (slave = new AudioParameterBool (juce::ParameterID ("slave", 1), "Slave", false)); addLegacyParameter (volume = new AudioParameterFloat (juce::ParameterID ("volume", 1), "Volume", -60.f, 12.f, 0.f)); + + // Needed before prepareToPlay: state restore opens the file. + formats.registerBasicFormats(); + for (auto* const param : getParameters()) param->addListener (this); } diff --git a/src/ui/block.cpp b/src/ui/block.cpp index 72e342dd5..546d26222 100644 --- a/src/ui/block.cpp +++ b/src/ui/block.cpp @@ -23,6 +23,12 @@ namespace element { +/** Upper bound on a block's custom size. The block is buffered to an image and + has a shadow effect, so an unclamped size read from a session would allocate + two enormous bitmaps. */ +static constexpr int maxBlockWidth = 8192; +static constexpr int maxBlockHeight = 8192; + namespace detail { inline static Context* context (juce::Component* comp) { @@ -335,12 +341,17 @@ void BlockComponent::setDisplayModeInternal (DisplayMode mode, bool force) { struct EmbedBockAsync : MessageManager::MessageBase { - using PtrType = std::unique_ptr; - EmbedBockAsync (BlockComponent& b, const Node& n, UI& u, PtrType& p, DisplayMode om) - : block (b), node (n), ui (u), embedded (p), oldMode (om) {} + EmbedBockAsync (BlockComponent& b, const Node& n, UI& u, DisplayMode om) + : block (&b), node (n), ui (u), oldMode (om) {} void messageCallback() override { + // The block may have been deleted (session change, rebuild) + // before this message was delivered. + if (block == nullptr) + return; + + auto& embedded = block->embedded; ui.closePluginWindowsFor (node, false); if (embedded == nullptr) @@ -356,27 +367,26 @@ void BlockComponent::setDisplayModeInternal (DisplayMode mode, bool force) if (embedded != nullptr) { - block.addAndMakeVisible (embedded.get()); - block.updateSize(); - block.resized(); - embedded->addComponentListener (&block); + block->addAndMakeVisible (embedded.get()); + block->updateSize(); + block->resized(); + embedded->addComponentListener (block); } else { if (oldMode != Embed) - block.setDisplayModeInternal (oldMode, true); + block->setDisplayModeInternal (oldMode, true); } } - BlockComponent& block; + Component::SafePointer block; Node node; UI& ui; - PtrType& embedded; DisplayMode oldMode; }; if (auto* ui = ViewHelpers::getGuiController (this)) - (new EmbedBockAsync (*this, node, *ui, this->embedded, oldMode))->post(); + (new EmbedBockAsync (*this, node, *ui, oldMode))->post(); } else { @@ -1230,7 +1240,7 @@ void BlockComponent::updateSize() { if (detail::canResize (*this) && customWidth > 0 && customHeight > 0) { - setSize (customWidth, customHeight); + setSize (jmin (customWidth, maxBlockWidth), jmin (customHeight, maxBlockHeight)); resized(); } else @@ -1254,10 +1264,8 @@ void BlockComponent::setCustomSize (int width, int height) { int mw = width, mh = height; getMinimumSize (mw, mh); - if (width < mw) - width = mw; - if (height < mh) - height = mh; + width = jlimit (mw, maxBlockWidth, width); + height = jlimit (mh, maxBlockHeight, height); if (customWidth != width || customHeight != height) { diff --git a/src/ui/pluginmanagercomponent.cpp b/src/ui/pluginmanagercomponent.cpp index 58fb83e1e..f68ff6503 100644 --- a/src/ui/pluginmanagercomponent.cpp +++ b/src/ui/pluginmanagercomponent.cpp @@ -582,6 +582,7 @@ PluginListComponent::PluginListComponent (PluginManager& p, PropertiesFile* prop PluginListComponent::~PluginListComponent() { + plugins.removeChangeListener (this); list.removeChangeListener (this); } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6c4a87153..9c99a5e3e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -70,6 +70,7 @@ add_test(NAME "DSPScriptTest" COMMAND test_element --run_test=DSPScriptTest) add_test(NAME "Element" COMMAND test_element --run_test=Element) add_test(NAME "GraphNodeTests" COMMAND test_element --run_test=GraphNodeTests) add_test(NAME "GzipTests" COMMAND test_element --run_test=GzipTests) +add_test(NAME "LatencyClampTests" COMMAND test_element --run_test=LatencyClampTests) add_test(NAME "IONodeTests" COMMAND test_element --run_test=IONodeTests) add_test(NAME "LinearFadeTest" COMMAND test_element --run_test=LinearFadeTest) add_test(NAME "MidiChannelMapTest" COMMAND test_element --run_test=MidiChannelMapTest) diff --git a/test/LatencyClampTests.cpp b/test/LatencyClampTests.cpp new file mode 100644 index 000000000..46847b283 --- /dev/null +++ b/test/LatencyClampTests.cpp @@ -0,0 +1,62 @@ +// SPDX-FileCopyrightText: Copyright (C) Kushview, LLC. +// SPDX-License-Identifier: GPL-3.0-or-later + +#include + +#include +#include + +#include "engine/graphnode.hpp" +#include "fixture/TestNode.h" +#include "testutil.hpp" + +using namespace element; + +namespace { +struct LatencyNode : public TestNode +{ + using TestNode::setLatencySamples; +}; +} // namespace + +BOOST_AUTO_TEST_SUITE (LatencyClampTests) + +BOOST_AUTO_TEST_CASE (LatencySamplesAreClamped) +{ + LatencyNode node; + node.setLatencySamples (512); + BOOST_REQUIRE_EQUAL (node.getLatencySamples(), 512); + + node.setLatencySamples (-100); + BOOST_REQUIRE_EQUAL (node.getLatencySamples(), 0); + + node.setLatencySamples (std::numeric_limits::max()); + BOOST_REQUIRE_EQUAL (node.getLatencySamples(), Processor::maxLatencySamples); +} + +BOOST_AUTO_TEST_CASE (DelayCompensationSurvivesRestoreBeforePrepare) +{ + // GraphManager-style refcounting means the graph must live on the heap. + ProcessorPtr keep (new GraphNode (*element::test::context())); + auto& graph = *static_cast (keep.get()); + + auto* node = graph.addNode (new TestNode (2, 2, 0, 0)); + BOOST_REQUIRE (node != nullptr); + + // State is restored before the node is prepared during a session load, + // so the sample rate is unknown at this point. + node->setDelayCompensation (10.0); + BOOST_REQUIRE_EQUAL (node->getDelayCompensation(), 10.0); + BOOST_REQUIRE_EQUAL (node->getDelayCompensationSamples(), 0); + + graph.prepareToRender (48000.0, 512); + BOOST_REQUIRE_EQUAL (node->getDelayCompensationSamples(), 480); + + node->setDelayCompensation (1.0e9); + BOOST_REQUIRE_EQUAL (node->getDelayCompensationSamples(), Processor::maxLatencySamples); + + graph.releaseResources(); + graph.clear(); +} + +BOOST_AUTO_TEST_SUITE_END()