Attached Controls shouldn't send notification on setValue()

Audio Plugins
1

I’ve been digging into an issue where my plugins won’t respond to automation very well in some hosts (Sonar, FL Studio mostly)

After mistakenly thinking it was something to do with AsyncUpdate, I believe I’ve now actually found the issue.

When a parameter is changed, an asyncUpdate gets dispatched, which causes setValue() in SliderAttachment::Pimpl to be called, which sets the value of the Slider. In the sliders listener, the parameter is set which ends up calling setValueNotifyingHost() which causes some hosts to have a heart attack.

I think setValueNotifyingHost() should only be called when the user moves a control or the plugin state changes. It shouldn’t be getting called in response to automation causing setParameter() to be called.

So in juce_AudioProcessorValueTreeState.cpp, I think in the void setValue (float newValue) override functions should set their underlying control with the dontSendNotification flag.

1 Like
2

This change has the problem, that eventually you use more than one listener on the slider, and the others are not the host. So they wouldn’t get any update from the host’s automation.

I think a host should be able to cope with a plugin that wants to set a value the parameter already has…
But I see that this is not a solution for you, because you don’t develop the host…

NB: sure, eventually these other listeners could be listeners of the parameter instead of the sliders, but that’s a big change, so many peoples code would break and it would end up like the AudioProcessorListener, that is only useful for the host, as I understand it

3

What about something like this:

void setValue (float newValue) override
{
    ScopedValueSetter<bool> setter (inSetValue, true);
    slider.setValue (newValue, sendNotificationSync);
}

void sliderValueChanged (Slider* s) override
{
    if (! inSetValue && ! ModifierKeys::getCurrentModifiers().isRightButtonDown())
        setNewUnnormalisedValue ((float) s->getValue());
}

bool inSetValue = false;

Then other callbacks will still get called, but the host callback won’t.

4

Actually if you are attaching a slider to a parameter and then attaching a listener to the slider, could you not argue that everything should be attaching to the parameter NOT the slider?Otherwise you’re basically creating a scenario where you have multiple masters. IMO the parameter should be the one and only master and everything should listen to the parameter change.

5

That’s the way I always do it, but I can see the case where you might want to attach to the slider, if you want some part of the UI to update when the user is interacting with the UI. Popup parameter readout maybe, you’d want it to popup when the user is moving the slider, but not when it’s moving by automation.

6

I totally agree G-Mon which is why I would agree that your original suggestion is probably the right one, i.e. that in juce_AudioProcessorValueTreeState the dontSendNotification flag should be used.

I was merely responding to @daniel, in that if someone is expecting to listen to a slider to get parameter changes then I think it’s defensible to suggest that is the wrong way to handle it, however I can see why someone would attempt exactly that. Instead whatever is listening to the slider should also listen to the parameter changes IMO.

7

I am totally with both of you, that the better design will always listen to the parameter directly (that’s why I added the last paragraph of my first response).
I was only pointing out, that it breaks peoples code, that relied to the current behaviour.

If there is a situation, where the current behaviour is needed, is probably a futile discussion. I’m sure you can make up situations to fit for each side.

I’m in the luxury position not to take decisions for juce, so I leave it to them…

1 Like
8

I’ll look into this in more detail later today (need to wait until my FL studio download finishes :slight_smile:). Using AudioProcessorValueTreeState and SlideAttachments etc. is really the idiomatic way to do parameters nowadays and JUCE should really work when using these.

9

I’m still having FL Studio automation issues, so that might be something else. But it did fix my Sonar issues.

10

OK I just tried this in Sonar and I can’t seem to find anything wrong. I’ve modified JUCE’s audio demo plugin to use ValueTrees. Check out the following code:

https://drive.google.com/open?id=0B3yZpQ3OG9ShQ0tWd3l1NExLdEE

11

Ok, I reproduced it in the demo plugin, see here: https://www.youtube.com/watch?v=IuhYM3zdMBk

I added a Component that redraws itself all the time to add a bit of CPU usage. I set the range of the param to be: 0.1f, 100.0f, 0.001f

I think the range is they key, there needs to be a bit of rounding error so that p->getValue() != newValue even when the value hasn’t changed. There are different by about 1e-8

        const float newValue = state.getParameterRange (paramID)
                                  .convertTo0to1 (newUnnormalisedValue);

        if (p->getValue() != newValue)
            p->setValueNotifyingHost (newValue);

Updated code link: https://www.dropbox.com/s/d3y47mezdbm8l1w/audio%20plugin%20demo.zip?dl=0

12

Here it is in FL Studio 12.3: https://www.youtube.com/watch?v=y3cuCYK5RJk&feature=youtu.be

The slider doesn’t even move unless I click on the automation clip. These are both on Windows 10 BTW.

13

Did you manage to reproduce the issue with my updated demo plugin code?

14

Still on this… I can reproduce this in FL Studio but not in Sonar. As you’ve mentioned, FL Studio does a lot of strange stuff with parameters.

15

I’m using Sonar Platinum 22.5.0 BUILD 45 [2016.05] - x64 on Windows 10
I’m using the VST2 64 bit build of the demo plugin I posted, build against the latest develop code

I’m automating gain and it looks like this:

Pasted image980×396 24 KB

16

I think I’ve found the issue with FL Studio, not sure how to fix. FL Studio stops automating the parameter whenever setValueNotifyingHost() is called.

I think there is a race condition with the AudioProcessorValueTreeState. setValue() is called by the engine thread and updates the value variable, which triggers a timer that copies the new value into the ValueTree. The timer, in the message thread, calls updateFromValueTree() compares the newValue against value (which by now may have been updated by the engine) and if they are different, calls setValueNotifyingHost().

Not sure the correct way to fix this, but the plugin should never call setValueNotifyingHost() unless the plugin is causing a parameter value to change, either through user UI interaction or loading a new preset, etc.

17

I know you guys have been busy, but will you have a chance to look at this soon? I’m hoping to get a plugin released by the end of the month. Here is a video of automation stopping in Studio One 3.3.1. It takes a while to happen, it took about 3 minutes of playback before the issue showed up. https://www.youtube.com/watch?v=vzP2Zbag3ec&feature=youtu.be

Here is the updated sample plugin code. You wouldn’t have been able to reproduce the issue in the previous code I uploaded unless you were loading a preset first. The constructor of JuceDemoPluginAudioProcessor wasn’t giving the state a valid ValueTree. I also made the CPUWaster class auto adjust, maybe you have a faster machine than I do and it wasn’t wasting enough CPU.

18

This is just a heads-up that I’ve been looking at this for a while. It’s quite a tricky problem, but a fix is coming.

1 Like
19

I have an ugly hack that seems to have fixed it.

void copyValueToValueTree()
{
    if (state.isValid())
    {
        updateLocked = true;
        state.setProperty (owner.valuePropertyID, value, owner.undoManager);
        updateLocked = false;
    }
}

and

// This method can't be used until the parameter has been attached to a processor!
jassert (processor != nullptr && parameterIndex >= 0);

if (! updateLocked)
    processor->setParameterNotifyingHost (parameterIndex, newValue);

}

I find it hard to follow the logic of how the parameter system works with all the timers, callbacks, listeners, etc, etc. But as far as I understand, when a parameter gets updated, sometime later that value gets copied into the value tree. This should never get sent back to the host, so I have it blocked. Any other time the value tree is updated, the host does get notified.

20

That probably fixes it for the majority of cases. The trouble is that a simple solution like this is not thread safe - the copyValueToValueTree method is always called on the message thread, but you could update a parameter from any thread. If you’re unlucky then an important parameter update on a different thread would be missed.