FabFilter VST3 Bugs

Hi

Not sure if issues related to the vst3 wrapper goes here or in the "Plugins" section?

Anyway

I have discoered an issue that can be recreated in the Demo Host.

The plugin FabFilter Pro q 2 (Demo available here: http://www.fabfilter.com/products/pro-q-2-equalizer-plug-in)

If you send midi to this plugin, it creates an access violation.

Is it the plugin or the host?

Any thoughts?

The Plugin Host does crash at "processor->process (data);", but nothing's jumping out in the JUCE code for me right now. Seems like you would need to get in touch with FabFilter and see if they could debug; maybe they can offer some clues.

From the fabfilter guys:
When Pro-Q 2 receives a MIDI message in the processor, it notifies the controller by sending a parameter change via the host.
It does so by calling data.outputParameterChanges -> addParameterData() to create a new parameter queue if needed, and then it calls addPoint() on the queue.

The problem is that addParameterData() returns NULL in the test host, so the call to addPoint() crashes.
Pro-Q 2 doesn’t test the pointer before dereferencing it, but reading the VST3 docs I get the impression that NULL is not a valid return value for addParameterData().

So it seems to be å bug in the Juce vst3 wrapper, combined with some optimistic coding from fab filter?

Gahhhh! The VST3 docs aren't very clear about anything; they're some of the worst I've ever seen for this reason... And not checking for nullptrs?! No wonder I'm losing my hair...
 

Potential fix, tested with success here on my end:

    //==============================================================================
    class ParameterChangeList  : public Vst::IParameterChanges
    {
    public:
        ParameterChangeList() {}
        virtual ~ParameterChangeList() {}

        JUCE_DECLARE_VST3_COM_REF_METHODS
        JUCE_DECLARE_VST3_COM_QUERY_METHODS

        Steinberg::int32 PLUGIN_API getParameterCount() override                                { return (Steinberg::int32) queues.size(); }
        Vst::IParamValueQueue* PLUGIN_API getParameterData (Steinberg::int32 index) override    { return queues[(int) index]; }

        Vst::IParamValueQueue* PLUGIN_API addParameterData (const Vst::ParamID& id, Steinberg::int32& index) override
        {
            for (int i = queues.size(); --i >= 0;)
            {
                if (queues.getUnchecked (i)->getParameterId() == id)
                {
                    index = (Steinberg::int32) i;
                    return queues.getUnchecked (i);
                }
            }
            ParamValueQueue* q = queues.add (new ParamValueQueue (id));
            index = getParameterCount() - 1;
            return q;
        }

    private:
        class ParamValueQueue  : public Vst::IParamValueQueue
        {
        public:
            ParamValueQueue (const Vst::ParamID parameterId) : id (parameterId) {}
            virtual ~ParamValueQueue() {}

            JUCE_DECLARE_VST3_COM_REF_METHODS
            JUCE_DECLARE_VST3_COM_QUERY_METHODS

            Steinberg::Vst::ParamID PLUGIN_API getParameterId() override    { return id; }
            Steinberg::int32 PLUGIN_API getPointCount() override            { return (Steinberg::int32) changePoints.size(); }

            Steinberg::tresult PLUGIN_API getPoint (Steinberg::int32 index,
                                                    Steinberg::int32& sampleOffset,
                                                    Steinberg::Vst::ParamValue& value) override
            {
                if (isPositiveAndBelow (index, getPointCount()))
                {
                    ChangePoint& cp = changePoints.getReference ((int) index);
                    sampleOffset = cp.sampleOffset;
                    value = cp.value;
                    return kResultTrue;
                }

                sampleOffset = -1;
                value = 0.0;
                return kResultFalse;
            }

            Steinberg::tresult PLUGIN_API addPoint (Steinberg::int32 sampleOffset,
                                                    Steinberg::Vst::ParamValue value,
                                                    Steinberg::int32& index) override
            {
                if (isPositiveAndBelow (index, getPointCount()))
                {
                    changePoints.add (ChangePoint (sampleOffset, value));
                    index = getPointCount();
                    return kResultTrue;
                }

                return kResultFalse;
            }

        private:
            struct ChangePoint
            {
                ChangePoint (Steinberg::int32 o, Steinberg::Vst::ParamValue v) noexcept
                    : sampleOffset (o), value (v) {}

                Steinberg::int32 sampleOffset;
                Steinberg::Vst::ParamValue value;
            };

            Atomic<int> refCount;
            const Vst::ParamID id;
            Array<ChangePoint> changePoints;

            JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ParamValueQueue)
        };

        Atomic<int> refCount;
        OwnedArray<ParamValueQueue> queues;

        JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ParameterChangeList)
    };

 

Great jrlanglois!

Maybe Jules will add this in?

Yes I can (alomst) understand forgetting to check for nullptr, but not wanting to check for nullptr is just asking for trouble.

Especially if the source of the pointer comes from a third party....

But for every fix like this the Juce framework becomes better.. so

Did you happen to test it? I didnt go much futher after testing it with Pro Q2...

This is great - thanks Joel! If anyone could sanity-check that it works ok, that feedback would be appreciated!

I'm on call right now, so have a bit of time on my hands while waiting. I managed to test a slew of other plugins against the new code, and haven't found any issues. But, of the many plugins I have installed, very few make use of the ParameterChangeList in the JUCE Plugin Host.

Jules or Dave; I think the only thing left to do to sanity-check this code is to load up a VST3 in Tracktion, and get some automation going with it.

Yes, seems to work fine.

Although FabFilter is the only one that I get this issue with.

Thanks again jrlanglois

Any news, Jules? Everything seems fine on our end.

Dave pointed out that the implementation you posted isn't thread-safe, and these change events will be coming both from the audio thread and the GUI thread. So once I get some time to add some kind of locking, I'll post the results..

Not sure if this is a Juce thing (Thinking it might not be)

But FabFilter plugs, running as VST3 does not repond to setStateInformation()

Can anyone confirm this ?

If this is a clear fabfilter issue I will contact them again. All other VST3s in my host seem to recall fine.

It's impossible for us to know what FabFilter are doing internally, so only they can answer the question of whether this is something we'd need to fix.