From 6f949c613ee5b7e80153a84becf56efcc040e49c Mon Sep 17 00:00:00 2001 From: jatinchowdhury18 Date: Sun, 14 Jan 2024 11:09:47 -0800 Subject: [PATCH] Improving automation stability for forwarded parameters (#344) --- src/headless/CMakeLists.txt | 1 + .../tests/ForwardingParamStabilityTest.cpp | 118 ++++++++++++++++++ src/processors/BaseProcessor.cpp | 3 + src/processors/BaseProcessor.h | 6 +- src/state/ParamForwardManager.cpp | 84 ++++++++----- 5 files changed, 182 insertions(+), 30 deletions(-) create mode 100644 src/headless/tests/ForwardingParamStabilityTest.cpp diff --git a/src/headless/CMakeLists.txt b/src/headless/CMakeLists.txt index 64639264..48b70a78 100644 --- a/src/headless/CMakeLists.txt +++ b/src/headless/CMakeLists.txt @@ -18,6 +18,7 @@ target_sources(BYOD_headless PRIVATE tests/AmpIRsSaveLoadTest.cpp tests/BadModulationTest.cpp + tests/ForwardingParamStabilityTest.cpp tests/NaNResetTest.cpp tests/ParameterSmoothTest.cpp tests/PreBufferTest.cpp diff --git a/src/headless/tests/ForwardingParamStabilityTest.cpp b/src/headless/tests/ForwardingParamStabilityTest.cpp new file mode 100644 index 00000000..ca4fdc97 --- /dev/null +++ b/src/headless/tests/ForwardingParamStabilityTest.cpp @@ -0,0 +1,118 @@ +#include "UnitTests.h" +#include "processors/chain/ProcessorChainActionHelper.h" +#include "state/ParamForwardManager.h" + +class ForwardingParamStabilityTest : public UnitTest +{ +public: + ForwardingParamStabilityTest() + : UnitTest ("Forwarding Parameter Stability Test") + { + } + + bool addProcessor (ProcessorChain& chain, UndoManager* um) + { + // get random processor + auto& storeMap = ProcessorStore::getStoreMap(); + auto storeIter = storeMap.begin(); + int storeIndex = rand.nextInt ((int) storeMap.size()); + std::advance (storeIter, storeIndex); + + auto& actionHelper = chain.getActionHelper(); + actionHelper.addProcessor (storeIter->second.factory (um)); + + return true; + } + + bool removeProcessor (ProcessorChain& chain) + { + const auto& procs = chain.getProcessors(); + if (procs.isEmpty()) + return false; + + auto procIndexToRemove = rand.nextInt (procs.size()); + auto* procToRemove = procs[procIndexToRemove]; + + auto& actionHelper = chain.getActionHelper(); + actionHelper.removeProcessor (procToRemove); + + return true; + } + + using ParamNames = std::array; + auto runPlugin() + { + BYOD plugin; + auto* undoManager = plugin.getVTS().undoManager; + auto& procChain = plugin.getProcChain(); + + struct Action + { + String name; + std::function action; + }; + + std::vector actions { + { "Add Processor", [&] + { return addProcessor (procChain, undoManager); } }, + { "Remove Processor", [&] + { return removeProcessor (procChain); } }, + }; + + for (int count = 0; count < 100;) + { + auto& action = actions[rand.nextInt ((int) actions.size())]; + if (action.action()) + { + int timeUntilNextAction = rand.nextInt ({ 5, 500 }); + juce::MessageManager::getInstance()->runDispatchLoopUntil (timeUntilNextAction); + count++; + } + } + + auto& forwardingParams = plugin.getParamForwarder().getForwardedParameters(); + ParamNames paramNames {}; + for (auto [idx, param] : chowdsp::enumerate (forwardingParams)) + { + if (auto* forwardedParam = param->getParam()) + paramNames[idx] = forwardedParam->getName (1024).toStdString(); + } + + juce::MemoryBlock state; + plugin.getStateInformation (state); + + return std::make_tuple (paramNames, state); + } + + void testPlugin (const ParamNames& paramNames, const juce::MemoryBlock& state) + { + BYOD plugin; + plugin.setStateInformation (state.getData(), (int) state.getSize()); + + auto& forwardingParams = plugin.getParamForwarder().getForwardedParameters(); + for (auto [idx, param] : chowdsp::enumerate (forwardingParams)) + { + if (auto* forwardedParam = param->getParam()) + expectEquals (forwardedParam->getName (1024).toStdString(), + paramNames[idx], + "Parameter name mismatch"); + else + expectEquals (std::string {}, + paramNames[idx], + "Parameter name mismatch"); + } + } + + void runTest() override + { + rand = Random { 1234 }; // getRandom(); + + beginTest ("Forwarding Parameter Stability Test"); + const auto [paramNames, state] = runPlugin(); + testPlugin (paramNames, state); + } + + Random rand; +}; + +static ForwardingParamStabilityTest forwardingParamStabilityTest; diff --git a/src/processors/BaseProcessor.cpp b/src/processors/BaseProcessor.cpp index f58c4cb8..bdf645dd 100644 --- a/src/processors/BaseProcessor.cpp +++ b/src/processors/BaseProcessor.cpp @@ -139,6 +139,7 @@ std::unique_ptr BaseProcessor::toXML() xml->setAttribute ("x_pos", (double) editorPosition.x); xml->setAttribute ("y_pos", (double) editorPosition.y); + xml->setAttribute ("forwarding_params_offset", forwardingParamsStartIndex); if (netlistCircuitQuantities != nullptr) { @@ -204,6 +205,8 @@ void BaseProcessor::loadPositionInfoFromXML (XmlElement* xml) auto xPos = (float) xml->getDoubleAttribute ("x_pos"); auto yPos = (float) xml->getDoubleAttribute ("y_pos"); editorPosition = juce::Point { xPos, yPos }; + + forwardingParamsStartIndex = xml->getIntAttribute ("forwarding_params_offset", -1); } void BaseProcessor::addConnection (ConnectionInfo&& info) diff --git a/src/processors/BaseProcessor.h b/src/processors/BaseProcessor.h index f2155156..b2083ebb 100644 --- a/src/processors/BaseProcessor.h +++ b/src/processors/BaseProcessor.h @@ -186,6 +186,8 @@ class BaseProcessor : private JuceProcWrapper juce::Point getPosition (Rectangle parentBounds); const auto& getParameters() const { return AudioProcessor::getParameters(); } + int getForwardingParametersIndexOffset() const noexcept { return forwardingParamsStartIndex; } + void setForwardingParametersIndexOffset (int offset) { forwardingParamsStartIndex = offset; } bool isOutputModulationPortConnected(); const std::vector* getParametersToDisableWhenInputIsConnected (int portIndex) const noexcept; @@ -296,7 +298,7 @@ class BaseProcessor : private JuceProcWrapper PortMagnitude() = default; PortMagnitude (PortMagnitude&&) noexcept {} - chowdsp::LevelDetector smoother; + chowdsp::LevelDetector smoother {}; Atomic currentMagnitudeDB; }; @@ -312,5 +314,7 @@ class BaseProcessor : private JuceProcWrapper std::unordered_map> paramsToDisableWhenInputConnected {}; std::unordered_map> paramsToEnableWhenInputConnected {}; + int forwardingParamsStartIndex = -1; + JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (BaseProcessor) }; diff --git a/src/state/ParamForwardManager.cpp b/src/state/ParamForwardManager.cpp index 8c7e14a8..293aa191 100644 --- a/src/state/ParamForwardManager.cpp +++ b/src/state/ParamForwardManager.cpp @@ -1,7 +1,8 @@ #include "ParamForwardManager.h" -ParamForwardManager::ParamForwardManager (AudioProcessorValueTreeState& vts, ProcessorChain& procChain) : chowdsp::ForwardingParametersManager (vts), - chain (procChain) +ParamForwardManager::ParamForwardManager (AudioProcessorValueTreeState& vts, ProcessorChain& procChain) + : chowdsp::ForwardingParametersManager (vts), + chain (procChain) { // In some AUv3 hosts (cough, cough, GarageBand), sending parameter info change notifications // causes the host to crash. Since there's no way for the plugin to determine which AUv3 @@ -50,32 +51,50 @@ void ParamForwardManager::processorAdded (BaseProcessor* proc) auto& procParams = proc->getParameters(); const auto numParams = procParams.size(); - // Find a range in forwardedParams with numParams empty params in a row - int count = 0; - for (int i = 0; i < (int) forwardedParams.size(); ++i) + const auto setForwardParameterRange = [this, &procParams, &proc, numParams] (int startOffset) { - if (forwardedParams[i]->getParam() == nullptr) - count++; - else - count = 0; + setParameterRange (startOffset, + startOffset + numParams, + [&procParams, &proc, startOffset] (int index) -> chowdsp::ParameterForwardingInfo + { + auto* procParam = procParams[index - startOffset]; + + if (auto* paramCast = dynamic_cast (procParam)) + return { paramCast, proc->getName() + ": " + paramCast->name }; + + jassertfalse; + return {}; + }); + }; - if (count == numParams) + if (const auto savedOffset = proc->getForwardingParametersIndexOffset(); savedOffset >= 0) + { + setForwardParameterRange (savedOffset); + } + else + { + // Find a range in forwardedParams with numParams empty params in a row + int count = 0; + for (int i = 0; i < (int) forwardedParams.size(); ++i) { - int startOffset = i + 1 - numParams; - setParameterRange (startOffset, - startOffset + numParams, - [&procParams, &proc, startOffset] (int index) -> chowdsp::ParameterForwardingInfo - { - auto* procParam = procParams[index - startOffset]; - - if (auto* paramCast = dynamic_cast (procParam)) - return { paramCast, proc->getName() + ": " + paramCast->name }; - - jassertfalse; - return {}; - }); - - break; + if (forwardedParams[i]->getParam() == nullptr) + count++; + else + count = 0; + + if (count == numParams) + { + int startOffset = [i, numParams, proc] + { + const auto savedOffset = proc->getForwardingParametersIndexOffset(); + if (savedOffset >= 0) + return savedOffset; + return i + 1 - numParams; + }(); + proc->setForwardingParametersIndexOffset (startOffset); + setForwardParameterRange (startOffset); + break; + } } } } @@ -84,12 +103,19 @@ void ParamForwardManager::processorRemoved (const BaseProcessor* proc) { auto& procParams = proc->getParameters(); - for (auto [index, param] : sst::cpputils::enumerate (forwardedParams)) + if (const auto savedOffset = proc->getForwardingParametersIndexOffset(); savedOffset >= 0) + { + clearParameterRange (savedOffset, savedOffset + procParams.size()); + } + else { - if (auto* internalParam = param->getParam(); internalParam == procParams[0]) + for (auto [index, param] : sst::cpputils::enumerate (forwardedParams)) { - clearParameterRange ((int) index, (int) index + procParams.size()); - break; + if (auto* internalParam = param->getParam(); internalParam == procParams[0]) + { + clearParameterRange ((int) index, (int) index + procParams.size()); + break; + } } } }