ddiakopoulos / MoogLadders

:sound: Collected C++ implementations of the classic 4-pole moog ladder filter
The Unlicense
319 stars 41 forks source link

SimplifiedMoog issues #3

Closed SwissFrank closed 9 years ago

SwissFrank commented 10 years ago

Many thanks ddiakopoulos for taking the time to organize these modules and make them available.

So far I've tried SimplifiedMoog code verbatim and am happy to report that cutoff and resonance can be modulated in real time. However I have some questions.

1) variable _output is used in the stage 1 input, but not initially set (or, say, reset to zero). Shall we explicitly set it to zero around say line 16?

2) line 85 " _output = _stage[3] " is outside the oversampling loop, so the second oversample's stage0 will begin with the previous sample's output, not the first oversample's output. Should it perhaps be moved to line 82?

3) not important but lines 14-15 has no effect thanks to line 22-23.

I then have the following questions:

1) at zero resonance, the cutoff seems to be about 3 octaves below the argument in Hz. For instance, when supplied with 3520 (A7), the cutoff seems to be 440Hz(A4).

image

As resonance is increased, a peak forms about 1.25 octaves below where it ought to be:

image

I should confirm: is the input units for computeCutoff() meant to be Hz?

SwissFrank commented 10 years ago

I believe you also want to _stageTAHNH[] should also be cleared to 0.0 around line 19.

In experimenting how to move the resonant peak up to where it should (that is, if the input of computeCutoff() is meant to be in Hz), I found I had to multiply frequency by around 2.41509433. This happens to be about PI/1.3, which is interesting as _g is already based on pi and h is based on 1.3. I'm wondering if something somewhere should have been squared? (Or whether computeCutoff()'s input is in some units other than Hz?)

Accordingly I added the following correction factor:

_g = (2 * M_PI) * _cutoff / x2; // feedback coefficient at fs*2 because of doublesampling
_g = _g * M_PI / 1.3; // correction factor that allows _cutoff to be supplied Hertz

Also note that while this corrects the resonant peak, this filter still seems to start the cutoff another octave and a half before the resonant peak, unlike my Butterworth filter implementation whose cutoff slope starts at the same frequency the resonant peak builds.

ddiakopoulos commented 10 years ago

Do you think you could open a pull request for the changes that you propose? This was adapted from a Matlab sample from the DAFX 2nd Ed. book. Here's the original Matlab impl they propose in the book.

function [out,out2,in2,g,h0,h1] = moogvcf(in,fc,res)
% function [out,out2,in2,g,h0,h1] = moogvcf(in,fc,res)
% Authors: Välimäki, Bilbao, Smith, Abel, Pakarinen, Berners
% Parameters:
% in = input signal
% fc= cutoff frequency (Hz)
% res = resonance (0...1 or larger for self-oscillation)
%
%--------------------------------------------------------------------------
% This source code is provided without any warranties as published in 
% DAFX book 2nd edition, copyright Wiley & Sons 2011, available at 
% http://www.dafx.de. It may be used for educational purposes and not 
% for commercial applications without further permission.
%--------------------------------------------------------------------------

fs = 44100;  % Input and output sampling rate
fs2 = 2*fs;  % Internal sampling rate
% Two times oversampled input signal:
in = in(:); in2 = zeros(1,2*length(in)); in2(1:2:end) = in; 
h = fir1(10,0.5); in2 = filter(h,1,in2);  % Anti-imaging filter
Gcomp = 0.5;  % Compensation of passband gain
g = 2*pi*fc/fs2;  % IIR feedback coefficient at fs2
Gres = res;  % Direct mapping (no table or polynomial)
h0 = g/1.3; h1 = g*0.3/1.3; % FIR part with gain g
w = [0 0 0 0 0]; % Five state variables
wold = [0 0 0 0 0];  % Previous states (unit delays)
out = zeros(size(in)); out2 = zeros(size(in2));
for n = 1:length(in2),
    u = in2(n) - 4*Gres*(wold(5) - Gcomp*in2(n));  % Input and feedback
    w(1) = tanh(u);  % Saturating nonlinearity
    w(2) = h0*w(1) + h1*wold(1) + (1-g)*wold(2);  % First IIR1
    w(3) = h0*w(2) + h1*wold(2) + (1-g)*wold(3);  % Second IIR1
    w(4) = h0*w(3) + h1*wold(3) + (1-g)*wold(4);  % Third IIR1
    w(5) = h0*w(4) + h1*wold(4) + (1-g)*wold(5);  % Fourth IIR1
    out2(n) = w(5);  % Filter output
    wold = w;  % Data move (unit delays)
