#1 most common programming mistake that we see on the forum

This post is here so that we have something we can link to when this same issue comes up, because the JUCE team is getting very bored of spending time replying again and again to the same damned problem!

This isn’t a JUCE bug - there’s nothing we can do in our codebase that would stop people making it, which makes it painful for us to watch beginner after beginner come along and fall headfirst into the same trap!

It’s also understandable that people will post a new topic when they hit it, because it’s not the kind of problem you can find an answer to with a search. The forum probably contains hundreds of posts about the same thing, but the code is always different, and there’s no keyword you could look for to find them.

But there’s always a process method, and it always contains a loop that looks something like this:

for (int channel = 0; channel < 2; ++channel)
{
    float* thisChannel = data[channel];

    for (int sample = 0; sample < numSamples; ++sample)
    {
        thisChannel[sample] = someFunctionOf (lastSample); // where lastSample is a member variable.
        lastSample = thisChannel[sample];
    }
}

…and the poster will complain of “noise” or “artefacts” or “clicking”.

Usually there’s a variable holding a previous sample value. Or there’s a single IIR or FIR that they’re applying to all channels, but it’s basically the same mistake.

To anyone who can’t see the bug, it’s because you need to keep separate state for each channel. Otherwise, that last sample value from the end of channel 0 will be used at the the start of channel 1 when you go round the outer loop.

If anyone has any ideas about how we could somehow pre-warn newbies and stop them doing this, please let us know, as it’d save us a lot of time explaining the same thing over and over again!

9 Likes

easy: make processBlock const! :nerd_face: :rofl:

more serious: adjust the default AudioProcessor the Projucer creates when creating a new Audio Plug-In project:

  1. define a struct named ChannelState with some example variable
  2. create an array of ChannelStates as a member of AudioProcessor
  3. adjust the example code in the template to use the variable of the correct ChannelState object
  4. add comments.
  5. bonus points: resize the array to the correct number of channels.
1 Like

Well, I vote against that… I don’t want the boilerplate convoluted with stuff, that is towards a specific purpose. I would have to delete it in 99% of the cases.
But I like your idea to give more examples, maybe extend the tutorial plugin with a lowpass and a highpass filter…
Useful and educational at once…

1 Like

Definitely more online tutorials…and DEFINITELY one with IIR filters on multiple channels because that’s the #1 mistake I read on the forum.

Honestly, more tutorials in general would be helpful. ValueTrees come immediately to mind.

2 Likes

@jules I’d love to do a tutorial on this. Maybe we can convene and discuss the key points that need to be covered?

There’s the beginnings of an example here:

Rail

@Rail_Jon_Rogut nice thanks for this. I’ll have a look at this in the morning and start shaping things up

My assumption is that if we used a nested for loop as in Jules example above, we would need an if statement for each channel, is that right? Something like this (if we had 2 outputs)…

for (int channel = 0; channel < buffer.getNumChannels(); ++channel)
{
    //separate write pointers for left and right
    const float* dataLeft = buffer.getWritePointer(0);
    const float* dataRight = buffer.getWritePointer(1);
    
    for (int sample = 0; sample < buffer.getNumSamples(); ++sample)
    {
        if (channel == 0)
        {
            //output to left speaker
            dataLeft[sample] = doSomeProcessing (lastSampleL);
            lastSampleL = dataLeft[sample]; //current sample becomes last sample
        }
        else if (channel == 1)
        {
            //output to right speaker
            dataRight[sample] = doSomeProcessing (lastSampleR);
            lastSampleR = dataRight[sample]; //current sample becomes last sample
        }
    }

That would be a pretty bad way of doing it. If the plugin ever needed to support more channels, more if statements would need to be added and the chance of mistakes in variable names would increase.

Something like this would be better :

 for (int channel = 0; channel < buffer.getNumChannels(); ++channel)
 {
    float* data = buffer.getWritePointer(channel);
    for (int sample = 0; sample < buffer.getNumSamples(); ++sample)
    {
        data[sample] = doSomeProcessing (lastSamples[channel]);
        lastSamples[channel] = data[sample]; //current sample becomes last sample
	}
 }
2 Likes

@Xenakios That’s great thanks for this. This brings me back to what we discussed the other day. How would we handle this situation without branching? I know the difference equation is better handled in a class. This seems to work fine for me by the way:

for (int channel = 0; channel < buffer.getNumChannels(); ++channel)
{
    //input data reads from buffer, then we process data and put back out
    const float* inputData = buffer.getReadPointer(channel, 0);
    float* outputData = buffer.getWritePointer (channel, 0);
    
    //place audio samples into buffer
    for (int sample = 0; sample < buffer.getNumSamples(); ++sample)
    {
        //get the values here from the slider
        a1 = *tree.getRawParameterValue("a1Control");
    
        //design equation a0 = a1 - 1.0
        a0 = a1 - 1.0;
    
        //get current value from read pointer
        float xn = inputData[sample];
        
        //delay by one sample---previous value from output is now the delayed value
        float xn1 = z1;
        
        //use difference equation y(n) = a0x(n) + a1x(n-1)
        float yn = a0 * xn + a1 * xn1;
    
        //current output is stored to become previous output in next loop
        z1 = xn;
    
        //output to speakers
        outputData[sample] = yn;
    }
}

You should just put the DSP code into a class so that you really can have different states for each channel by having multiple instances of the DSP. With that code above you may have just got lucky that it doesn’t have problems like clicks in the audio.

@Xenakios Thanks for your help. I think this touches on why this is an issue a lot of people run into.

The problem as I understand it is clear in that the values for the two channels need to be kept separate, but the answer to how that’s achieved in most general cases without branching doesn’t seem quite clear to me.

I wonder if you have some misconception about how the processBlock method gets called…?

The host doesn’t call it just once. The method gets repeatedly called over and over again. It should be quite obvious why the DSP code for each audio channel should have completely independent variables that keep track of the previous audio samples/other state.

You can achieve that in a variety of ways, but the easiest way is to just have a class that holds all the state of the DSP and then have an array/other container of those DSP objects as a member of the AudioProcessor subclass.

@Xenakios yes I understand that…what I’m asking is in a case like the one I’ve shown above, what’s the correct way to handle that situation without separating the channels through branching? I know you’ve said put it in a class, but if I’m telling this to a beginner that is just trying to make sure the dsp is working properly before placing it in its own class, what is the best way?

The best way is to tell them to use a class for the DSP, no matter how simple the DSP is. They will very quickly need to use classes for that anyway. Almost any non-trivial DSP requires keeping track of some state and it very quickly becomes a complete mess to handle within the AudioProcessor subclass itself.

Or is it better for me to say to always do your dsp in its own class from the start , because you need to handle everything separately for each channel and that can’t be sensibly achieved by putting the functions themselves in the process block…

Ha sorry you replied before I clariefied! Ok great. So then to round things up, the reason so many people run into this problem (like me) is because as a general rule you should put your dsp processes out to a separate class, and create separate objects to handle the dsp separately for each channel…do I understand that right?

Yes.

I haven’t looked through the Juce tutorials for a while, but don’t they really say anything about this issue…?

I haven’t seen anything on it personally…I run a tutorial series called The Audio Programmer channel on YouTube and would like to do a video on it, possibly today, so just want to make sure I’m putting out the right info beforehand.

It’s a tricky topic because ideally you wouldn’t want to confuse beginners with having to use classes “just to do a calculation”. But they really will bump into having to do that very quickly. Only the most trivial DSP processing like changing volume or a simple stateless distortion can be done without classes. (Or without a massive mess of member variables in the AudioProcessor subclass.)