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

Coefficients for odd-order highpass filters mixed up? #29

Open
galchinsky opened this issue Sep 20, 2016 · 4 comments
Open

Coefficients for odd-order highpass filters mixed up? #29

galchinsky opened this issue Sep 20, 2016 · 4 comments

Comments

@galchinsky
Copy link
Collaborator

https://www.kvraudio.com/forum/viewtopic.php?f=33&t=386712 says that Biquad.cpp may contain an error and suggests the following patch:

--- a/DSPFiltersCPP/source/Biquad.cpp 
+++ b/DSPFiltersCPP/source/Biquad.cpp 
@@ -143,8 +143,8 @@ void BiquadBase::setOnePole (complex_t pole, complex_t zero) 
const double a0 = 1; 
const double a1 = -pole.real(); 
const double a2 = 0; 
- const double b0 = -zero.real(); 
- const double b1 = 1; 
+ const double b0 = 1; 
+ const double b1 = -zero.real(); 
const double b2 = 0; 

But I don't undestand if this is correct solution or ad-hoc workaround which can break someone's code. If you know more, let me know.

@sagamusix
Copy link

All I can say from a few quick experiments is that the solution does indeed fix the issue of phase inversion, and that it does not appear to alter the sound, so the suggested fix from that thread is possibly correct.

@IkerMesa
Copy link

IkerMesa commented Nov 17, 2016

The computation of odd-order high-pass filters of type Butterworth is broken.
I have checked it with Matlab:
If I run this filter with C++,
for(int theOrder=1; theOrder<20; ++theOrder)
{
Dsp::SimpleFilter <Dsp::Butterworth::HighPass<20>, 1> myFilter; //1 channel, 20th order. Latter i can change the order.
myFilter.setup(theOrder, fs, cuttOffFreq);
myFilter.process(numSamples, mySignal);
}

If I perform the same filtering with MATLAB: Filter algorithm:
[z,p,k] = butter(theOrder, (2_cuttOffFreq / fs) , 'high');
[sos,gain] = zp2sos(z,p,k);
f = dfilt.df2sos(sos);
signalFiltered= gain_filter(f, mySignal);

If theOrder is even the resulted signals are the same with this library and with matlab. However, if theOrder is odd the filtered signals are different.

@IkerMesa
Copy link

Hi, any news regarding this issue?

The computation of odd-order high-pass filters of type Butterworth is erroneous.

@berndporr
Copy link

id-0001
Certainly correct that this causes a phase inversion and the fix is correct. @anleu confirmed that now with a unit test using scipy output. Also makes total sense as there is complete symmetry between zero and pole. See my scribbles.

berndporr added a commit to berndporr/iir1 that referenced this issue Oct 5, 2023
This fixes a bug reported already in the original DSPFilters:
vinniefalco/DSPFilters#29 and
confirmed for the JAVA port berndporr/iirj#27.
That's now fixed.
myd7349 added a commit to myd7349/brainflow that referenced this issue Jan 7, 2024
myd7349 added a commit to myd7349/brainflow that referenced this issue Jan 7, 2024
myd7349 added a commit to myd7349/brainflow that referenced this issue Jan 7, 2024
This is a known issue with DSPFilters:
vinniefalco/DSPFilters#29
and has been fixed in iir1 and iirj:
berndporr/iir1@379d697
berndporr/iirj#27
Andrey1994 pushed a commit to brainflow-dev/brainflow that referenced this issue Jan 7, 2024
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

4 participants