annoying problem

[likeregularchickens]

Hi,

I've got a really annoying problem that i cant figure out and i've been trying for a *long* time...
I'm making a modular synth, and i've got a sin generator module, and an add module which just adds two streams together.

i'm trying to get some FM going, and i've got at the moment:

sinegenerator 'modulator' with freq 1hz, and amplitude 1 - so values are -1 up to 1.

'modulator's output is added to a constant value of 800...

this total is then used for the frequency of the sine generator 'carrier', with amplitude 2000. this gives should give a sinewave with a frequency that goes between 799 and 801... this is then played through the speakers.

now, im sure in theory this is correct (?), and i think its starts off ok, but i'm getting results that sound like 'modulator's amplitude is getting larger over time. eg the difference in frequencies around 800mhz is getting larger over time. I've checked and checked and checked and the output from modulator is definately correct. I think it must be to do with my sine wave generator, but it generates tones perfectly... it's just when it comes to this FM example its all going wrong...

I'm looking at the index variable specifically but i cant figure it out. I think maybe its happening as the index gets high, but that doesnt make much sense to me. however, if i reset index to 0 at some point it just goes back to the beginning. So.. i guess what i'm asking is, is this code rubbish, and if so how should it be done?

Code: Select all

// pFreqBuffer holds the frequency value
// pAmpBuffer holds the amplitude

for(UINT i=0; i<numSamples; i++)
{
	UINT value = CAudioStream::GetSampleRate();

	double t = float(index++)/CAudioStream::GetSampleRate();

	val = 6.2831385307179586f*pFreqBuffer[i]*t;

	m_pBuffer[i] = pAmpBuffer[i]*sin(val);

}

if anyone could help i'd be very grateful.
I've uploaded a sound here:
http://rapidshare.de/files/7973881/sound.wav.html
if you listen you can see its definately not constant...

Thanks alot.

[drinelli]

I don`t want to reread what you have written, but is Fm not * instead of +,

[drinelli]

No sorry that is ringmodulation, bad memory.

[drinelli]

I was going to ask something I think was rubbish, but on the other hand I could be wrong about it being rubbish (do not reckognize the language you program in either) But here it goes, if you use deep tones as a modulator the sine curve will be slow, as a result the sound will change over time, in a cyclick way, it also might be that you have accidently synchronized the carrier with the modulator. Have not listened to your audio though cause I am trying to avoid linking to anything. But I hope I have been able to help you in some way.

[likeregularchickens]

hmm thanks for your help. I was hoping to get by using floats and then convert to short without any real dithering but i've had to just use shorts (16 bit) the whole way now. I got better results doing this but there was still big problems.

I've modulated the carrier sine wave with a square wave and that sounds fine, so I think there must be something wrong with my sine generator code (although it still sounds fine on its own).

[Fuzzpilz]

Oh boy... several issues.

- You're calling your sample rate retrieval function twice per sample and not even doing anything with one of the returned values. Don't do it more often than once, outside the loop.

- You're probably better off going back to floats all the way to the end, provided you learn a little more about how floats work.

- You're calling sin() directly. Bad idea. Instead, use e.g. a table lookup or an IIR oscillator.

- Do not use a time counter the way you're doing. If you want to use sin() (or a table, which I recommend), use a wrapping phase accumulator, i.e. a counter that stays between 0 and whatever represents 2*pi. One reason is that floats lose accuracy the further you move from 0; another is part of my next point:

- You're not doing the FM quite right. Think of your oscillator's output as a continuous-time function: sin(p(t)), if you like. Its "instantaneous" frequency is not perceived as p(t)/t, but as dp(t)/dt. If p(t)=k*t for some constant k, there is no difference between the two (and since square waves are piecewise constant, using a low-frequency (relative to the carrier) square wave as modulator isn't a problem either), but as soon as it's not, you get problems.

Some code that isn't actually good, but will perhaps illustrate some of my points:

Code: Select all

float sin_lookup(float x)
{
    // insert interpolated lookup in a 1024-sample sin table here
}

float freqmul=1024.0f/CAudioStream::GetSampleRate();
for(i=0; i<numSamples; i++)
{
    phase+=pFreqBuffer[i]*freqmul; // increment phase accumulator
    if(phase>1024)phase-=1024; // wrap if necessary
    m_pBuffer[i]=pAmpBuffer[i]*sin_lookup(phase); // output
}

[Frank123]

Nice answer from fuzzpilz , but I think index at nTableSize would not work, so use:

Code: Select all

if(phase>=1024)phase-=1024;

Or append extra values (1024+)for easier interpolation at the end of the table which repeat the first values.

Code: Select all

[drinelli]

Though I do not understand much of what is said here, I would suggest not to listen to my thoughts alltogether, I really do not anywhere sound like an expert here, I bow in the dust for superior minds

[likeregularchickens]

Thanks alot Fuzzpilz, your answer helps alot. I guess it must have been because the floats were losing accuracy that the sound was changing over time. With the original code I was just trying to get something working, and then I was going to address the bad coding practises like using the sin function later, but obviously it saves problems to be efficient as you code.

One last question... is a 1024 sample lookup table usually big enough? seems kind of small to me.

Thanks again.

[Fuzzpilz]

It's big enough if you interpolate. If you don't, you should probably use a bigger table - 32768 would be more than enough in that case.