I’m fairly new to using JUCE and I’m trying to make a tremolo plugin. Whilst I’ve got it to work, there are popping sounds that are particularly noticeable at the higher frequencies.
// Initial Values
float frequency = 1.0;
float currentAngle = 0.0;
void TremoloAudioProcessor::prepareToPlay (double sampleRate, int samplesPerBlock)
{
currentSampleRate = sampleRate;
updateAngleDelta();
}
void TremoloAudioProcessor::updateAngleDelta()
{
const double intervalsPerCycle = frequency / currentSampleRate;
angleDelta = intervalsPerCycle * double_Pi;
}
void TremoloAudioProcessor::processBlock (AudioSampleBuffer& buffer, MidiBuffer& midiMessages)
{
const int totalNumInputChannels = getTotalNumInputChannels();
const int totalNumOutputChannels = getTotalNumOutputChannels();
const int numSamples = buffer.getNumSamples();
for (int i = totalNumInputChannels; i < totalNumOutputChannels; ++i)
buffer.clear (i, 0, buffer.getNumSamples());
// Processing
for (int channel = 0; channel < totalNumInputChannels; ++channel)
{
float* channelData = buffer.getWritePointer (channel);
for (int i = 0; i < numSamples; ++i)
{
// Calculate modulation
mod = (sin(currentAngle) / 2) * 0.5;
currentAngle += angleDelta; // Increment for next sample
channelData[i] *= mod; // Sample * Modulation
// Wrap angle
if (currentAngle >= double_Pi)
{
currentAngle -= double_Pi;
}
}
}
}
Ah, this mistake again. We must have seen the same thing in different forms a hundred times on the forum! I wish there was some way we could stop people doing the same thing.
The mistake is to have some kind of state (in your case currentAngle) and to update it once per sample, but then to go round your outer loop again to do the other channels, but re-using the same state again. You need to either have separate values for each channel, or to make your outer loop the one that does samples, and the inner loop the one that does channels.
@noahdayan Maybe we need to make sure this is prominently demonstrated in all our plugin demo code, it’d save a lot of people a lot of confusion!
The other issue is that you reuse the state for all channels, which you won’t notice as a problem until you try this with 2+ channels. (Where the frequency will apparently change proportion to the number of channels.) Essentially, you need a currentAngle per channel.
@martinrobinson-2 I did think that this was the case with the double_Pi , however it seems to modulate at twice the frequency? I’ve no idea as to why, but it seems to work fine if I don’t multiply it by 2.
Not sure about that - as currentAngle is nearly always less than double_Pi, branch prediction will work well, making the if() cheaper than so many fmod() calls.
I think the problem with std::fmod is that it effectively has to do a divide which is a very slow instruction.
With a while loop, the branch predictor can almost always guess which branch to take so the branch is effectively free and most of the time nothing is done (as the variable is inside the mod limit most of the time).
That’s probably one of the other most-frequent mistakes we see here! So I’ve just done some refactoring to eliminate double_Pi from our codebase and examples.
We already had some templated constants e.g. MathConstants::pi, so just made all our code use that instead. Would recommend users do the same!
(I’ve left the old symbol there for backwards-compatibility, but might deprecate it at some point)
That’s impressive, thanks!
Also the quick-bench will definitely go into my favourite bookmarks!
I wrongly assumed, that the implementation of fmod would be quite similar, but if it actually divides, then it becomes obvious, that I had to be wrong…
I think the problem stems from accuracy. I think a std::fmod has to have an accuracy guaranteed by the standard (or at least the architecture it’s running on) so it doesn’t matter how much over your y value you call for x, the result needs to have a constant error.
Doing a while loop compounds a small subtraction error so is likely to differ slightly from fmod.
In practice, this won’t matter for even audio signals, the noise floor is too low. For phase wrap arounds like this the accuracy certainly won’t matter. If you were doing some scientific computing however…
The other thing to be aware of is that this is probably optimized for the case where x is close to y. If you feed it a value much larger than y, it will have to loop and do the subtraction multiple times which may be slower. I think it’s still faster for around 10 times but there’s probably a crossover somewhere. If someone wants to knock up a quick-bench I’d be interested to see it…
which only works, if the increment was < 2 * π… but I didn’t want to go nitpicking… But the while version works all the times with the performance limitation you mentioned…
AND the increment has to be positive… fmod needs to handle negative numbers as well…
fmod needs to handle negative numbers as well…