synthio: Add a form of biquad filter that uses BlockInputs

[jepler]

... for frequency & q_factor. Not tested on HW but it looks good in a manual host computer test. Here, a sine wave is played 4 times:

first, unfiltered. Then with 3 frequency-swept filters: low-, high- and band-pass. The band pass filter has a much higher Q-factor than the other filters.

This needs to be separately wired into the new audio filtering code

[relic-se]

Looking great so far, but I have a few quick notes:

• I assume you're using the property name synthio.BlockBiquad.kind to avoid confusion with the global, type. Have you considered using synthio.BlockBiquad.mode with the enum synthio.FilterMode instead?
• Looking over the calculations within common_hal_synthio_block_biquad_tick, I don't immediately see any reason why synthio.BlockBiquad.kind needs to be read-only. It may be nice to be able to switch between filter types without having to construct a new object. x and y of biquad_filter_state may need to be reset when the filter type is changed.
• Within common_hal_synthio_block_biquad_tick, it may be worth it to test the values f0 and Q against the previous tick's values to see if the coefficients don't need to recalculated.

Since you're keeping backwards compatibility with the original synthio.Biquad, we can keep audioeffects.Filter unchanged for now and wait after this PR is merged to make it compatible. But if you want to go ahead and tackle it, the common_hal_synthio_block_biquad_tick function from these lines would need to be added to this area.

[jepler]

I considered caching f0 & q values but decided against it until we had evidence these calculations were a problem.

Naming suggestions are always appreciated, thanks.

I could make the type/kind assignable and see what happens. I mentioned in the docs being unsure about the situation with the x[] and y[] coefficients when changing the F & Q numbers, but I'm even more dubious that those numbers would make any sense when changing the filter type, since that's like the hugest step change possible .. I'd leave it for a future addition if it DOES turn out to work or even sorta-work.

[jepler]

i see you mention resetting x[] and y[] but .. we don't have access to them when setting the property, because they're associated with a note. You can have the same filter on multiple notes, and each note has its own x[] and y[].

[jepler]

@todbot ping in case of interest

[jepler]

I took your suggestion to rename FilterType to FilterMode. I also added the notch filter from https://webaudio.github.io/Audio-EQ-Cookbook/audio-eq-cookbook.html.

shelf & peaking don't seem to be useful until it's possible to have multiple filters (and/or their application is beyond my knowledge); they also pose a problem in that they have an additional "A" parameter related to overall gain. Presumably all BlockBiquads would have an "A" or "gain" property that would be ignored if it is not pertinent. And I still don't get what an all-pass filter is for.

[jepler]

Shall I mark the original Biquad APIs deprecated? (only one of the 3 Synthesizer functions show, but all would be deprecated)

