Optimization - create a "Equalizer band parameter group" factory method to generate and process an array of "Equalizer band parameter group"

[nathanjhood]

Chore

Describe the chore

I have successfully moved the Biquads DSP into one of my favourite C++ constructs - the auto-ranged 'for' loop - meaning that each audio block is now processed by a Biquad array, before the next block comes in.

This is a huge improvement over the previous method, where the same block was being passed from Biquad to Biquad per sample-tick, which was creating classic audio block artefacts (crackling/noise, focused in the high end of the audio spectrum).

Interesting side-note: in developing the above, JUCE's 'declare non-copyable with leak detector' macro really shone a light on the exact syntax to make sure we're not making any copies of the processing array by mistake... this macro might be very handy for a generic Var class that we might validate non-copying on the audio thread DSP as a unit test in the future? hmmm...

Now, I am keen to do the same array-iteration/range-based-for-loop thing for the parameters, which are now all tidily grouped together, given the current build's project version number for compatibility control (very nice feature!) and all sit under a global parameter group, which is also versioned nicely.

The tricky part is the construction and initialization of an unknown number of parameter groups, containing as-yet unspecified parameters. Consider the labels and unique ID's which all need to be known at construction/initialization, but accept string classes.

After giving this some preliminary consideration, I am thinking that the best thing to do might be:

• Define an object, which is a juce audio parameter group, containing one frequency float, one gain float, one resonance float, one type choice, and one bypass boolean
• Define a factory function for constructing one of the above object on-call
• When calling 'createParameterLayout', pass in the size_t of BiquadArray, which we define globally, and use it to construct an array (or container class of some sort) of size BiquadArray, with each element in the array being a call to the EQ band parameter group factory method
• If my logic is sound (aside from being abstract), then we should be able use this global size_t BiquadArray to drive the generation of two array types; one containing the DSP processors, and one containing a matching group of parameters.

One current benefit/issue (concurrently) in the current implementation is that the entire list of params is being updated on every new audio block... this means that parameters should be updatable simultaneously, perhaps over Midi or channel data of some sort (also think CLAP, VCV Rack, etc). This is also a drain, as the entire thing is written in a big ugly list, that all gets processed one at a time, per call to update().

It will be much better to use some sort of looping construct to processes each param group 'for each' DSP processor, but ideally, while maintaining this 'non-blocking' principle, where a user should in theory be able to update two un-related parameters from two different equalizer bands at the same time, and not have one block the other. If blocking between params and/or param groups cannot be avoided, then it must be managed with logic and reason (haw haw).

This doesn't really need to be a blazing fast CPU fest at all... but those repetitive lists of params and updates is quite an eyesore for a language as rich as C++, and most of the time, using these more modern constructs leads to much safer and more performant code (plus the things you learn in doing so).

Additional context

I created a size_t of 4 which is the number of Biquads to hold in the array. I feel that this var might actually sit nicely at a more global level, where it can be referenced as a single source of truth for how many "equalizer bands" our plugin should manage; both DSP array, and the number of parameter groups to be generated.

This size_t must be a compile-time constant, probably via a constexpr, since we are not intending to attempt dynamic creation and deletion of N number of bands at runtime. If we were ever to go down the route of only generating the visual elements for the active number of bands, then that should not be dynamically constucting and deconstructing any DSP classes, nor parameter groups. I advise creating groups in pow2 numbers, with an ideal limit being 16 bands (think 16 MIDI channels). The requirement in such a case would be to have 16 bands - and their corresponding parameters - constructed on plugin instantiation, and then the "show/hide" of inactive bands would only pertain to graphical rendering, such as in our Editor unit - which is currently not implemented at all.

[nathanjhood]

Going on a bit of a knowledge dive right now while trying to do this factory method (or a basic implementation of a factory method that works for our parameter needs). Tricky bit is about returning references and lifetime.

Static declaration on the return type means that the returned object will be alive for the duration of the lifetime of the program that runs it. I am unsure if this means our param group lives on after the plugin has died... it only takes one more static declaration to return an singleton instance instead!

Of course this is what standard template library gave us unique pointers for. But this is C++, sir... many aspects of the requirements, lifetime, and scope to consider and thoroughly test.

For example, then what? Do we construct a solid param group in-place within the function? Should we just pass in an enum to choose a set of strings to use for the labels? Or pass in fully-specified parameters to return a constructed group of? What about ownership, lifetime?

Nice getting into the thick stuff with C++ again, but I am slightly regretting prioritizing this slightly tough nut over some other outstanding tasks, which offer more reward for smaller effort!

Worse yet, it has me deeply questioning some of the current implementation's validity, meaning I am unsure that I am happy to fall back and leave this issue untouched.

At least with a parameter group, it should be pretty easy to validate any erroneous copies visually... but let's see what we get into here.

[nathanjhood]

This is proving a little tricky to retro-fit. Actually, I'd tried my hardest to avoid making this job be a case of retro-fitting, but after going at it, I can see that the concept itself of the task needs to be a bit more defined.

What is going to be array-ized, and what isn't? And how do we map between the two?

Non-array elements

All of the StoneyDSP::Biquads units - Processor, Wrapper, Editor, Parameters (GUI/apvts elements).
Oversampler
Dry/Wet mixer

Array elements

Biquads DSP
Biquads parameters (non-GUI side)
Potentially - apvts parameter groups?

Connection

The DSP array is already well-proven and deployed on main.