end
out2 = filter(h,1,out2);  % Antialiasing filter at fs2
out = out2(1:2:end);  % Decimation by factor 2 (return to original fs)
SwissFrank commented 10 years ago

Thanks Dimitri.

I don't know Matlab, but I can kind of understand.

Question: does this simply duplicate each input value twice, or does it interpolate? If it doesn't interpolate, should it? Does the end result simply take every second result, or does it average? If it doesn't average, should it?

  in2(1:2:end) = in;

  out = out2(1:2:end);  % Decimation by factor 2 (return to original fs)

Second question: the Matlab example has just one tanh()? And the C++ code has 4?

I know almost nothing about GIT or about your project. By "pull request," do you mean, you would like me to make revisions and submit them? Or is that another way to suggest the changes? If so I'm happy to, I just want to understand your suggestion.

On Wed, Jan 15, 2014 at 2:45 AM, Dimitri Diakopoulos < notifications@github.com> wrote:

Do you think you could open a pull request for the changes that you propose? This was adapted from a Matlab sample from the DAFX 2nd Ed. book. Here's the original Matlab impl they propose in the book.

function [out,out2,in2,g,h0,h1] = moogvcf(in,fc,res) % function [out,out2,in2,g,h0,h1] = moogvcf(in,fc,res) % Authors: Välimäki, Bilbao, Smith, Abel, Pakarinen, Berners % Parameters: % in = input signal % fc= cutoff frequency (Hz) % res = resonance (0...1 or larger for self-oscillation) % %-------------------------------------------------------------------------- % This source code is provided without any warranties as published in % DAFX book 2nd edition, copyright Wiley & Sons 2011, available at % http://www.dafx.de. It may be used for educational purposes and not % for commercial applications without further permission. %--------------------------------------------------------------------------

fs = 44100; % Input and output sampling rate fs2 = 2_fs; % Internal sampling rate % Two times oversampled input signal: in = in(:); in2 = zeros(1,2_length(in)); in2(1:2:end) = in; h = fir1(10,0.5); in2 = filter(h,1,in2); % Anti-imaging filter Gcomp = 0.5; % Compensation of passband gain g = 2_pi_fc/fs2; % IIR feedback coefficient at fs2 Gres = res; % Direct mapping (no table or polynomial) h0 = g/1.3; h1 = g_0.3/1.3; % FIR part with gain g w = [0 0 0 0 0]; % Five state variables wold = [0 0 0 0 0]; % Previous states (unit delays) out = zeros(size(in)); out2 = zeros(size(in2)); for n = 1:length(in2), u = in2(n) - 4Gres(wold(5) - Gcomp_in2(n)); % Input and feedback w(1) = tanh(u); % Saturating nonlinearity w(2) = h0_w(1) + h1_wold(1) + (1-g)_wold(2); % First IIR1 w(3) = h0_w(2) + h1_wold(2) + (1-g)_wold(3); % Second IIR1 w(4) = h0_w(3) + h1_wold(3) + (1-g)_wold(4); % Third IIR1 w(5) = h0_w(4) + h1_wold(4) + (1-g)_wold(5); % Fourth IIR1 out2(n) = w(5); % Filter output wold = w; % Data move (unit delays) end out2 = filter(h,1,out2); % Antialiasing filter at fs2 out = out2(1:2:end); % Decimation by factor 2 (return to original fs)

— Reply to this email directly or view it on GitHubhttps://github.com/ddiakopoulos/MoogLadders/issues/3#issuecomment-32288201 .

ddiakopoulos commented 9 years ago

I fixed a bunch of these issues in a recent update!