Tremolo plugin - Artefacts at high frequencies


#1

Hi,

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;
	    	}
    	}
    }
}

Any ideas?


#2

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!


#3

I think you’ve misunderstood double_Pi to be equal to 2π whereas ‘double’ refers to double-precision (64 bit):

void TremoloAudioProcessor::updateAngleDelta()
{
    const double intervalsPerCycle = frequency / currentSampleRate; 
    angleDelta = intervalsPerCycle * double_Pi * 2.0;
}

and

	        // Wrap angle 
    	    	if (currentAngle >= double_Pi)
    	    	{
    	    		currentAngle -= double_Pi * 2.0;
    	    	}

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.


#4

Beaten to it! :slight_smile:


#5

Ah I’m an idiot! Thank you!

@martinrobinson 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.


#6

OK, so it was a combination of the two issues:

  • The bug where you reused the state across both channels bumped the frequency up to x2; but
  • The waveshape would have been incorrect due to only using the the first half of the sine shape.

#7

the nicest way to write that:

currentAngle = std::fmod (currentAngle + angleDelta, double_Pi * 2.0);

Also @KNJturtle have a look at dsp::Oscilator, that’s currently cutting edge in JUCE…


#8

Thanks! Everything works perfectly now :slight_smile:


#9

Does that mean, “the most cpu efficient way” ?


#10

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.


#11

For a less home-made approach to building an oscillator, be sure to look at the DSP module!


#12

Fair point, I doubt it will be measurable, but there could be a difference.

But the more interesting note is what Jules and I linked above, the dsp::Oscilator…


#13

[quote=“daniel, post:7, topic:25452”]Fair point, I doubt it will be measurable, but there could be a difference.
[/quote]

It’s definitely measureable, about 3x faster in my experiments.
http://quick-bench.com/FLCiEq69Nkcv4-D6aLjfMQOyJYI

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).


#14

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)


#15

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…

Thanks again.


#16

@IvanC I guess that means we need to update the dsp::Oscillator implementation!


#17

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…


#18

Yes, the original post even only did

if (currentAngle >= double_Pi)
{
    currentAngle -= double_Pi * 2.0;
}

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… :wink: fmod needs to handle negative numbers as well…


#19

Oh yeah, I might have a look at some point if you want to update Fabian’s oscillator code with the recommendations from here :wink: