nathanjhood / Biquads

Simple two-pole equalizer with variable oversampling.
https://www.stoneydsp.com/projects/biquads
GNU General Public License v3.0
13 stars 1 forks source link

Refactor - StoneyDSP Library #69

Open nathanjhood opened 6 months ago

nathanjhood commented 6 months ago

The existing implementation is still a little messy; previous attempts at improving things within the codebase have lead to some complexity that is probably not necessary, given that the project is mostly just academic in nature.

Moreover, I've gotten to grips with making API's and modules with C++ and CMake in graceful ways that should now allow me to build my own library/extension for JUCE.

My intention therefore is to begin parting out the relevant source code for modularisation. This is already somewhat considered in some of the design aspects of the C++ used code, but there is currently no larger library or framework supplying these files to (potential) consumers.

The C++ module code shall be supported by a possibly optional CMake API, designed to provide consumers with easy CMake targets to link with in their projects, allowing them to easily pull in the C++ modules from various sources such as vcpkg, or even have system installations of the library, with which to build.

Furthermore, the proposed work would also allow some scope for the creation, development, and testing of a custom vcpkg registry from which the modules could be supplied.

Ideally, this external project should be able to do a CMake find_package(StoneyDSP 1.0.0 CONFIG REQUIRED) and then a target_link_libraries(Biquads PRIVATE stoneydsp::stoneydsp_audio stoneydsp::stoneydsp_graphics), and then only require the standard ProcessBlock() and MainComponent from the JUCE audio plugin template.

I have begun work on the library itself, and will be using this project as a test/example of the library.

nathanjhood commented 5 months ago

On reflection (been a while), I know that the existing OS implementation is quite hacky; It is not a requirement for the base DSP to work, but rather an extension that further demonstrates some points made in the analysis/write up.

Since both open issues are related to the OS implementation, and it is this implementation which is holding back progress somewhat, I've moved that implementation over to a backup branch for visitors to continue referencing, and have updated the main branch to reflect the other latest changes - because I wanted to get multi-platform CI/CD off the ground, above and beyond just oversampling on a Windows-only plugin.

I will "circle back" around to the oversampling at a slightly later date - readers, if curious about this, please check the backup branch for the meantime.

nathanjhood commented 5 months ago

I have merged a v1.2.0 which rolls back the GUI and the oversampling, in favour of multi-platform builds validated with Tracktion Pluginval via CTest, over GitHub Actions.

Now, this project can be deployed for all platorms.

However, in the context of this ticket, I consider only half of the work to be complete: I moved some of the codebase into the StoneyDSP library modules, which are currently still local to the project - these should obviously be centralized, perhaps delivered over a CDN or package manager, sourced from the StoneyDSP library repo - and much of the implementation is still sitting in the Biquads unit files.

Ideally, those unit files (i.ie., everything under include and src), will end up being very very generic across all of my plugins. I forsee the possibility that only the encapsulating namespace, and root unit files (Biquads.hpp/cpp) need change from project to project.

It may be possible to do some kind of aliasing, like using or some sort of typedef or preprocessor macro, to override what AudioPluginAudioProcessorEditor evaluates to. That might allow the root unit files to contain all (or most) of the unique information, while the Processor, Wrapper, Parameters, and Editor be largely generic from project to project.

In regards to this Biquads implementation, I have moved the atomic coefficient object and a general Biquads class (not used - just for reference for the moment) into the stoneydsp_audio module, and called on a few constants and other helpers from stoneydsp_core.

In continuation, I would ideally like to have the transforms be placed into the stoneydsp_audio module also, and the coefficient calculations either go along with it, or go under stoneydsp_core/maths.

The various distinct pieces would be filed under stoneydsp_audio/filter/2_pole in individual files and class declarations, and then called and used in stoneydsp_2PoleFilterBLT, or something like that.

The ProjectInformation namespace/object at the root - which is of course different from project to project - can sit next to our sneaky typedef/macro/using/alias, which would do something like:

namespace StoneyDSP {
namespace Biquads {

using AudioPluginAudioProcessor = Biquads;

}
}

...before it includes the other unit files.

Then, the Wrapper object's member - the DSP to be used - wouldn't need to be changed from project to project. Instead, we just point our alias at some other pre-constructed class from the library.

I am not sure how well this might work, but I will investigate it while continuing with this task.

Still to be done, then:

With the above complete, and the "genericizing" of the project files a secondary goal, I will re-evaluate the status of this ticket.

nathanjhood commented 5 months ago