The GUI-side parameters are tricky because all though generic to some extent (at parameter group - level), every single GUI-side parameter also carries a whole load of unique labels in the form of juce::Strings (which can, at least, be extracted to UTF8-level char arrays). So, this necessitates something like a factory function and some kind of enum/lookup-table and hashing function, for constructing the unique labels which identify parameters across multiple axis: parameter name, DSP array index number, parameter units, parameter range... and so forth.

The other issue there is that we're already making unique pointers inline inside of the parameter group function. We're not really calling a class constructor and initializing it's members here on the GUI side.

All of this prevents us from being easily able to mimic the basic array design from the DSP side to the GUI side; since they both are being constructed and managed via different methodologies - as they correctly should, since they are not similar in functionality nor functional requirements.

Another strong issue is the usage of a wrapped process block as a templated class, which is then added as a member, twice, to the audio processor. Not actually an issue in itself, more rather that this implementation means I can't really just graft all of the GUI-side/apvts parameter creation onto the Wrapper class and do away with the Parameters unit entirely, since then we'd end up with two parameter layouts being rendered at once - one for the floats engine, one for the doubles engine. Obviously makes no sense at all, so I can't take the otherwise logical/sane approach of just making a single, whole Biquads instance - DSP and GUI/apvts parameters - and then placing that into an std::vector... because at some point lower down, we're constructing two of them - one floats, and one doubles engine.

So it turns out that having the Parameters unit was always a wise move, in the context of having the templated Wrapper class instantiated twice at Audio Processor level. The JUCE audio parameter classes on the wrapper side are safe to keep multiple copies of in this way, since they don't trigger the rendering of parameters on the GUI-side; that's what the Parameter unit's job is.

Whichever means the DSP and GUI sides each use to achieve their means, the real task in any case is figuring out how to safely map the initialization-construction of managed arrays/containers on the DSP side, from the parameters established on the GUI-side.

The CI/CD managed to build and run pluginval, which hit a SEGFAULT when attempting to open the editor while processing audio. This suggests a possible threading issue where I've accidentally bypassed the atomicity of the JUCE audio parameters, probably by some dodgy array/pointer mix up.

Rather than pull it all apart, build it and run it in the debugger 20 times, which will take 20 hours, I'll try another approach, starting with std::vector. It's really important to be sure to construct arrays/vectors of objects and not pointers, and also to have their lifetimes managed, and have easy access to the underlying class object's methods.

I am considering making another wrapper-type class, which is templated, and will simply hold a StoneyDSP::Audio::Biquads as a data member along with a set of parameter pointers for one Biquads instance, and will provide some sort of means of accessing the process() method, the setFrequency/Resonance/Gain() etc parameters underneath, and some of the casting from the parameter type to the templated data type.

This could then be held in a managed array/container, of size however-many-biquads-we-want.

We just then need to figure out the best way to get the correct set of GUI-side parameters (via the parameter groups, hopefully) to map to the correct index in the biquads array. Ideally, this should probably include the full initializer list, so that we can still call jassert(someParamPtr != nullptr) at construction time for debugging purposes, as standard.

That may mean we need to provide a constructor which accepts a full set of JUCE parameter types... or, even better, accepts a JUCE parameter group, which must itself contain the full set of parameters for a biquads instance.

Actually that last line was a slight epiphany. Maybe I'll try that next.

For the moment, I also want to try some of the CMake presets out, so I'll make another branch to shift some of the current wip onto (because I made some cool CMake presets on it), and keep this task afloat separately.

[nathanjhood]

So, the thinking outlined above is: instead of constructing multiple arrays/containers and attempting to keep them all in sync at compile-time, construction-init, deletion, and during runtime... let's try to make one object, containing a full and working Biquads instance (inc. parameter pointers and the means to update them).

Once proven, this object can then be placed first into a naive array, and then upgraded to either an std::vector, std::array, or std::unique_ptr.

This helpful SO post is worth considering:

Initial size

• vector and unique_ptr<T[]> allow the size to be specified at run-time
• array only allows the size to be specified at compile time

Resizing

• array and unique_ptr<T[]> do not allow resizing
• vector does

Storage

• vector and unique_ptr<T[]> store the data outside the object (typically on the heap)
• array stores the data directly in the object

Copying

• array and vector allow copying
• unique_ptr<T[]> does not allow copying

Swap/move

• vector and unique_ptr<T[]> have O(1) time swap and move operations
• array has O(n) time swap and move operations, where n is the number of elements in the array

Pointer/reference/iterator invalidation

• array ensures pointers, references and iterators will never be invalidated while the object is live, even on swap()
• unique_ptr<T[]> has no iterators; pointers and references are only invalidated by swap() while the object is live. (After swapping, pointers point into to the array that you swapped with, so they're still "valid" in that sense.)
• vector may invalidate pointers, references and iterators on any reallocation (and provides some guarantees that reallocation can only happen on certain operations).

Compatibility with concepts and algorithms

• array and vector are both Containers
• unique_ptr<T[]> is not a Container

Our requirements

• We don't intend to allow any dynamic allocation at all
• No copy/move semantics should be permitted
• We should know our exact requirements at compile-time
• We should never invalidate the pointer while the object is live

Do perhaps we should consider taking a look at std::array. It doesn't seem as though we're explicitly forbidden from any particular means above, since they do all provide methods we need and we can simply manage the avoidance of doing anything we' wouldn't want to do, such as copy/move/swap or invalidating pointers while live.

It certainly is better having the awareness of these options and their individual pro's and con's.