FIx: Use full-precision multiply for biquad feedback coefficients#950
FIx: Use full-precision multiply for biquad feedback coefficients#950rt-rtos wants to merge 1 commit into
Conversation
FILT_MUL_SS was SMULR6, which truncates both operands to 12 fractional bits before multiplying. For low-cutoff biquads the split-feedback pole corrections e = 2 + a1 and f = 1 - a2 are ~2^-10, so they retained only 2-3 significant bits and the pole landed almost randomly around its target: - sub-150 Hz HPF/BPF with Q >= 2 rang up ~+12 dB into sustained full-scale clipping, or lost its resonance entirely depending on where the quantized pole fell; - the sub-100 Hz LPF numerator (1-c)/2 ~ 3e-5 rounded to zero, producing silent output. Replace it, in the fixed-point build only, with SMUL64R: an exact 32x32->64 rounding multiply. Costs a couple of extra instructions per multiply in the biquad kernels. reduced comment verbosity
filters.c uses SMUL64R (direct 64x64->64) on desktop and ESP32S3. Incorporates and supersedes #950.
|
The |
|
No worries. I am glad you could claw back more performance than was lost for correctness. Appending my A/B bench. Delayed because it required new scenes.
Which code each filter reachesflowchart TD
G1["G1 LPF"] --> SFB["dsps_biquad_f32_ansi_split_fb"]
G2["G2 BPF"] --> SFB
G3["G3 HPF"] --> SFB
G4["G4 LPF24"] --> TW["split_fb_twice (BFP)"]
EQ["parametric EQ"] --> PEQ["parametric_eq_process"]
SFB --> FMS["FILT_MUL_SS"]
TW --> T16["top16SMUL + scan_max"]
PEQ --> TEQ["FILT_MUL_SS_EQ = top16SMUL"]
FMS --> B950["#950: SMULR6 to SMUL64R<br/>slower, more accurate"]
T16 --> B951["#951: whole BFP path replaced<br/>by nobfp kernel on SMUL64R"]
TEQ --> NC["untouched by both"]
B950 --> R1["+4.6 to +6.5%"]
B951 --> R2["-23.8%"]
NC --> R3["-0.12%, CRC same"]
Stale comments discovered in the process
Also dead, so irrelevant to both PRs despite using MeasurementsESP32-S3 240 MHz, fixed-point, 48 kHz, block 256, free pacing, 5 captures/side, #951 as merged (
|
| Scene | Path | A cyc | B cyc | delta | noise | headroom | output |
|---|---|---|---|---|---|---|---|
| juno6 | LPF24 | 1,150,097 | 1,043,114 | -9.30% | ±0.00% | 10.2% -> 18.5% | CHANGED |
| saw_lpf6 | LPF24 | 450,022 | 342,971 | -23.79% | ±0.00% | 64.9% -> 73.2% | CHANGED |
| filt_lpf60 | G1 | 320,411 | 341,352 | +6.54% | ±0.01% | 75.0% -> 73.4% | CHANGED |
| filt_bpf90 | G2 | 322,343 | 340,888 | +5.75% | ±0.02% | 74.8% -> 73.4% | CHANGED |
| filt_hpf65 | G3 | 326,180 | 341,210 | +4.61% | ±0.01% | 74.5% -> 73.4% | CHANGED |
| filt_eq | EQ | 348,365 | 347,952 | -0.12% | ±0.01% | 72.8% -> 72.8% | same |
juno6 headroom 10.2% -> 18.5% is the most consequential single number
here: it nearly doubles the margin on the tightest scene in the set.
#950 alone (2291ff8 -> 042b5be)
| Scene | delta | output |
|---|---|---|
| filt_lpf60 | +6.51% | CHANGED |
| filt_bpf90 | +5.54% | CHANGED |
| filt_hpf65 | +4.60% | CHANGED |
| filt_eq | +0.00% | same |
| saw_lpf6 | -0.00% | same |
| juno6, dx76, sine8, fx_sine8, idle | +0.00 to +0.24% | same |
The 12 dB regressions are identical across #950 and #951 (within 0.2 pp), which
confirms they come from the multiply swap alone and not from anything else in
#951. The LPF24 gain is entirely #951's
Conclusion:
- Huge
LPF24perf win - Minor
LPF,BPF,HPFperf loss. Improved correctness.
|
I don´t have both RPI Picos to test on device, but codegen seems like a win there too.
|
|
I tried it out on my Pimoroni 2 (RP2350) board. I expected it not to help, and I sort-of got that confirmed (shutdown with fewer notes under SMUL64) but I have no faith that the Arduino UI is actually recompiling when I want it to, or that it's using the optimization flags I requested. I should adapt the "make speedtest" target to run for my RP2350 setup, then I could measure for sure. |
|
From what I read, the RP2040 multiply is 32x32->32, but on the RP2350 it's 32x32->64, which would make all the difference. I must go deeper! |
2 participants
Use full-precision multiply for biquad feedback coefficients
Discovered while investigating the root cause of intermittent clipping caused by resonant
HPF in sub the 100hz range.
Symptom
An HPF with cutoff below ~100 Hz and Q > 1 produces massive sustained clipping,
worse at higher Q. Also reproducible on the LPF when a high-Q resonance overlaps
the note fundamental, though it's less audible there because the low cutoff
attenuates most of the band. The repro is erratic - some cutoff/Q combinations
ring up violently, others nearby lose their resonance instead.
Root cause
FILT_MUL_SSisSMULR6, which truncates both operands to 12 fractional bits(
>>11on s8.23) before multiplying.The split-feedback kernel
dsps_biquad_f32_ansi_split_fbexists precisely toprotect pole precision - it computes feedback as
y0 = w0 + 2*y1 - y2 - e*y1 + f*y2with the correctionse = 2 + a1andf = 1 - a2deliberately small. But small is exactly what 12-bit truncationdestroys. At fc = 65 Hz, Q = 4,
e ~ f ~ 0.0011, so after quantization theyretain only 2-3 significant bits.
So the mechanism meant to preserve the pole is the one that loses it. With the
corrections down to a couple of bits, the pole's frequency and damping land
quasi-randomly around their targets: overshoot and it rings up ~+12 dB into
sustained clipping - full-scale, since an HPF passes the whole spectrum -
overdamp and the resonance vanishes instead. Which side the rounding falls on
decides which you get, which is why the repro felt erratic.
Independently, the low-cutoff LPF numerator
(1-c)/2 ~ 3e-5quantizes tozero, which is why LPF below ~100 Hz goes silent.
Ruled out: block-floating-point state overflow (instrumented - zero
near-overflow events in all failing static-cutoff runs).
Evidence
Verified on a host-side replica of the fixed-point path: s8.23
SAMPLE,SMULR6, thesplit_fbkernel, andfilter_process's normbits BFP logic, alldriven by a retriggered 110 Hz saw at 0.4 FS and run against a double-precision
reference on the same coefficient stream. 4-second runs, static cutoff.
SMULR6)SMUL64R)With the exact multiply, every row matches the double-precision reference to 3
decimals and all clipping disappears. Above ~200 Hz the old path was already
fine, which is consistent with the diagnosis -
eandfonly get small at lowcutoff.
The one case that still clips
The double-precision reference clips
identically at the same peak. That's legitimate resonance gain from a 6-octave
sweep on a Q=8 resonator.
Recreating it
The harness is a single self-contained C file replicating the fixed-point path
(no AMY build required):
Happy to attach the harness as a gist or paste it in-thread if that's useful for
independent verification - it's ~190 lines.
The fix
In the fixed-point build only,
FILT_MUL_SSbecomesSMUL64R, an exact32x32->64 rounding multiply:
The float build keeps
SMULR6untouched.Cost is roughly 2 extra Xtensa instructions per multiply across the biquad
kernels. Note this includes the parametric EQ path, which was a deliberate
SMULR6choice for CPU reasons - if EQ cost measurably regresses on target, theEQ path can be specialized back to
SMULR6, since its fixed mid-bandcoefficients are not precision-sensitive. Happy to do that pre-emptively if
preferred.
Hardware-verified on ESP32-S3: the original repro is gone and
resonant low-frequency filters sound better subjectively.
Related, not addressed here
dsps_biquad_gen_hpf_f32/_bpf_lack thew0floor (~70 Hz) and Q floor(0.51) that
_lpf_has. With the precise multiply they're stable down to the4.8 Hz
LOWEST_RATIOclamp, so those floors are now a voicing question ratherthan a stability one.