AudioProcessorValueTreeState && thread-safety

This is fine if your parameter listeners are only needed to update the UI, but if you need those callbacks for altering the processing in some other places of your plug-in, you’re going to have big issues during offline bounces/rendering of tracks.

Cakewalk SONAR, for one, completely grabs the message thread during those, and runs the rendering in another thread, so your callbacks will queue up until the end of the offline rendering, when serving them may be too late.

I think two sets of listeners may be needed, one for sync notifications and another for asynchronous ones, or the possibility to specify, when calling addListener(), whether the given listener should be notified synchronously on whatever thread, or possibly asynchronously on the message thread.

Something in that direction was proposed here: Async addition to ListenerList

Ouch - good to know.

Does that mean it may not get a run of the message thread after setStateInformation is called?

This is seeming like a better idea by the minute

1 Like

Well… in the specific context of an offline bounce (let’s call it a “non-realtime render”), I don’t think the DAW has reasons to call setStateInformation(): from the DAW point of view, that has to do with restoring session state and/or recalling previously saved states of the plug-in, which is not something that happens during a render.

On the other hand, you may very well need to call setStateInformation() for your own purposes during a render, for example to react to the reception of a MIDI Program Change, which may be programmed to happen at a given time during your song.
In that case, I confirm that SONAR will not allow the message thread to run anytime soon after you queue that call for async processing.

As a test, I have also tried blocking and waiting after posting one such notification, to see after how long the message thread would have picked it up (with a generous timeout of 10 seconds). I found that sometimes it gets picked up after some hundreds of milliseconds (which still slows things down terribly), but more often than not the message thread doesn’t even pick them up at all within the given timeout.

I think we had a situation with a non-DAW host which was calling setStateInformation right before a render commenced. But I can’t remember the details. I’ll ask Doug who was working on it.

Anyway I’ve half re-written AudioProcessorValueTreeState this morning to take out the ValueTree. Instead it creates one when required:

/**
 * Thread-safe, return the current state of the processor configuration.
 */
ValueTree toValueTree () const;

I’m just redoing the attachment classes. Does anyone understand this self-callback-mutex? Unless I’m mistaken setValue and comboBoxChanged here should only really be called on the message thread anyway, so what’s the lock all about?

struct AudioProcessorValueTreeState::ComboBoxAttachment::Pimpl  : private AttachedControlBase,
                                                                  private ComboBox::Listener
{
    Pimpl (AudioProcessorValueTreeState& s, const String& p, ComboBox& c)
        : AttachedControlBase (s, p), combo (c), ignoreCallbacks (false)
    {
        sendInitialUpdate();
        combo.addListener (this);
    }

    ~Pimpl()
    {
        combo.removeListener (this);
        removeListener();
    }

    void setValue (float newValue) override
    {
        const ScopedLock selfCallbackLock (selfCallbackMutex);

        {
            ScopedValueSetter<bool> svs (ignoreCallbacks, true);
            combo.setSelectedItemIndex (roundToInt (newValue), sendNotificationSync);
        }
    }

    void comboBoxChanged (ComboBox* comboBox) override
    {
        const ScopedLock selfCallbackLock (selfCallbackMutex);

        if (! ignoreCallbacks)
        {
            beginParameterChange();
            setNewUnnormalisedValue ((float) comboBox->getSelectedId() - 1.0f);
            endParameterChange();
        }
    }

    ComboBox& combo;
    bool ignoreCallbacks;
    CriticalSection selfCallbackMutex;

    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (Pimpl)
};

It would get called synchronously from setValue() above, via ComboBox::setSelectedItemIndex()

Yes, but on the same thread so it won’t lock?

Couldn’t setValue() be called from another thread?

I dont’ think so:

void parameterChanged (const String&, float newValue) override
{
    lastValue = newValue;

    if (MessageManager::getInstance()->isThisTheMessageThread())
    {
        cancelPendingUpdate();
        setValue (newValue);
    }
    else
    {
        triggerAsyncUpdate();
    }
}

The Pimpl structure of these classes really doesn’t help either…

Okay, some work still to do, but something like this:

I’d like to know how things have progressed and whether it’s worth going with AudioProcessorValueTreeState

3 Likes

We are still working on this. That’s all I can say, sorry!

Considering AudioProcessorValueTreeState has issues, what’s the currently recommended way to manage AudioProcessorParameters in a thread-safe manner in an audio plug-in?

2 Likes

you could just use AudioProcessorParameterWithID or, some of its derived classes AudioParameterFloat/Bool/Choice like the DSP Demo

Hi Fabian. Could this be helpful as a starting point? https://github.com/getdunne/juce-AudioProcessorValueTreeStateTest
I can’t claim to be an expert, but I’m also very interested to create “best practice” samples for JUCE. With so few available it’s a tough learning curve.

I pushed some functionality that addresses these concerns a few days ago:

The (get/set)StateInformation functions should now look like

    void getStateInformation (MemoryBlock& destData) override
    {
        auto state = parameters.copyState();
        ScopedPointer<XmlElement> xml (state.createXml());
        copyXmlToBinary (*xml, destData);
    }
    
    void setStateInformation (const void* data, int sizeInBytes) override
    {
        ScopedPointer<XmlElement> xmlState (getXmlFromBinary (data, sizeInBytes));
        
        if (xmlState != nullptr)
            if (xmlState->hasTagName (parameters.state.getType()))
                parameters.replaceState (ValueTree::fromXml (*xmlState));
    }

and we’ll update the tutorial once this change makes it to the master branch.

1 Like

I think it should be mentioned in the doc of those new methods that thread safety is obtained using locks, hence it is thread-safe but not realtime-safe

Yes, that’s probably a good idea.

You really shouldn’t be copying or replacing the state ValueTree on a realtime thread in the first place though!

Certainly! and that’s why I propose to add that to the doc, otherwise someone may be tempted to do that in order to load a preset in response to MIDI Program Change messages, which are received by the audio callback on the realtime thread

2 Likes