Has this simple Circular Buffer code got a slight error, or is it OK?

I have been following a tutorial on Youtube about how to do a Circular Buffer.
I’ve been learning a lot from the Audio Programmer’s channel.

So here he is writing the bufferData to fill up to the end buffer (in the 1st line), and he’s circling around to 0 again to start writing at the start of the same buffer (the 2nd line), which is fine:

# Juce Tutorial 40- Building a Delay Plugin Pt 1 (Creating a Circular Buffer)

Here’s my question though: In the 2nd line .copyFromWithRamp() I see its just copying bufferData again. Shouldn’t it be bufferData minus the the bufferData thats been written in the first line?
Isn’t this writing duplicate bufferData … or not ?

couple things:

no, it’s not duplicating, because if you look at the arguments for juce::AudioBuffer::copyFromWithRamp, you will see that you’ve actually passed the function separate startSample and numSamples arguments, so theoretically this could work.

However, there are a couple ways this could be improved:

IDK if you check for this elsewhere in your code, but I believe that there is a jassert inside copyFromWithRamp that checks that the numSamples argument is greater than 0. So, if bufferLength - bufferRemaining ends up being 0 or less than 0, you will get an assertion failure.

You can fix this by doing:

mDelayBuffer.copyFromWithRamp (channel, mWritePosition, bufferData, bufferRemaining, 0.8, 0.8);  // no change here...

const int sampsLeft = bufferLength - bufferRemaining;

if (sampsLeft > 0)
    mDelayBuffer.copyFromWithRamp (channel, 0, bufferData, sampsLeft, 0.8, 0.8);

One other suggestion, more minor: you’re using the “copy with ramp” function, but your ramp’s start gain and end gain are the same. You could either use AudioBuffer’s copyFrom and then applyGain functions separately to make it clear that it’s not really a ramp, or you can do it in one function call with JUCE’s FloatVectorOperations class, like this:

FloatVectorOperations::copyWithMultiply (mDelayBuffer.getWritePointer(channel),
                                         bufferRemaining);  // last arg here is number of values

thanks, but the startSample: isn’t it the “destination” start sample, i.e. the point at which we start writing into the destination buffer? The startSample is not telling us what bufferData we should be reading.
Am I wrong on that?
(thanks for the jassert heads up, didn’t know about that.)

1 Like

There is a bug and IIRC that was covered in one of the comments in the video. You need to advance the read pointer in the second line by the amount of what was copied in the first line.

1 Like

ah, yes, you’re right, I didn’t catch that!

so even though that function doesn’t have a “reading start sample”/“source start sample” argument, you can pretend there is one by simply advancing the pointer you pass in to source, using pointer arithmetic:

mDelayBuffer.copyFromWithRamp (channel, mWritePosition, bufferData, bufferRemaining, 0.8, 0.8);  // this is still the same

const int sampsLeft = bufferLength - bufferRemaining;

if (sampsLeft > 0)
    mDelayBuffer.copyFromWithRamp (channel, 0, 
                                   bufferData + bufferRemaining, // advance the read pointer by how many samples we copied before
                                   sampsLeft, 0.8, 0.8);

the AudioBuffer’s .getReadPointer function does also have an optional second argument for what sample number the pointer will start at, so that second copy operation could also look like this:

const int sampsLeft = bufferLength - bufferRemaining;

if (sampsLeft > 0)
    mDelayBuffer.copyFromWithRamp (channel, 0, 
                                   sourceAudioBuffer.getReadPointer (channel, bufferRemaining);
                                   sampsLeft, 0.8, 0.8);

This one appeals to me more. Will it work equally as well as your other option with getReadPointer?
(bufferData is a const float*, bufferRemaining is an int I think, or double.)
I’m asking because I have put the buffer filling code in a different function and am calling that.
Otherwise I guess I’ll have to pass the juce::AudioBuffer<float>& buffer as an argument to that function.

this should work to encapsulate inside a function!