Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Second order All pass "so_apf.h" might have missing frequency #5

Open
visuttaja opened this issue Feb 24, 2022 · 6 comments
Open

Second order All pass "so_apf.h" might have missing frequency #5

visuttaja opened this issue Feb 24, 2022 · 6 comments

Comments

@visuttaja
Copy link

I can see at top comment section mentioning frequency but parameters are only for Q and samplerate(fs).
Then when I test filter it gives only abrupt bump. Happily nothing worse happens though.
I think one of those fs's must be the frequency but I don't understand much about filters math so I cannot correct the situation my self. Maybe you could kindly review the second order all pass code to clarify which is the frequency - if any.
It is this part which I feel is missing frequency parameter and corresponding code part in file so_apf.h:

tp_coeffs& calculate_coeffs(float Q, int fs)

@visuttaja
Copy link
Author

Ok. I found some info! All pass filter is supposed to change only phase of signal keeping level of different frequencies the same. But still the abrupt bump is not right output? Or is it after all?

@dimtass
Copy link
Owner

dimtass commented Feb 26, 2022

For the first and second order all pass filters the meaning of Q and fc are:

  • fc, corner frequency
  • Q, steepness of phase shift at fc (second-order only)

For more info I could refer you to the chapter 6 (6.6.7) of this book here.

All the filters in this code are derived from this book, which worth every cent.

Regards.

@visuttaja
Copy link
Author

Yes. I understand. That is why I pointed out that in the code you publish the formal parameter names for second order all pass filter are
tp_coeffs& calculate_coeffs(float Q, int fs)
where it probably should be like you point out
is tp_coeffs& calculate_coeffs(float Q, int fc)

Note the "fs" changes to "fc" like you point out.

And if so then fc should be used in code instead of fs.
I did implement all the filters you kindly provided in java and all them worked fine except
second order all pass filter. So I found that the guidance in that file is
/**

  • Second order all-pass filter
  • Dimitris Tassopoulos 2016-2020
  • fc, corner frequency
  • Q, steepness of phase shift at fc (second-order only)
    */
    But in code there is no fc used. And not as parameter either. So it is not consistent and there is something going on.
    Then I concluded that possibly there would be some typo you are aware if you check it out and would be able to solve immediately with you insight on the matter.
    I mean....If there is error you would like to know about it no matter what is the source?
    I am not blaming you you see. Just telling.

@dimtass
Copy link
Owner

dimtass commented Feb 27, 2022

Thanks for the feedback. It's possible that there are bugs in the code and there were a couple of PRs also from other users to fix them.

@lorenzolamasse
Copy link

lorenzolamasse commented May 12, 2022

Actually, Will Pirkle issued some errata including one for the 2nd order APF (page 188), alpha is correct, but beta relies on thetac, which is not described and your code is apparently wrong there. It should read:
coef_size_t a = (tan(pi * Q / fs) - 1.0) / (tan(pi * Q / fs) + 1.0); // same as your code
coef_size_t b = -cos(2 * pi * fc / fs); // instead of -cos(pi * Q/fs)
Source here: https://www.willpirkle.com/support/errata/
Hope that helps ;-)

@dimtass
Copy link
Owner

dimtass commented May 14, 2022

@lorenzolamasse good find. I'll take a look at the errata and the book and fix any errors. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants