AU Wrapper Broken For Legacy Parameters


#1

Enabling JUCE_FORCE_USE_LEGACY_PARAM_IDS breaks AU plugins at runtime

Specifically, juce_AU_Wrapper.mm Line 1858:

// Consider yourself very unlucky if you hit this assertion. The hash code of your
// parameter ids are not unique.
jassert (! paramMap.contains (static_cast<int32> (auParamID)));

I commented out the assertion and checked the hashmap. After the loop (for (auto* param : juceParameters.params)) runs 39 times for my 39 parameters, paramMap.size() only shows the map contains one entry.

I’m on commit e89c4d5d4e5e9beed4adf7c21fc25e6b4a70e72f from develop

Seems like this issue arises from https://github.com/WeAreROLI/JUCE/commit/e05a1549f226b2ae4567551508f1082c8d220058?


#2

Specifically what is happening when I step-through:

 static String getParamID (AudioProcessorParameter* param, bool forceLegacyParamIDs) noexcept
    {
        if (auto* legacy = dynamic_cast<LegacyAudioParameter*> (param))
        {
            return legacy->getParamID();      /* <------- returns here, using the parameter's string ID */
        }
        else if (auto* paramWithID = dynamic_cast<AudioProcessorParameterWithID*> (param))
        ...

gets called, and its result is used in generateAUParameterID where it converts the value to an int via juce::String::getIntValue()… which will obviously yield 0 for every parameter I have.

Suggested correction:

 static String getParamID (AudioProcessorParameter* param, bool forceLegacyParamIDs) noexcept
    {
        if (auto* legacy = dynamic_cast<LegacyAudioParameter*> (param))
        {
            return String (legacy->idx); 
        }
        else if (auto* paramWithID = dynamic_cast<AudioProcessorParameterWithID*> (param))
        ...

#3

The next few commits should address that issue - please try using the tip of the develop branch.


#4

I know that just looking at the code would probably provide the answer, but i’m sure that a lot of us would be curious as to how you @t0m figured out the solution to this problem, and what exactly was causing it. I know for myself, I like hearing how a developer solved a problem and their explanation for what was causing it.


#5

Well as you said - looking at the code provides the answer:

There really isn’t much of a story behind this one. We noticed the same problem as @TonyAtHarrison encountered about a week ago; we were also hitting the jassert in some plug-in code I was experimenting with. It’s a little confusing because we’re dealing with three interacting situations: forceUseLegacyParamIDs from a preprocessor macro (which overrides everything), and managed vs unmanaged parameters. Additionally the existing behaviour has been broken for years, which was pretty surprising since no one has ever mentioned anything on here… but we just worked our way through the various combinations of options and made sure the right thing happened in each case.


#6

While this specific bug may not have been called out, the fact that parameters have never worked 100% properly through the various iterations of AudioProcessorParameter/AudioProcessorParameterWithID/AudioProcessorValueTreeState HAS been mentioned. Many times. For example:

Please stop reinventing parameters in the pursuit of “improving the interface” and put in the time and effort to (a) truly understand the nuances of parameter implementations for the various plug-in formats and (b) chase down the corner cases that make the difference between a 90% solution (“Look, the demo with two sliders works in one host!”) and a 100% solution (works in every major host, displays correctly for “generic” UIs, works properly with automation in all modes, works properly with control surfaces, all types of controls work properly (toggle/stepped/continuous, integer/float/named), handles forward/backward compatibility gracefully - including addition, deletion, or reordering of parameters). Yes, these things are hard and time consuming… which is why those of us who invested time in developing custom classes to do all of that are resistant to breaking changes forced on us without a similarly comprehensive solution.


#7

This bug primarily affected people who are rolling their own parameter classes. If you are not using the AudioProcessorParameter classes then overriding AudioProcessor::getParameterID (int index) to return anything other than String (index) would have caused problems.

I understand that you’re concerned about the latest parameter changes, but it’s far more compelling and productive to highlight specific problems rather than just broadcast unsubstantiated FUD. As I asked in the other thread: could you please provide some examples of things which are not possible using a subclass of AudioProcessorParameter?

If you wanted to preserve existing behaviour you could simply create a wrapper parameter class which just forwards calls to the corresponding methods of your AudioProcessor subclass:

class ParameterWrapper :   public AudioProcessorParameter
{
public:
    ParameterWrapper (YourAudioProcessor& yourProcessor, int audioParameterIndex)
        : processor (yourProcessor), parameterIndex (audioParameterIndex)
    {
    }

    float getValue() const override                 { return processor->getParameter (parameterIndex); }
    bool isBoolean() const override                 { return false; }
    String getCurrentValueAsText() const override   { return processor->getParameterText (parameterIndex); }
    ...

    YourAudioProcessor& processor;
    const int parameterIndex;
}

The only significant methods of AudioProcessorParameter that don’t have a direct mapping are

float getValueForText (const String&) const
String getText (float, int) const

but these are not particularly difficult to implement, and I suspect most people will already have the necessary code available to do this.


#8

I am confused… did these commits already happen? The one you linked to is the one that created the missing params in AU or am I misunderstanding something? I noticed I’m not seeing any parameters anymore in my AU version of one plugin that still uses legacy ids and then I went back to a stable version of Juce and now I don’t understand whether it’s save to go to the tip again.


#9

Sorry, I should have been clearer: the next few commits after e89c4d5d4e5e9beed4adf7c21fc25e6b4a70e72f (which were made a week or so ago) will fix the problem; the tip should be safe. Please let us know if you encounter any issues!


#10

This is exactly what I’m attempting to do in order to update our code for version 5.3, and as it happens I had already built a wrapper class previously. However, it inherits from AudioProcessorParameterWithID rather than AudioProcessorParameter. Since AudioProcessorParameter::processor is declared private, I can’t do the call-forwarding to my AudioProcessor that you describe. So, this leads me to my question: why is AudioProcessorParameter::processor private, and does it have to remain that way?


#11

If I remember correctly the private data member processor is now only used in a few places, all of which are commented to say that the method being called is deprecated and therefore it seems likely this member will disappear shortly anyway. Therefore you’re probably better off storing a reference to the AudioProcessor directly in your wrapper parameters.


#12

Oh. Heh. I guess that was just a little too simple for me to think of myself! Thanks for the help–much appreciated.