Issue building a basic biquad fiter


#1

Hello everyone,

I’ve just started getting into JUCE (with a fairly decent DSP and C++ background), and as I’ve seen others try, I want to build a basic biquad filter (I know the IIR object exists, but I want to build this from scratch to get familiar with the way JUCE works).

Anyway, the idea seems pretty simple: keep a history of input values and output values so that I have the delayed inputs and outputs needed for the difference equation calculation. I could do this only storing the two values outside the new input block for each new block, but to keep things clear in my head I created x_hist(N+2) (input) and y_hist(N+2) (output) AudioSampleBuffers (with N being the blocksize) to contain this data and pass it to/from the mainInputOutput Buffer.

Problem is, I can’t get it to work right, and have been debugging for hours without success, going through many similar topics on the forum… basically, in the processBlock method the values [0,1] of the previous input block are copied to [N,N+1], the new input buffer data is then stored into [0,N-1], which should ensure a stream of N+2 samples of coherent data needed for the difference equation…
The strange thing is that the copy works fine (first samples of previous block are copied to last two samples of the “history” buffer), but once the new block data is copied into that buffer, the end values [up to N-1] are not coherent with the stored values [N,N+1], which of course causes very noticeable clicks. I’ve checked this by using breakpoints in the code and looking into a float array that I copy the buffer content into (super dirty way of doing things, but hey!).

At this point I’m thinking either:
1/ I missed something so stupid that I can’t find it in my code, no matter how much I look.
2/ I am missing something fundamental about how AudioSampleBuffer and processBlock work and I’m going about this the wrong way.

Any help would be greatly appreciated!
This is my first post on the forum so I hope the format is OK, I will post part of my code below:

    void prepareToPlay (double /*sampleRate*/, int /*maxBlockSize*/) override { 

	int bSize = this->getBlockSize();
	int numChannels = this->getMainBusNumInputChannels();
	
	// 2nd order filter so we need a history going back by 2.
	// Only set the size if it is not correct (i.e. it hasn't been initialised yet)
    // The x_hist and y_list are allocated empty inside the constructor
	if (x_hist->getNumSamples() != bSize + 2) {
		x_hist->setSize(numChannels, bSize + 2, true);
		x_hist->clear();
	}
	if (y_hist->getNumSamples() != bSize + 2) {
		y_hist->setSize(numChannels, bSize + 2, true);
		y_hist->clear();
	}
}

Process Block code:

  void processBlock (AudioSampleBuffer& buffer, MidiBuffer&) override
{
    AudioSampleBuffer mainInputOutput = busArrangement.getBusBuffer (buffer, true, 0);

	float a0 = *p_a0;
	float a1 = *p_a1;
	float a2 = *p_a2;
	float b1 = *p_b1;
	float b2 = *p_b2;

	int N = buffer.getNumSamples();

	// Write the first two samples of the old values to the end of the history buffers
	for (int j = 0; j < 2; j++) {
		for (int i = 0; i < mainInputOutput.getNumChannels(); i++) {
			*x_hist->getWritePointer(i, N + j) = *x_hist->getReadPointer(i, j);
			*y_hist->getWritePointer(i, N + j) = *y_hist->getReadPointer(i, j);
		}
	}

	for (int j = 0; j < N; j++)
		for (int i = 0; i < mainInputOutput.getNumChannels(); i++) {
			// Copy the N samples of the input buffer into x_hist (makes things clearer)
			*x_hist->getWritePointer(i, j) = *mainInputOutput.getReadPointer(i, j);
		}

	// For each sample in the buffer (counting backwards so we can calculate the Y(n) series correctly!)
	for (int j = N - 1; j >= 0; j--)
	{
		for (int i = 0; i < mainInputOutput.getNumChannels(); i++) {
			// Calculate the Y(n) series, going up backwards from the older values
			*y_hist->getWritePointer(i, j) =
				a0 * (*x_hist->getReadPointer(i, j))
				+ a1 * (*x_hist->getReadPointer(i, j + 1))
				+ a2 * (*x_hist->getReadPointer(i, j + 2))
				- b1 * (*y_hist->getReadPointer(i, j + 1))
				- b2 * (*y_hist->getReadPointer(i, j + 2));
		}
	}
	
	// Final copy to the output buffer
	for (int j = 0; j < N; j++)
		for (int i = 0; i < mainInputOutput.getNumChannels(); i++) {
			mainInputOutput.setSample(i, j, *y_hist->getReadPointer(i, j));
		}
}

I’m on Windows, building with VS2015, and debugging with the Plugin_Host example code.


