[dsp module] coding style discussion

General JUCE discussion
1

I am a deep user of JUCE, but when I use the dsp module, I feel it is not very comfortable to use. One of the most important reasons is that the code style of the dsp module is too different from other JUCE modules. Let’s use the new “Panner” class as an example. The Panner::process method (and all other similar methods of the dsp classes) uses a ProcessContext template type as a parameter. However, using the template type will bring many negative effects. One of them is that because the actual type of the parameter is unknown, the doxygen documentation will unable to locate any useful information of the type:
image
As you can see, this type is grayed out and is not clickable or navigable. However, it is clear that this type is not arbitrary, it needs to have certain conditions (such as having certain methods or properties). To make matters worse, not only does doxygen fail to provide me with this information, nor is it provided in the comments. In other modules of JUCE, first of all, I did not see a method that uses a template as a parameter (I also don’t like this approach. All such practices can theoretically be achieved through abstract classes + pure virtual functions + inheritance. This kind of approach is better for ease of use and readability), and even if there are, the comments will specify in detail what conditions are required for the type.

This is not the whole problem. When neither the type itself nor the comments provide me with information on how to use it, I think the only solution is probably to read the implementation of this method, until I see such an implementation. . .

    template <typename ProcessContext>
    void process (const ProcessContext& context) noexcept
    {
        const auto& inputBlock = context.getInputBlock();
        auto& outputBlock      = context.getOutputBlock();

        const auto numInputChannels  = inputBlock.getNumChannels();
        const auto numOutputChannels = outputBlock.getNumChannels();
        const auto numSamples        = outputBlock.getNumSamples();

        jassert (inputBlock.getNumSamples() == numSamples);
        ignoreUnused (numSamples);

        if (numOutputChannels != 2 || numInputChannels == 0 || numInputChannels > 2)
            return;

        if (numInputChannels == 2)
        {
            outputBlock.copyFrom (inputBlock);
        }
        else
        {
            outputBlock.getSingleChannelBlock (0).copyFrom (inputBlock);
            outputBlock.getSingleChannelBlock (1).copyFrom (inputBlock);
        }

        if (context.isBypassed)
            return;

        outputBlock.getSingleChannelBlock (0).multiplyBy (leftVolume);
        outputBlock.getSingleChannelBlock (1).multiplyBy (rightVolume);
    }

The combination of template type and “auto” type automatic derivation makes this code completely unreadable. All positioning, navigation, auto-complete functions in the IDE are all unavailable here. I think this is very unfriendly to users.

I have a few suggestions that you should seriously consider:

  1. Change the template parameters to use “abstract class + pure virtual function + inheritance” to achieve.
  2. Make the comments of the dsp module more delicate. There are too many rough comments in this module.
  3. If the type is not too long, use less “auto” in the implementation to enhance readability.

Of course, I personally hope that all three of my suggestions will be accepted by you. But I know you may have some other considerations. So this post is also a discussion post. Friends who disagree with my suggestions are welcome to raise some objections.

3 Likes
2

While from a functional point of view this is correct, from a runtime performance point of view this can make a huge difference. If ProcessContext would be an abstract base class to ProcessContextReplacing and ProcessContextNonReplacing the compiler would probably compile a version of that function that takes that abstract type and has to perform a virtual function lookup at runtime each time a member function on context is called. On the other hand if ProcessContext is a templated type, the compiler will generate a version of that function especially for the type of context passed in from the function template. If the context passed in does not inherit a base class like the JUCE contexts do, it has no vtable as far as I know, so there is no need for a virtual function lookup at runtime and the compiler can perform some heavy optimizations.

And by the templated nature of the whole dsp classes this can lead to a significant performance boost in the end. I really appreciate the clever usage of such techniques in the dsp module.

I think there is a tradeoff between easy usage and maximum performance here. For most of the other JUCE classes the performance impact of such implementation strategies would be negligible and therefore there is no real need for such heavy templated interfaces (which makes the classes handy to use). For DSP however, performance can matter a lot and I think this is a good reason to chose an implementation style that supports maximum compiler optimization.

6 Likes
3

But at least I think the comment should be improved. I can’t see this comment now that it has anything to do with ProcessContextReplacing and ProcessContextNonReplacing, unless I go to those two classes and read their comments, or read the sample code, or directly read the implementation of the module. In short, from the perspective of document friendliness, the dsp module is not as good as other modules, and needs to be improved.

4

I think the documentation could be improved with better examples. It does seem like a beast of a namespace that does try to be overly clever for the sake of performance boosts, but that’s just me…

3 Likes
5

Shouldn’t the DemoRunner’s DSP catagory add demos of those newly added classes in 6.0?
DemoRunner is a good way of demonstrating the use of classes, because it can compare functions and code in real time. The way of plug-in example + AudioPluginHost is relatively less friendly. And there are already so many demos of DSP functions in the existing DemoRunner. It is a pity that these newly added classes are missing.

So can we also add the demos of these newly added DSP classes in 6.0 to DemoRunner?
@t0m @ed95 @reuk

1 Like
6

For 6, we thought it would be most useful to show the DSP functions in their most likely usage scenario, i.e. in a plugin. The DSPModulePluginDemo example has been fully rewritten to demonstrate the new functionality of the DSP module.

We seriously considered adding the plugins in examples/Plugins to the DemoRunner, but soon realised that for the plugins to be usable, we’d need some kind of mini hosting environment inside the DemoRunner. We would have ended up duplicating most of the functionality of the existing AudioPluginHost app. Instead, we decided to build the demo plugins directly into the AudioPluginHost. This means that in JUCE 6, the AudioPluginHost includes the JUCE example plugins as built-in ‘internal’ processors, out-of-the-box, which should make the plugin examples much more accessible.

2 Likes