search

sfstoolbox / sfs-matlab

SFS Toolbox for Matlab/Octave
https://sfs-matlab.readthedocs.io
MIT License
96 stars 39 forks source link

Fix phase offset of Linkwitz-Riley Allpass Filters and add testing function #185

Closed fietew closed 5 years ago

fietew commented 5 years ago

For some filter orders (even, but not divisible by four), there was a wrong sign for LR Allpass Filters. This PR fix this and adds a testing function.

fs446 commented 5 years ago

bug fix seems ok to me, just corrected the labeling of axis in the test script.

hagenw commented 5 years ago

The test runs fine o my machine. Is this the desired output: Fig1 fig1 Fig 2 fig2 Fig 3 fig3

fietew commented 5 years ago

Is this the desired output:

It seems, there is a problem regarding the Octave output. The greens lines in Fig1 and Fig3 (upper left) should be horizontal. I will have a look.

fietew commented 5 years ago

The behaviour is, again, related to a bug in zp2sos in Octave (see https://savannah.gnu.org/bugs/?51936), where single or non-existing zeros are handled incorrect. This was already mentioned in #169 . I used a similar workaround for this.

hagenw commented 5 years ago

Looks good now, here are the new figures I'm getting by running the test: Fig 1 fig1 Fig 2 fig2 Fig 3 fig3

hagenw commented 5 years ago

Is it ok to squash the commits during merge?

Alternatively I would propose to condense them to two, one for the fix and one for adding testing functions.