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.
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
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.
bump
bump!
Please fix this, many threads about it:
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
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.
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.
Alright, I’m convinced. I’ll take a look at providing some non-allocating alternatives.
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.
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
so the modulation should be handles with care anyways, even with allocating coefficients
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:
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)
more thoughts:
-
It would be great to have something like Coefficients::applyLowPass(…) which can be directly applied to existing coefficients, without any allocation involved.
-
Now we have ScopedNoDenormals - I guess JUCE_SNAP_TO_ZERO might be outdated ?
-
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.
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)
Yes, the ArrayCoefficients functions are designed so that they do not need to allocate.