fix(dsp): the notch analysed the daemon's ticks and re-primed its filter every block (#1303) - #1318
Merged
Merged
Conversation
…ter every block (#1303) Three defects at the same seam, measured rather than reasoned about. DETECTION ran on whatever block arrived. The daemon is the only surface that enables the notch, and it delivers ~400-sample ticks, so the detector lost 10*log10(4096/400) of processing gain and ran `inner`/`floor_halfwidth_hz` — both meaningful only at the 4096-sample resolution they were fitted at — in a regime where the protected band spans ~9 resolution cells. It now analyses a rolling fft_size buffer while the filter still acts on the block as given: the filter must act on what it is handed, the detector must not. That was the error in my own design's prior-art field, and it hid the fix. THE FILTER was re-primed to block[0] on every call. At Q=25 a notch settles in ~29 samples, so measured suppression was 11.9 dB at 400-sample blocks against >200 dB with state carried — a perfectly detected interferer reached the demodulator ~12 dB down. The detected bin jitters between adjacent bins, so coefficients are retuned in place and only genuinely new notches are primed. A detection within `min_spacing_hz` of a live notch is the same interferer by this module's own definition, so that existing constant decides reuse. A SHORT BLOCK was windowed by the first `take` taps of a full-length Hann — a rising quarter-sine, ~24 dB low. Warm-up only now, but still wrong. What the review overturned: "the wrong window masks interferers" is false at the amplitudes that matter (today detects a strong tone at 400 samples every block), and scaling `inner` by the lobe ratio would have made false notches 14x worse while gaining nothing. Gated fast, because REQ-QRM-01's own suite is held out of the gate for runtime and so checks none of this per-PR. My first proposed test was vacuous: frequency error is within a bin at every block size today, because zero-padding interpolates. Implements: REQ-QRM-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
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.
Closes #1303. Three defects at the same seam, measured rather than reasoned about.
1. Detection ran on whatever block arrived
The notch sits at the single
route_audio_stage(InputCapture)seam, and the daemon is the only production surface that enables it (ModemEngine::newdefaults it off; the config defaults it on andserver.rs:184-187applies that — no CLI, ARDOP or KISS caller). The daemon delivers ~400-sample ticks.Analysing those directly costs
10·log10(fft_size/take)— about 10 dB of processing gain at 400 — and putsinnerandfloor_halfwidth_hz, which are meaningful at the 4096-sample resolution they were fitted at, into a regime where the protected band spans ~9 resolution cells.Detection now runs on a rolling
fft_sizebuffer while the filter still acts on the block as given. The filter must act on what it is handed; the detector must not. That distinction was the error in my own design's prior-art field — I wrote "a notch must act on the block it is given" — and it is what hid the fix.2. The filter was re-primed every block — the larger defect
process_blockrebuilt and re-primed every biquad on each call. At Q=25 a notch settles in ~29 samples, so measured suppression of a pure tone was:A perfectly detected interferer reached the demodulator ~12 dB down — and
input_preroutedmeans that is what it got. The detected bin jitters between adjacent bins on a steady tone (2199.22 / 2201.17 Hz), so coefficients are now retuned in place and only genuinely new notches are primed. A detection withinmin_spacing_hzof a live notch is the same interferer by this module's own definition of distinctness, so that existing constant decides reuse — no new constant was introduced.This is the twin I had marked UNCHECKED in the design. It is bigger than the defect I filed.
3. A short block was windowed by a prefix of a longer Hann
The first 400 taps of a 4096-point Hann run 0 → 0.091: a rising quarter-sine, ~24 dB low. Warm-up only now, but a short block must still be analysed correctly rather than through a ramp.
What the review overturned in my framing
innerby the lobe ratio would have made it 14× worse. The arithmetic was right — measured lobe 41/21/83 bins at take 400/800/200 against predicted 41.0/20.5/81.9 — but scalinginnerto 61 shrinks the median ring and yields 0.70 false notches/block against 0.05, for no gain. Inside a zero-padded main lobe the magnitude is monotone, so the ±6-bin local-max test accepts only the true peak anyway.chunks(4096). That line is theprefilterarm (NotchMode::Fixed, no detection). The detection arms feed an unpaced loopback that drains the whole buffer in one read, so the seam analyses the first 4096 samples of one ~200k block.Gates — fast, deliberately
REQ-QRM-01's own suite is
#[ignore]d for runtime and held out ofscripts/gate.sh(#1274), so before this it checked none of these properties per-PR.detection_is_the_same_whether_audio_arrives_in_ticks_or_one_blocka_notch_suppresses_a_tone_across_a_stream_of_small_blocks—fixed_notch_kills_its_tonecannot see this: it measuresout[1024..]of one long block and skips the transient entirelya_short_block_is_windowed_by_a_hann_of_its_own_length— against Hann coherent-gain theory (peak ≈ A·take/4), not a recorded numberMy originally proposed test was vacuous: frequency error is within a bin at every block size today, because zero-padding interpolates.
Sabotage-verified with clean attribution — priming every block fails only the suppression gate; the prefix window fails only the warm-up gate; detecting on the raw block fails only the equivalence gate.
Both evidence tiers
GATE: PASS 3c4503a41504bff775b16797d3c1e1548e42a6f8 clean— 328 suites, 2514 passed, 0 failed.scripts/slow-tests.sh notch— SLOW-TESTS: PASS, 3 passed, 0 failed, 2055 s. This one was not optional: the rolling buffer changes which 4096 samples that suite analyses (first → last of its whole-buffer read), so its rescue result could not be assumed to survive. A green gate does not cover it.Evidence provenance, stated plainly
The numbers driving the design choices above were measured by the design review using a Python port of
spectrum_db/peaks_from_spectrum/ the RBJ biquad, run against the realic9700-idle-hot.wavcorpus. That port was not asserted equivalent to the Rust. I verified the mechanisms in the source and sabotage-verified each fix against its own gate; the gates assert properties (equivalence, theory, suppression) rather than any of those numbers, and the slow suite passing is independent. But the figures quoted here are that port's, not the shipped code's.Consumer
ModemEngine::apply_rx_notchatroute_audio_stage(PipelineStage::InputCapture)(engine.rs:7044-7051), reached by every receive path — but enabled only byopenpulse-daemon.Prior art
#1254 is the same archetype in the sibling consumer at the same seam: a fixture block size no production path produces, making an acceptance gate pass. Its resolution (accumulate across calls) is available here after all, once detection and filtering are separated — which is exactly what my design said was impossible.
Twins
#1254 enumerated the other consumers of this buffer:
apply_dc_block(guardsn == 0),apply_rx_agc(per-sample),DcdState(guards empty),NoiseFloorTracker(fixed there). The notch was the last with a length-sensitive assumption. Filed separately: #1316 —band_filledreturns true on 45 % of idle blocks at every block size, because the shaped floor alone fills the protected band.Implements: REQ-QRM-01
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6