madskjeldgaard / portedplugins

A collection of plugins for the SuperCollider sound environment, all of which are ported / remixed from elsewhere
GNU General Public License v3.0
180 stars 13 forks source link

VAxx filters' frequency parameter not scaled properly #28

Open devilfish707 opened 2 years ago

devilfish707 commented 2 years ago

Describe the bug

Cutoff frequency of all the zdf filters is scaled too high

To Reproduce

{ VALadder.ar(Saw.ar(200), 50)}.freqscope

Screenshots

Schermafbeelding 2021-12-20 om 22 46 13

Desktop (please complete the following information):

jamshark70 commented 2 years ago

I can confirm.

// clearly audible resonance at 440 Hz
// 440 / 144 ~= 3
a = { (VALadder.ar(Saw.ar(110), 144, 0.99, 0) * 0.1).dup }.play;

At 44.1 kHz, this x3 ratio seems to hold consistently, e.g., if I set the cutoff to 110, then I clearly hear the third harmonic 330 Hz resonating.

Work-around-able by writing SynthDefs with ffreq * 0.33333333 perhaps, but better to fix it.

madskjeldgaard commented 2 years ago

I agree - this is definately a problem. Also, kind of related: It sounds to me like the filters have too much gain boost even on low settings. I think this should be fixed in the code. If anyone feels like doing a PR that would be amazing - otherwise I will get around to it some rainy day. Thanks!

jamshark70 commented 2 years ago

This appears to be independent of sample rate btw -- I've tried at 44.1 kHz and 96 kHz, and they both seem to sound the same.

madskjeldgaard commented 2 years ago

This appears to be independent of sample rate btw -- I've tried at 44.1 kHz and 96 kHz, and they both seem to sound the same.

Thanks. That's a helpful observation. I have a suspicion that the problem might be somewhere around here, but I'm not sure yet. It's only a hunch so far:

https://github.com/madskjeldgaard/portedplugins/blob/main/plugins/odinfilters/Korg35Filter.cpp#L51

Particularly the first part - I suspect that the first part should not be 2 * samplerate but 2 / T where T is 1/samplerate.

This is what that section looks like in William Pirkle's Designing Synthesizer's book (first edition):

double wd = 2*pi*Fc;     
double T = 1/SampleRate;       
double wa = (2/T)*tan(wd*T/2); 
double g = wa*T/2;  // <--- i.e. g = tan(wd*T/2)   
// --- final calculation
double alpha = g/(1.0 + g);
madskjeldgaard commented 2 years ago

A slightly related issue here. I think these filters need a little overhaul https://github.com/madskjeldgaard/portedplugins/issues/30

jamshark70 commented 2 years ago

Particularly the first part - I suspect that the first part should not be 2 * samplerate but 2 / T where T is 1/samplerate.

But, if t = 1/sr, then 2/t == 2 / (1/sr) == 2 * sr.

TBH my filter theory isn't that good... would take awhile for me to spot it.

madskjeldgaard commented 2 years ago

Particularly the first part - I suspect that the first part should not be 2 * samplerate but 2 / T where T is 1/samplerate.

But, if t = 1/sr, then 2/t == 2 / (1/sr) == 2 * sr.

TBH my filter theory isn't that good... would take awhile for me to spot it.

haha good point - also explains why there was no difference when I messed with this ;)

jamshark70 commented 2 years ago

One other detail: VALadder and VAKorg35 seem to filter at the given frequency 3. VADiodeFilter is at given frequency 2.17. No idea why.

madskjeldgaard commented 2 years ago

Comparing to the port of the same filters and code in the Odin project I honestly cannot tell the difference between how we do it and how they do it.

https://github.com/surge-synthesizer/surge/blob/main/src/common/dsp/filters/K35Filter.cpp#L66