EDIT: I guess I'm sort-of thinking about trying a PIMPL-like way of connecting a predefined set of interfaces using a compile-time variable...

nathanjhood commented 5 months ago

StoneyDSP::Audio::Biquads has been successfully extrapolated into a class, defined within the stoneydsp_audio module.

This concludes on half of the refactoring task at hand - the moving of our class from outside of the project files, and into an external library/module.

The other half which still persists, is to now move stoneydsp_audio - and stoneydsp_core - back over into StoneyDSP, and have this Biquads project fetch our library from something like vcpkg...

stoneydsp_audio as it currently stands is actually a whole unique set of files in the Biquads project root folder under modules. This is not how we want it; as I make further improvements to the DSP implementation, I'd like those changes/improvements to populate downstream to all consumers of my Biquads class. Following the single source of thruth principle, this leads to some more conclusions on how the DSP library shall be managed, maintained, versioned, and distributed - which perfectly demonstrates why I picked a refactor of this project as a means of beginning the DSP library:

I've been making improvements to the audio processor structure while de-coupling the DSP from it. I have realized some redundant areas in the previous design and removed/refactored quite a lot of that away already, but I'm not totally happy with a few of the design decisions with regards to ownership, and initialization.

Which class owns which, and then which class initializes which? I've de-tangled several looping idioms in the constructors that started to look very dodgy when looking at them a bit closer. However, I've done only about half of the work involved in what I've discovered along those lines. (currently I'm quite unhappy about all the un-managed pointers to the main Audio Processor class being passed in to initialize-construct it's own members... in some cases, quite redundandtly...)

I am thinking that the APVTS and the entire parameter layout should all be defined and looked after in our Parameters unit, which may pass a couple of methods to the Audio Processor which instantiates it, allowing the Audio Processor to access the APVTS (and vice versa), while providing a level of indirection between our parameter pointers and the audio processor class.

The parameters class/object can then also be made accessible to the audio processor wrapper, where the parameter values can be exchanged atomically with the audio-thread class instances. It will be necessary to consider carefully how this is done, as we can potentially reduce complexity quite a bit by simply considering the chain of dependencies between each of these class objects, and which shall need to be defined first.

It should not be at all necessary to pass multiple un-managed *this calls from the audio processor to multiple underlying references of itself - and via a forward declaration, even!

Regarding the Biquads Audio Processor:

There are sketchy aspects of this design which I think are valid concerns, not least because this was my first ever serious coding project and I'd held on to lots of things I'd learned at the time, but probably misunderstood, or over-used, or can now think of solutions which are more aligned to the way I have done things elsewhere within the codebase and/or want to pursue as I move forward.

Regarding the Biquads DSP:

It is very hard for me to validate the Biquads class as a whole, from top to bottom, even using unit tests and a debugger. The data structure is much too large and it's members too inter-related. I aim to continue refactoring the DSP such that the coefficient calculations will become a set of namespaced math functions, templated, and that the blinear transformations will be something like a DSP class, defined seperately. Ideally, both shall hold and manage array-based containers of coefficients (instead of unique declarations per coefficient), and the Biquads DSP class will define a method which passes the calculated coefficient array to the bilinear transform array, possibly atomically.

Looking at both concerns above, what actually happens is, the Biquads Audio Processor is defined here in the Biquads project, and so those concerns will remain associated with this project; if I am not finished with that part of the work by the time I have vcpkg (or something else) serving and fetching our modules from the library, then I will make future epics here in this repo for fulfilling those tasks.

The concerns relating to the DSP library shall not really be applicable to the management this repository - even if this is where we discover them, and figure out how to fix them. Instead, since the DSP library shall be fetched remotely from the StoneyDSP project, those concerns shall become epics and issues over on the StoneyDSP repository, instead.

I am also looking at the potential idea of having each StoneyDSP module exist as a submodule, such that the StoneyDSP org holds the StoneyDSP repo, plus a repo each for stoneydsp_core and stoneydsp_audio, making it slightly easier to version-control each one, to manage these epics and issues in a more specific fashion, and also perhaps make it slightly easier for non-vcpkg acquisitors to pick and choose which modules they want from the library, without being lumped with the whole thing.

In either case, I find the most interesting element in the work to be the nature of the provider/consumer relationship in this coding project. As I de-couple the DSP from the plugin files, I will need to be mindful not to break the interface that I have defined for the Biquads class, including as I dive deeper into de-coupling parts of the DSP from itself internally. This Biquads project, being the consumer, must not be broken by upstream changes in the StoneyDSP library.