#2

Hi PlasticLobster,

you use AudioProcessor::getBlockSize(), I hope you noticed the documentation:

This can be called from your processBlock() method - it’s not guaranteed to be valid at any other time.

So if it ends up a sensible value, you have just been lucky. Instead the maxBlockSize from the prepareToPlay() call is the one you actually wanted to use…

Remember it’s not the ONLY block size that may be used when calling processBlock, it’s just the normal one. The actual block sizes used may be larger or smaller than this, and will vary between successive calls.

You should design your code to work also in shorter blocks. I would suggest to add at the end of processBlock the last two samples at the beginning of the temporary buffer. Then copy the new block in the next processBlock to the samples 2 to block->getNumSamples() + 2.

Next little performance consideration: the samples per channel are aligned in memory. You placed your samples loop as outer loop and then iterate inside through the channels. You will get a great speed up, if you just swap these two loops, because you access subsequent memory addresses, so the cache can be used best.

Also to copy samples, don’t iterate but use the AudioBuffer::copyFrom(), which will use SIMD operations.

Hope there was something useful for you.


#3

Hi Daniel and thanks a lot for your answer,

I had seen the getBlockSize() warning, but hadn’t quite understood the way block size worked, thanks for the info it’s clear now!

My main problem was that I thought the buffer samples were ordered [newest at 0 ; oldest at blockSize-1], when in fact the oldest sample is the 0 index and the newest one is at blockSize-1… So basically all my calculations and sample value shifts were working backwards! Silly mistake but I actually didn’t see find anything very explicit about it in the documentation (although it makes sense for it to work this way in case the block is shorter for any reason).

Everything works OK now, and I’ve adapted the code with some of your advice (thanks for the performance tips also!).

Here is the code, in the hope that it can avoid someone making the same mistakes as me:

    void prepareToPlay (double /*sampleRate*/, int maxBlockSize) override { 

	int numChannels = this->getMainBusNumInputChannels();
	
	// 2nd order filter so we need a history going back by 2.
	// Only set the size if it is not correct (i.e. it hasn't been initialised yet)
	if (x_hist->getNumSamples() != maxBlockSize + 2) {
		x_hist->setSize(numChannels, maxBlockSize + 2, true);
		x_hist->clear();
	}
	if (y_hist->getNumSamples() != maxBlockSize + 2) {
		y_hist->setSize(numChannels, maxBlockSize + 2, true);
		y_hist->clear();
	}
}


void processBlock (AudioSampleBuffer& buffer, MidiBuffer&) override
{
    AudioSampleBuffer mainInputOutput = busArrangement.getBusBuffer (buffer, true, 0);

	// Filter coefficient scaling (temporary)
	float a0 = 2 * (*p_a0) -1;
	float a1 = 2 * (*p_a1) - 1;
	float a2 = 2 * (*p_a2) - 1;
	float b1 = 2 * (*p_b1) - 1;
	float b2 = 2 * (*p_b2) - 1;

	int N = buffer.getNumSamples();

	// Write the latest two samples of the old values to the start of the X(n) and Y(n) buffers
	for (int i = 0; i < mainInputOutput.getNumChannels(); i++) {
		for (int j = 0; j < 2; j++) {
			*x_hist->getWritePointer(i,  j) = *x_hist->getReadPointer(i, j+N);
			*y_hist->getWritePointer(i,  j) = *y_hist->getReadPointer(i, j+N);
		}
	}

	// Fill the X(n) buffer from 2 to N+2 with the new input data
	for (int i = 0; i < mainInputOutput.getNumChannels(); i++)
		x_hist->copyFrom(i, 2, mainInputOutput.getReadPointer(i, 0),N);

	// Calculate the Y(n) series (new values from 2 to N+2)
	for (int i = 0; i < mainInputOutput.getNumChannels(); i++) {
		for (int j = 0; j < N; j++) {
			*y_hist->getWritePointer(i, j+2) =
				a0 * (*x_hist->getReadPointer(i, j+2))
				+ a1 * (*x_hist->getReadPointer(i, j + 1))
				+ a2 * (*x_hist->getReadPointer(i, j))
				- b1 * (*y_hist->getReadPointer(i, j + 1))
				- b2 * (*y_hist->getReadPointer(i, j));
		}
	}
	
	// Copy the current Y(n) series to the output buffer
	for (int i = 0; i < mainInputOutput.getNumChannels(); i++)
		mainInputOutput.copyFrom(i, 0, y_hist->getReadPointer(i, 2), N);
}

Thanks for you help!