Non-allocating alternatives for the IIR::Coefficients::make... functions

First of all: I really like the dsp classes :slight_smile:

However, when it comes to the IIR::Coefficient class, I feel like I’m missing something obvious. To calculate filter coefficients, there are a lot of nice make... functions. After having calculated the coefficients, these functions internally allocate a new coefficient struct to return and populate it with the calculated coefficients. While I like the ref-counted approach of this, it makes these functions unsafe for all situations, where the calculation would happen on the audio thread. This is especially the case, if coefficient recalculation happen on parameter changes, which are invoked from the audio thread in case of VST3 plugins.

As this seems to be such a normal use-case and the solution to this seems so easy to implement, I wonder if I’m really missing something obvious here? :sweat_smile: Is there any functionality I didn’t find to recalculate the coefficients and place them into an already existing, pre-alloated IIR::Coefficient struct, besides of implementing my own free function with basically a copy of the coefficient calculation code found inside the IIR::Coefficient class? Maybe @IvanC as the designer of the class could share some insight?

Bump

if only juce classes were allocator aware, or at least supporting c++17 pmr memory resources, this would be trivial to tweak by using a bump allocator with some preallocated block.

2 Likes

Mhm, while these are interesting options for containers like AudioBuffer, Strings or all the Array classes, I’m not sure if this would be such a trivial solution to this problem, at least not from the viewpoint of every JUCE user. Not all of them are pro level C++ programers, if you come from a more DSP background reading something like std::pmr::polymorphic_allocator could maybe not seem like a trivial soultion to your problem :smiley:

What I think of would be something like adding a constructor like

IIR::Coefficients<NumericType>::Coefficients (int numCoefficient)
{
    coefficients.ensureStorageAllocated (numCoefficients);
    for (int i = 0; i < numCoefficients; ++i)
        coefficients.add (NumericType());
}

To create a pre-alloated coefficient array.

Then add a member functions like

void IIR::Coefficients<NumericType>::setCoefficient NumericType b0, NumericType b1,
                                                    NumericType a0, NumericType a1)
{
    // like the corresponding constructor
}

void IIR::Coefficients<NumericType>::setCoefficient NumericType b0, NumericType b1, NumericType b2,
                                                    NumericType a0, NumericType a1, NumericType a2)
{
    // like the corresponding constructor
}

Add non-static member function variantions of the make functions like

void IIR::Coefficients<NumericType>::computeFirstOrderLowPass (double sampleRate, NumericType frequency)

which contain the code of the current make ones but call setCoefficients where the current ones call new. The original make functions could remain and use the new constructor internally to first pre-allocate a new Coefficient struct, then call compute on it and return the newly allocated coefficients.

If I find some time next week, I could create a Pull Request with these changes, but some feedback from the JUCE team on what they think about all this would be great.

1 Like

bump

bump!

1 Like

Please fix this, many threads about it:

3 Likes

I’d be really interested to hear what @IvanC has to say, but my impression is that neither of JUCE’s IIR filters are particularly suited to modification during playback.

If you create one set of coefficients with the make functions, do some processing, and then apply a new set of coefficients, there’s no guarantee that the filter’s internal state will make sense for the new coefficients and the filter may become unstable. If the new coefficients are close enough to the old coefficients the error may be acceptable, but I don’t think stability can be guaranteed in general.

In addition, the dsp::IIR make functions generate coefficients for filters of different orders, but the docs specify that reset must be called whenever the order changes. This means that when switching between certain filter kinds, some kind of click or discontinuity is inevitable.

For these reasons, I don’t think that rewriting these functions to avoid allocations is a good idea. In the best case, users will be able to avoid allocating during prepareToPlay, which isn’t really a problem in the first place. In the worst case, users will see the new-and-improved allocation-free functions, assume that it’s completely safe to apply new coefficients during playback, and create unstable plugins that might damage hardware and eardrums.

