Improving automation stability for forwarded parameters (#344)

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

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<std::string, 500>;
+    auto runPlugin()
+    {
+        BYOD plugin;
+        auto* undoManager = plugin.getVTS().undoManager;
+        auto& procChain = plugin.getProcChain();
+
+        struct Action
+        {
+            String name;
+            std::function<bool()> action;
+        };
+
+        std::vector<Action> 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;

src/processors/BaseProcessor.cpp

@@ -139,6 +139,7 @@ std::unique_ptr<XmlElement> 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)

src/processors/BaseProcessor.h

@@ -186,6 +186,8 @@ class BaseProcessor : private JuceProcWrapper
     juce::Point<int> getPosition (Rectangle<int> 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<String>* getParametersToDisableWhenInputIsConnected (int portIndex) const noexcept;
@@ -296,7 +298,7 @@ class BaseProcessor : private JuceProcWrapper
         PortMagnitude() = default;
         PortMagnitude (PortMagnitude&&) noexcept {}
 
-        chowdsp::LevelDetector<float> smoother;
+        chowdsp::LevelDetector<float> smoother {};
         Atomic<float> currentMagnitudeDB;
     };
 
@@ -312,5 +314,7 @@ class BaseProcessor : private JuceProcWrapper
     std::unordered_map<int, std::vector<String>> paramsToDisableWhenInputConnected {};
     std::unordered_map<int, std::vector<String>> paramsToEnableWhenInputConnected {};
 
+    int forwardingParamsStartIndex = -1;
+
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (BaseProcessor)
 };

src/state/ParamForwardManager.cpp

@@ -1,7 +1,8 @@
 #include "ParamForwardManager.h"
 
-ParamForwardManager::ParamForwardManager (AudioProcessorValueTreeState& vts, ProcessorChain& procChain) : chowdsp::ForwardingParametersManager<ParamForwardManager, 500> (vts),
-                                                                                                          chain (procChain)
+ParamForwardManager::ParamForwardManager (AudioProcessorValueTreeState& vts, ProcessorChain& procChain)
+    : chowdsp::ForwardingParametersManager<ParamForwardManager, 500> (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<RangedAudioParameter*> (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<RangedAudioParameter*> (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;
+            }
         }
     }
 }