Add RBJ Biquad filter (SIMD DF‑I) with LPF/HPF/BPF/Notch/APF/Bell/Shelves; API matches SVF #143
Conversation
|
Biquad will be a nice addition! I'll do a closer review in a moment, but right off the bat I see that this needs a test file or an example file (since we don't have unit tests yet [coming soon], an example will suffice). An "example" might be better. We're really trying to have good "example coverage" for the code base.
This is probably ok in this particular case because it's a typo, but in the future, it's best to have PRs that don't rely on other PRs. |
| return String("Biquad") | ||
|
|
||
| fn reset(mut self): | ||
| """Reset internal state of the filter.""" |
There was a problem hiding this comment.
It might be good to say a bit more about this. Why might a user need to use this method?
There was a problem hiding this comment.
reset function matches SVF description... should both be improved in a separate PR?
There was a problem hiding this comment.
I think only Biquad should be improved right here in this PR and an issue should be opened to also improve SVF.
|
If this gets merged, dev should first catch up with main @spluta |
Remove notes about normalization
Removed redundant comments and improved readability of BiquadModes documentation. - Removed ticks from table - Matched SVF description
Remove note about gain unit
Clarify q value
mjsyts
left a comment
mjsyts
left a comment
There was a problem hiding this comment.
All comments addressed except reset function description
I was originally going to use this as a cascade for a 6-pole Butterworth lowpass as a decimation filter. Would that make a good example? |
|
This is looking good. I am going to be a bit slow on merging due to the start of teaching. In the meantime, I think a "Test" file would be showing lpf and hpf filter sweeps over white noise just like SVF. A great "example" would be an EQ with lowshelf, 3 bells, and highshelf with GUI. This is asking a lot, but these would be dope. Decimation would be cool too. Don't we usually use a 4th order filter for decimation? This is what is used in the Oversampling library. |
This would be awesome. It's important that whatever the Test / Example is, it can be used to "test" the code. So to some extent it needs to be pretty straight forward that in about 30 seconds anyone could validate that the code is doing what it says. |
| frequency: SIMD[DType.float64, self.num_chans], | ||
| q: SIMD[DType.float64, self.num_chans] | ||
| ) -> SIMD[DType.float64, self.num_chans]: | ||
| """Lowpass biquad""" |
There was a problem hiding this comment.
Each one of these (lpf, hpf, etc) is going to need "Args:" and "Return:" in the docstring.
| frequency: SIMD[DType.float64, self.num_chans], | ||
| q: SIMD[DType.float64, self.num_chans] | ||
| ) -> SIMD[DType.float64, self.num_chans]: | ||
| """Bandpass (constant‑skirt gain; q sets peak gain)""" |
There was a problem hiding this comment.
"constant-skirt gain" and "q sets peak gain" can be explained or there can be link to where a user can learn what it means. Or if this information is trivial, then it should just be removed.
The issue is that a user might come to this and see "constant-skirt gain" and think, "hmm do I want that? let me find the 'non'-constant-skirt gain to compare. oh there isn't one?..." and now the user is off on some confusing sidequest rather than making art. So if details like this are important to include, it's good for them to not be dangling pointers.
| frequency: SIMD[DType.float64, self.num_chans], | ||
| q: SIMD[DType.float64, self.num_chans] | ||
| ) -> SIMD[DType.float64, self.num_chans]: | ||
| """Bandpass (constant‑peak gain = 0 dB; not a peaking EQ)""" |
There was a problem hiding this comment.
I wonder if this should not be called "peak" since the docstring says "not a peaking EQ". it is a confusing fn.
| q: SIMD[DType.float64, self.num_chans], | ||
| gain_db: SIMD[DType.float64, self.num_chans] | ||
| ) -> SIMD[DType.float64, self.num_chans]: | ||
| """Peaking EQ""" |
There was a problem hiding this comment.
See "fn peak" above. Now this one is called "bell" but is explained as "peaking". it will look like a typo to users.
Almost certainly, if you're using something like Chowdhury's implementation. When I was doing VCV development I swapped from what I think was internally a 4-pole LPF to a bespoke 6-pole Butterworth, which is just 3 cascaded biquad lowpass filters with the right coefficients. I'll do the examples. It'll help me understand what's going on.
So it'll be a good way to work through those things. |
3 participants
This PR adds a SIMD DF‑I Biquad filter with the common RBJ modes:
LPF, HPF, Notch, Allpass
Bandpass (CSG / constant‑skirt gain) and Peak (CPG / constant‑peak gain = 0 dB)
Bell (peaking EQ with gain_db)
Low/High‑Shelf using the current alpha‑based RBJ formulas (twoSqrtA = 2·√A·α)
All coefficients are normalized by a0; the hot path assumes a0 == 1. SIMD throughout; only the output sample is sanitized. API and naming mirror the SVF convenience methods for consistency.
Math sources: RBJ “Audio EQ Cookbook” (W3C/WebAudio adaptation) and RBJ notes on the shelf α‑form.
Also includes minor spelling/typo corrections carried forward from the previous PR (coeficients -> coefficients). (No behavior changes.)