If it turns out that I’ve got this all wrong and it’s actually completely safe to apply arbitrary coefficient changes during playback, then I’ll take a look at making these functions allocation-free.

Regarding the SpinLock in the older IIRFilter implementation, I don’t think this is of any concern in the normal case of accesses which are strictly externally synchronised. In such cases, there is guaranteed to be no contention on the lock, and the lock’s overhead should be negligible (one additional compareAndSet of an atomic bool for each call to the filter). That said, the lock would be redundant in such a situation, so I’ll have a think about some method for optionally disabling it.

Code that cannot guarantee external synchronisation on the IIRFilter will need the locks to remain. Removing the locks entirely could introduce data races in user code, so any removal of the locks must be opt-in.

Please correct me if I’m totally wrong, but I’m pretty sure that an IIR filter can only become unstable by choosing an unstable set of coefficients, but never by non-matching state values in the filter. It might of course be possible to introduce discontinuities in the signal with drastic parameter changes, but there are a lot of use-cases where the logic around the filter will limit this to a safe area or ensure proper fading and I know of products where this done, without any problems.

There are a lot of possibilities to create terrible noise when doing dsp wrong, so I’d vote for passing that risk to the user instead of limiting the API

2 Likes

I can confirm that I have an edited version of the JUCE IIRs with the new calls removed which all support super fast modulation and sound phenomenal – they’re my favorite filters I’ve ever used.

1 Like

I also had to do it manually, essentially refactored the class to use a plain std::vector for the coefs, and each makeLowShelf(), etc will just modify the vector in place instead of allocating a new one, which means you can also call it per sample if you want to.

It only took a few hours to do, and it sounds great.

4 Likes

Alright, I’m convinced. I’ll take a look at providing some non-allocating alternatives.

9 Likes

Hello everybody!

I confirm there is nothing wrong with modulating the IIR class or IIRFilter class coefficients in a way or another. But the modulation signal needs to be as smooth as possible otherwise you’ll get nasty discontinuity heavy artefacts.

That is the reason why there are the StateVariableFilter classes providing by design a filter class with integrated handling of fast cutoff frequency modulation.

Not sure that some unstability can happen because of modulation though, if the new coefficients are supposed to be the ones of a stable filter. Quite sure I read some articles about that but I don’t remember the answer.

1 Like

Modulating the cut off frequency of a highpass Filter, with a sine wave as input can increase the output level by 10-20dB. At least that was the result of my quick test :slight_smile:

so the modulation should be handles with care anyways, even with allocating coefficients :slight_smile:

I’ve added some non-allocating coefficient-creation functions here:

Please try out this change and let me know if you run into any problems.

I’ve also added a new SingleThreadedIIRFilter with no locking here:

5 Likes

Thank you, the std::array based solution looks extremely promising. Unfortunately I‘m quite busy at the moment, but I‘ll try to give it a try in one of my projects next week!

IMHO it should not the responsibility of the filter to care about any allocation or thread-safety issues.
Just one coefficient class (which can change its own coefficients, without allocation), one filter class (with its internal states) which takes a pointer to the coefficients (so they can easily shared along different channels)

1 Like

more thoughts:

  1. It would be great to have something like Coefficients::applyLowPass(…) which can be directly applied to existing coefficients, without any allocation involved.

  2. Now we have ScopedNoDenormals - I guess JUCE_SNAP_TO_ZERO might be outdated ?

  3. Having two two different implementations of this kind of IIR-Filter which share the same filename “juce_IIRFilter.h” in different locations is confusing, maybe the older should be deprecated.

1 Like

IIR::Coefficients has an operator= accepting a std::array of coefficients. As long as the array of coefficients is smaller than around 8 items, this assignment will not allocate.

Thanks! Do you if its guaranteed that the ArrayCoefficients::makeHighShelf also will also not allocate?

Im using this, in my processCallback , per Sample.

*iirFilterCoefficents = dsp::IIR::ArrayCoefficients<float>::makeHighShelf (...)

(btw can’t use svf-filter because of specific reasons)