diff --git a/shared-bindings/synthio/Biquad.c b/shared-bindings/synthio/Biquad.c
index 79a9fa4593..2323551a91 100644
--- a/shared-bindings/synthio/Biquad.c
+++ b/shared-bindings/synthio/Biquad.c
@@ -38,7 +38,11 @@ static const mp_arg_t biquad_properties[] = {
 
 //| class Biquad:
 //|     def __init__(self, b0: float, b1: float, b2: float, a1: float, a2: float) -> None:
-//|         """Construct a normalized biquad filter object.
+//|         """Construct a normalized biquad filter object. **DEPRECATED**
+//|
+//|         The Biquad class is deprecated and will be removed in a future release
+//|         of CircuitPython (but not before CircuitPython 10); use `BlockBiquad`
+//|         instead.
 //|
 //|         This implements the "direct form 1" biquad filter, where each coefficient
 //|         has been pre-divided by a0.
diff --git a/shared-bindings/synthio/Synthesizer.c b/shared-bindings/synthio/Synthesizer.c
index 87ea44de69..da81d4f18c 100644
--- a/shared-bindings/synthio/Synthesizer.c
+++ b/shared-bindings/synthio/Synthesizer.c
@@ -288,7 +288,9 @@ MP_PROPERTY_GETTER(synthio_synthesizer_blocks_obj,
 //|
 
 //|     def low_pass_filter(cls, frequency: float, q_factor: float = 0.7071067811865475) -> Biquad:
-//|         """Construct a low-pass filter with the given parameters.
+//|         """Construct a low-pass filter with the given parameters. **DEPRECATED**
+//|
+//|         Instead, construct a `BlockBiquad` directly.
 //|
 //|         ``frequency``, called f0 in the cookbook, is the corner frequency in Hz
 //|         of the filter.

[relic-se]

shelf & peaking don't seem to be useful until it's possible to have multiple filters (and/or their application is beyond my knowledge)...

I'm planning to add support for multiple filters within audiofilters.Filter in the near future to make it possible to build EQ-like filters. I'm good with not working about those filter modes until that is ready.

Shall I mark the original Biquad APIs deprecated? (only one of the 3 Synthesizer functions shown, but all would be deprecated)

Definitely not a question for me to answer. Do any other moderators (ie: @tannewt or @dhalbert) have thoughts on this? The result of this would also affect this draft PR: #9754. (no point in added a feature while it is already being deprecated)

Yet again, do you think there are performance benefits to using the original Biquad rather than the BlockBiquad if there is no expected change in filter settings? Lower performance hardware may benefit from the older class.

[jepler]

When you're working on allowing multiple filters, please see whether you can structure it so that synthio can share the code. e.g., if a structure more complicated than an list is needed / has to be traversed when filtering some samples, do it as a function both uses can call. Or, at least, make your best guess as to how that would need to work.

[relic-se]

When you're working on allowing multiple filters, please see whether you can structure it so that synthio can share the code. e.g., if a structure more complicated than an list is needed / has to be traversed when filtering some samples, do it as a function both uses can call. Or, at least, make your best guess as to how that would need to work.

@jepler Here's the PR for multiple filters within audiofilters.Filter: #9772. Currently, it's not much more complicated that traversing an array of biquad_filter_state elements. I couldn't think of anything in particular that would need to be shared with synthio at this time.

[jepler]

@dcooperdalrymple @todbot can either of you do a review of this PR on the technical side? I don't think you can mark it "approved" but your comments would be helpful.

[jepler]

It would not hurt to do something smarter when frequency value is inappropriate. @kevinjwalters noted that with the current biquad implementation, using a frequency value above f_s/2 gives "white noise like" outputs; I think it'd probably be better to have silence (or maybe no filter for notch & lpf?) in this case.

[relic-se]

Tests so far are promising. On an RP2040, I've got 12 notes running at once, each with a BlockBiquad with LFO on frequency and q_factor. It sounds stable @ 48kHz. I've tested it up to 72kHz successfully, but it stutters at 88.2kHz+. I would assume that faster processors (with FPU) like the RP2350 would be able to handle 96kHz. I haven't tested that myself yet.

import audiobusio
import board
import synthio

audio = audiobusio.I2SOut(bit_clock=board.GP0, word_select=board.GP1, data=board.GP2)
synth = synthio.Synthesizer(sample_rate=48000, channel_count=2)
audio.play(synth)

for i in range(12):
    synth.press(synthio.Note(
        frequency=synthio.midi_to_hz(48 + i * 3),
        amplitude=0.1,
        filter=synthio.BlockBiquad(
            mode=synthio.FilterMode.LOW_PASS,
            frequency=synthio.LFO(
                scale=4000,
                offset=4500,
                rate=0.1 + i * 0.05,
            ),
            Q=synthio.LFO(
                scale=0.4,
                offset=1.1,
                rate=0.025 + i * 0.0125,
            ),
        ),
        panning=i / 6 - 1.0
    ))

Notes:

• The BlockBiquad constructor lists q_factor in the docstring, but it's actually Q.
• I think a cached value of W0 and Q should be stored in either the biquad_filter_state or synthio_block_biquad_t struct. If the BlockInput value hasn't been modified since the last common_hal_synthio_block_biquad_tick call, it should avoid recalculating the biquad.

Everything else looks and sounds clean.

[jepler]

Thanks @relic-se particularly for noting the problem with Q vs q_factor. The implementation in synthesizer was similarly confused, so I updated the docs there too.

I added some code that should avoid recalculations when neither W0 (derived from f0 and sample rate) nor Q have changed. Nothing smart is done if only one or the other factor changes.

[relic-se]

I added some code that should avoid recalculations when neither W0 (derived from f0 and sample rate) nor Q have changed. Nothing smart is done if only one or the other factor changes.

Your implementation is great. I think this update is mostly beneficial for static BlockBiquad objects in which neither factor changes to avoid a lot of unnecessary computation. Of course, a standard Biquad could be used instead, but we're marking that as deprecated within this PR, so it's best not to encourage its usage.

[gamblor21]

I did not have time to test on hardware but the code looks good. I had 3 minor comments.

[gamblor21]

Changes are good. Looks good to me. Thanks!