Run parameter listeners synchronously when appropriate (#538)

* Run parameter listeners synchronously when the parameter change comes from the message thread

* Update tests

* Apply clang-format

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

modules/plugin/chowdsp_plugin_state/Backend/chowdsp_ParameterListeners.cpp

@@ -2,21 +2,50 @@
 
 namespace chowdsp
 {
-ParameterListeners::ParameterListeners (ParamHolder& parameters, int interval)
+ParameterListeners::ParameterListeners (ParamHolder& parameters,
+                                        const juce::AudioProcessor* parentProcessor,
+                                        int interval)
     : totalNumParams ((size_t) parameters.count())
 {
     parameters.doForAllParameters (
-        [this] (auto& param, size_t index)
+        [this, parentProcessor] (auto& param, size_t indexInParamHolder)
         {
-            const auto* rangedParam = static_cast<juce::RangedAudioParameter*> (&param);
+            auto* rangedParam = static_cast<juce::RangedAudioParameter*> (&param);
+            const auto index = parentProcessor != nullptr ? static_cast<size_t> (rangedParam->getParameterIndex()) : indexInParamHolder;
             paramInfoList[index] = ParamInfo { rangedParam, rangedParam->getValue() };
+
+            if (parentProcessor != nullptr)
+                rangedParam->addListener (this);
         });
 
     startTimer (interval);
 }
 
+ParameterListeners::~ParameterListeners()
+{
+    for (auto& paramInfo : paramInfoList)
+        paramInfo.paramCookie->removeListener (this);
+}
+
+void ParameterListeners::parameterValueChanged (int paramIndex, float newValue)
+{
+    if (! juce::MessageManager::getInstance()->isThisTheMessageThread())
+        return; // this will be handled by the timer callback!
+
+    auto index = static_cast<size_t> (paramIndex);
+    auto& paramInfo = paramInfoList[index];
+    paramInfo.value = newValue;
+    audioThreadBroadcastQueue.try_enqueue ([this, i = index]
+                                           { callAudioThreadBroadcaster (i); });
+    callMessageThreadBroadcaster (index);
+}
+
 void ParameterListeners::timerCallback()
 {
+    // If the parameters are attached to a processor then most of the listeners
+    // callbacks will be handled there. The point of this timer is to handle
+    // parameter changes that might have come from the audio thread, or for
+    // parameters that may not be connected to a processor.
     updateBroadcastersFromMessageThread();
 }
 

modules/plugin/chowdsp_plugin_state/Backend/chowdsp_ParameterListeners.h

@@ -28,11 +28,15 @@ enum class ParameterListenerThread
 };
 
 /** Utility class to manage a set of parameter listeners. */
-class ParameterListeners : private juce::Timer
+class ParameterListeners : private juce::Timer,
+                           private juce::AudioProcessorParameter::Listener
 {
 public:
     /** Initialises the listeners with a set of parameters. */
-    explicit ParameterListeners (ParamHolder& parameters, int intervalMilliseconds = 10);
+    explicit ParameterListeners (ParamHolder& parameters,
+                                 const juce::AudioProcessor* parentProcessor = nullptr,
+                                 int intervalMilliseconds = 20);
+    ~ParameterListeners() override;
 
     /**
      * Runs any queued listeners on the audio thread.
@@ -70,11 +74,14 @@ class ParameterListeners : private juce::Timer
 private:
     void callMessageThreadBroadcaster (size_t index);
     void callAudioThreadBroadcaster (size_t index);
+
     void timerCallback() override;
+    void parameterValueChanged (int, float) override;
+    void parameterGestureChanged (int, bool) override {}
 
     struct ParamInfo
     {
-        const juce::RangedAudioParameter* paramCookie = nullptr;
+        juce::RangedAudioParameter* paramCookie = nullptr;
         float value = 0.0f;
     };
 

modules/plugin/chowdsp_plugin_state/Backend/chowdsp_PluginState.h

@@ -23,9 +23,9 @@ class PluginState
         params = &parameters;
         processor = proc;
         undoManager = um;
-        listeners.emplace (parameters);
         if (processor != nullptr)
             parameters.connectParametersToProcessor (*processor);
+        listeners.emplace (parameters, processor);
     }
 
     /** Serializes the plugin state to the given MemoryBlock */

tests/plugin_tests/chowdsp_plugin_state_test/StateListenersTest.cpp

@@ -3,6 +3,26 @@
 
 TEST_CASE ("State Listeners Test", "[plugin][state][listeners]")
 {
+    struct TestProcessor : juce::AudioProcessor
+    {
+        const juce::String getName() const override { return {}; }
+        void prepareToPlay (double, int) override {}
+        void releaseResources() override {}
+        void processBlock (juce::AudioBuffer<float>&, juce::MidiBuffer&) override {}
+        double getTailLengthSeconds() const override { return 0.0; }
+        bool acceptsMidi() const override { return false; }
+        bool producesMidi() const override { return false; }
+        juce::AudioProcessorEditor* createEditor() override { return nullptr; }
+        bool hasEditor() const override { return false; }
+        int getNumPrograms() override { return 0; }
+        int getCurrentProgram() override { return 0; }
+        void setCurrentProgram (int) override {}
+        const juce::String getProgramName (int) override { return {}; }
+        void changeProgramName (int, const juce::String&) override {}
+        void getStateInformation (juce::MemoryBlock&) override {}
+        void setStateInformation (const void*, int) override {}
+    };
+
     SECTION ("Main Thread Listeners Test")
     {
         chowdsp::ParamHolder params {};
@@ -29,12 +49,46 @@ TEST_CASE ("State Listeners Test", "[plugin][state][listeners]")
         {
             mostRecentParamValue = (float) i / float (numIters - 1);
             static_cast<juce::AudioParameterFloat&> (pct) = mostRecentParamValue;
-            juce::MessageManager::getInstance()->runDispatchLoopUntil (100);
+            juce::MessageManager::getInstance()->runDispatchLoopUntil (200);
         }
 
         REQUIRE_MESSAGE (listenerCount == Catch::Approx (numIters).margin (2), "Incorrect number of listener callbacks!");
     }
 
+    SECTION ("Main Thread Synchronous Listeners Test")
+    {
+        chowdsp::ParamHolder params {};
+        chowdsp::PercentParameter::Ptr pct { "percent", "Percent", 1.0f };
+        params.add (pct);
+
+        TestProcessor processor {};
+        params.connectParametersToProcessor (processor);
+        chowdsp::ParameterListeners listeners { params, &processor };
+
+        float mostRecentParamValue = -1.0f;
+        int listenerCount = 0;
+        chowdsp::ScopedCallback listener = listeners.addParameterListener (
+            pct,
+            chowdsp::ParameterListenerThread::MessageThread,
+            [&listenerCount, &pct, &mostRecentParamValue]
+            {
+                REQUIRE_MESSAGE (juce::MessageManager::getInstance()->isThisTheMessageThread(),
+                                 "Listener called on a thread other than the message thread!");
+                REQUIRE_MESSAGE (juce::exactlyEqual (pct->getCurrentValue(), mostRecentParamValue),
+                                 "Parameter has the incorrect value when the listener is called!");
+                listenerCount++;
+            });
+
+        static constexpr int numIters = 50;
+        for (int i = 0; i < numIters; ++i)
+        {
+            mostRecentParamValue = (float) i / float (numIters - 1);
+            static_cast<juce::AudioParameterFloat&> (pct) = mostRecentParamValue;
+        }
+
+        REQUIRE_MESSAGE (listenerCount == numIters, "Incorrect number of listener callbacks!");
+    }
+
     SECTION ("Audio Thread Listeners Test")
     {
         chowdsp::ParamHolder params {};
@@ -59,7 +113,40 @@ TEST_CASE ("State Listeners Test", "[plugin][state][listeners]")
         {
             mostRecentParamValue = (float) i / float (numIters - 1);
             static_cast<juce::AudioParameterFloat&> (pct) = mostRecentParamValue;
-            juce::MessageManager::getInstance()->runDispatchLoopUntil (100);
+            juce::MessageManager::getInstance()->runDispatchLoopUntil (200);
+            listeners.callAudioThreadBroadcasters();
+        }
+
+        REQUIRE_MESSAGE (listenerCount == Catch::Approx (numIters).margin (2), "Incorrect number of listener callbacks!");
+    }
+
+    SECTION ("Audio Thread Listeners Connected To Processor Test")
+    {
+        chowdsp::ParamHolder params {};
+        chowdsp::PercentParameter::Ptr pct { "percent", "Percent", 0.0f };
+        params.add (pct);
+
+        TestProcessor processor {};
+        params.connectParametersToProcessor (processor);
+        chowdsp::ParameterListeners listeners { params, &processor };
+
+        float mostRecentParamValue = -1.0f;
+        int listenerCount = 0;
+        chowdsp::ScopedCallback listener = listeners.addParameterListener (
+            pct,
+            chowdsp::ParameterListenerThread::AudioThread,
+            [&listenerCount, &pct, &mostRecentParamValue]
+            {
+                REQUIRE_MESSAGE (juce::exactlyEqual (pct->getCurrentValue(), mostRecentParamValue),
+                                 "Parameter has the incorrect value when the listener is called!");
+                listenerCount++;
+            });
+
+        static constexpr int numIters = 50;
+        for (int i = 0; i < numIters; ++i)
+        {
+            mostRecentParamValue = (float) i / float (numIters - 1);
+            static_cast<juce::AudioParameterFloat&> (pct) = mostRecentParamValue;
             listeners.callAudioThreadBroadcasters();
         }