Popping per second with Sine oscillator

Hi all, made a sine oscillator as part of a larger project, have done this many times (even with JUCE) but this time I’m getting a ‘pop’ every second. I’ve checked all the usual issues (the thread by Jules etc) and can’t work it out. I’ve transplanted the sine class into an empty plugin and am still getting the same issue. Any help appreciated.

Below is a snippet of my buffer in the test setup, as well as the sine wave class.

#pragma once
#include "JuceHeader.h"
/***************************************************************************//**
 * Generates a sine wave oscillator. Call PrePlay to set internal sample rate
 *
 * 
 * 
 * 
 * 
 * 
 *
 * a
 * 
 ******************************************************************************/

class Oscillators {
public:

	float master_phase_ = 1.0f;
	float phase_increment_ = 1.0f;
	float sample_rate_ = 1.0f;
	float oscillator1_freq_ = 1.0f;

	float two_pi_ = 6.28318530718f;

	void PrePlay(float sr) {
		sample_rate_ = sr;
		phase_increment_ = two_pi_ / sample_rate_;
		master_phase_ = 0;
	}
	float GetNext() {
		Phase();
		return DoSine(220.0f, 0);
	}


private:
	void Phase() {
		if (master_phase_ > two_pi_) {
			master_phase_ = master_phase_ - (2.0f * two_pi_);
			return;
		}
		master_phase_ += phase_increment_;
	
	}
	float DoSine(float f, float ph) {
		return sinf(two_pi_ * f * master_phase_ + ph);
	}
	
	

};

process block and buffers:

    for (auto i = totalNumInputChannels; i < totalNumOutputChannels; ++i)
        buffer.clear (i, 0, buffer.getNumSamples());

  
    for (int channel = 0; channel < totalNumOutputChannels; ++channel)
    {
        auto* channelData = buffer.getWritePointer (channel);
        for (int i = 0;i < numSamples ; i++) {
            channelData[i] = Osc[channel].GetNext();
        }


        
    }

Does anything change if you change your oscillator phase variables to doubles instead of floats?

1 Like

Sadly not! :face_with_monocle:

What about removing the 2.0f* in that?

Still there! The only other parts I’ve changed are where I create the Oscillator objects (processor header) and where I pass the sample rate to the objects (preparetoplay). No idea what’s going on!

the problem seems to be in the master phase, which is updated every second, which is the size of the loop. But I don’t know how it works.

I can provide you with a more intelligible version in which the frequency and phase are just the speed and angle of the wave in radians. when the phase makes a full turn it restarts.

class Oscillator {
     float phase = 0, freq = 440.0f, sampleRate = 48000;
     float two_pi_ = 6.28318530718f;
    
public:
     void prePlay(float sr) {
         sampleRate = sr;
     }
     void setFrequency(float freqHz) {
         freq = freqHz / sampleRate * two_pi_;
     }
     float GetNext() {
         phase+=freq;
         if (phase> two_pi_) phase-= two_pi_;
         return sinf(phase);
     }
};
1 Like

That doesn’t click! Thank you, slightly unsatisfying still though as I’ve still no idea why my attempt was!

Your DoSine method should have been:

	float DoSine(float f) {
		return sinf(f * master_phase_);
	}

That is, you had an unnecessary multiply by two_pi_.

1 Like

Aha! That’s it! Many thanks

Too fast of me! I carried on using it and found that now it still clicks, but only with decimal frequencies!
I made another test setup, literally default plugin just with this in the process block:

    float inc = juce::MathConstants<float>::twoPi / getSampleRate();
//get increment for accumulator


    
        auto* channelData1 = buffer.getWritePointer (0);
        auto* channelData2 = buffer.getWritePointer(1);
        for (int spl = 0; spl < buffer.getNumSamples(); spl++) {

//accumulator
            phase = phase + inc;
            if (phase > juce::MathConstants<float>::twoPi) {
                phase = 0;
            }
//write to first channel, copy to second 
            channelData1[spl] = sinf(phase * 440.2399);
            channelData2[spl] = channelData1[spl];
        }

Only get a click with a decimal frequency (like 440.2399!!)

Shouldn’t that be

if (phase >= juce::MathConstants<float>::twoPi)
  phase -= juce::MathConstants<float>::twoPi;

Incrementing like this is highly unlikely to end up with an exact value for comparison, so maybe you’re just a bit out of phase every time it passes twoPi?

3 Likes

Still popping! Think I’ve tried every possible combo now! maybe I’ll have to redownload the JUCE library!

Re-installed JUCE library, have done this in a blank audio application and still getting the same click with only decimal frequencies!

    bufferToFill.clearActiveBufferRegion();
    // Your audio-processing code goes here!
    
    // For more details, see the help for AudioProcessor::getNextAudioBlock()

    auto* bufferL = bufferToFill.buffer->getWritePointer(0);
    auto* bufferR = bufferToFill.buffer->getWritePointer(1);



    static float CurrentTime = 0;
    static float increment = 1.0f / sampleRate_;


  
        for (int spl = 0; spl < bufferToFill.buffer->getNumSamples(); spl++) {

            CurrentTime = CurrentTime + increment;
            if (CurrentTime >= 1.0f) {
                CurrentTime = CurrentTime - 1.0f;
            }
            float y = sinf(CurrentTime * 440.2399f * juce::MathConstants<float>::twoPi);

            bufferL[spl] = y;
            bufferR[spl] = y;
        }
   
    


The problem may be that the time is not synchronized with the frequency, if time is 1 + X and you subtract 1, you get X which is not at the same angle. I guess that only integers multiplied by 1 fit in the cycle.

anyway, it is not necessary to know the actual time since the increment in each sample is related to the frequency and the sample rate.

        float freq = 440.2399f / getSampleRate();

        phase+= freq;
        if (phase>1.0f) phase-=1.0f;
        float y = sinf(phase * juce::MathConstants<float>::twoPi);

The way to think about it is that in your code CurrentTime is going to wrap every second but the sine wave that you want probably isn’t going to cycle nicely every second, which means you’ll get a discontinuity in the generated sine wave when when CurrentTime wraps. If that happens every second, then chances are you’ll hear a click/pop.


Aha, thank you v much both you were correct. Not sure why I just couldn’t picture it in my head! I was naively thinking if I just multiplied a 1 second accumulator by frequency that it would do that, but didn’t think about how that it’s necessarily aligned. Plotted it in sheets (lol) which makes it a bit clearer

1 Like