Fix synthio_biquad_filter_reset() clearing only half the filter state - #11278
Merged
Conversation
biquad_filter_state is four int32_t (x[2], y[2]), but the reset only memset the first 8 bytes -- x[0] and x[1] -- leaving y[0] and y[1], the feedback history, untouched. A "reset" filter keeps ringing on whatever it last saw, so a low-pass fed silence after loud audio plays a decaying tail of the previous signal instead of silence. Fixes adafruit#11268
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11268.
shared-module/synthio/Biquad.hdefines the filter state as fourint32_t:but
shared-module/synthio/Biquad.conly clears the first two:The struct is 16 bytes;
4 * sizeof(int16_t)is 8.x[0]andx[1]getcleared, but
y[0]andy[1]-- the feedback history -- do not.biquad_filter_sample()reads all four members asint32_t, so theint16_tsizing looks like a leftover from an earlier, narrower statelayout.
A biquad's
yhistory is what the recursion runs on, so what survives a"reset" is not a cosmetic detail: feeding a reset filter pure silence
produces a decaying tail of whatever it played before, and for a low-pass
with poles near the unit circle that tail starts close to full scale.
Both callers --
audiofilters_filter_reset_buffer()(called when anAudioOutstarts playback) andsynthio.Note's filter (re)initialisation-- want the full reset.
Repro
An 800 Hz low-pass fed a loud 200 Hz tone for four blocks, then
reset_buffer(), then silence. Peak of the first "silent" block, unixcoverage build of
main:Includes a regression test
(
tests/circuitpython/audiofilter_filter_reset_buffer.py) with the samescenario; it fails on current
mainand passes with the fix.