Site navigation

getRawParameterValue() vs getParameter()

General JUCE discussion
#1

In this commit, @tpoole made the audio parameter classes thread safe by backing with a std::atomic<float> rather than a float.

However, the documentation for AudioProcessorValueTreeState still states:

    /** Returns a parameter by its ID string. */
    RangedAudioParameter* getParameter (StringRef parameterID) const noexcept;

    /** Returns a pointer to a floating point representation of a particular parameter which a realtime process can read to find out its current value.
    */
    std::atomic<float>* getRawParameterValue (StringRef parameterID) const noexcept;

Digging into the implementation, they both call getParameterAdapter (paramID). Then the “Raw” version returns a pointer to a cached std::atomic<float> and the non-raw version returns a pointer to the underlying RangedAudioParameter

It seems to me that now that the RangedAudioParameter classes are backed by an atomic, calling getParameter() from a realtime process should also be safe.

Can anyone confirm?

Assuming I’m correct, is there any reason to prefer getRawParameterValue() besides avoiding the overhead of converting between normalised / non-normalised values?

1 Like
#2

You are correct, and it’s best to avoid getRawParameter().
It is not descriptive, and moving forward using the actual parameter brings more options to the table, e.g. if one day sample accurate automation will be implemented, I can imagine a
parameter.getValueAtSample (x) or similar, like an
applyParameterCurve (buffer, channel, factor=1.0f)… a lot can be imagined.

1 Like
#3

Thanks for confirming @Daniel. Yes, those additional possibilities do sound very appealing!

#4

Actually… I’ve just looked into this a bit more deeply, and I think neither getParameter() nor getRawParameter() is safe for read from a different thread.

An obvious initial concern is that the ParameterAdapter which wraps the parameter values are stored in a non-const std::map. This could be modified during a call to getParameterAdapter() by a concurrent call to createAndAddParameter() thereby invalidating the iterator used by std::map::find(). I think this could potentially be avoided by reserving memory for the std::map in advance.

Besides that the AudioProcessorValueTreeState could get deleted after a call to getParameter() and then caller is left with a dangling pointer. I haven’t thought this through fully, but wouldn’t it be safer for getParameter() to return a std::weak_ptr<RangedAudioParameter> ?

Or am I missing something?

#5

The AudioValueTreeState is not meant to be recreated for the lifetime of the AudioProcessor.
Doing that will result in more problems than only the getParameter().

By using the AudioProcessorValueTreeState as a member of the AudioProcessor this is guaranteed. This is why it is discouraged to use it in a unique_ptr (or worse raw pointer).

#6

I wasn’t talking about recreating the APVTS, just concurrent access to the RangedAudioParameter* during or after the APVTS destructor is called. But this is actually pretty hard to achieve in sensible usage.

Regarding my concern about the std::map::iterator being invalidated, this is actually impossible in debug builds because an assertion will fire if an attempt is made to add parameters after the underlying ValueTree has been set. After the parameters are created, there is no other way to modify the map.

In short, I think you’re right, so long as the APVTS class is used as intended, getParameter() seems safe. Which is a relief!

#7

Just came across this thread, and it’s interesting to consider making that change in my own code now.

Obviously this isn’t the only consideration, but at first blush, it seems that code would be less easy to read when using getParameter. If I want the “unit” value for a float parameter (i.e., its non-normalized value), I can get it this way:

freqParameter = apvts.getRawParameterValue ("freq");  // in constructor

*freqParameter  // anywhere in processBlock code

But to get that non-normalized float value using getParameter, requires doing this:

static_cast<AudioParameterFloat*>(apvts.getParameter("freq"))->get()  // anywhere in processBlock code

Or this:

apvts.getParameter("freq")->convertFrom0to1 (apvts.getParameter("freq")->getValue())

(Yes you can do this, but it returns the normalized (0-1) value:)

apvts.getParameter("freq")->getValue()