Repository navigation
perf(acars): envelope squelch — 2.8% CPU live, down from 8% - #182
Conversation
|
Warning Review limit reached
Next review available in: 8 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces an ACARS-specific envelope-based squelch gate to skip the expensive demodulation/deframing work while a channel is idle, aiming to significantly reduce steady-state CPU usage without changing decode output.
Changes:
- Add a per-channel envelope squelch (
Squelch) with pre-roll, hysteresis, hangover, and a caller-controlled “hold open” override. - Integrate the squelch into
MskDemodand add a deframer “collecting” interlock to prevent mid-frame truncation across chunks. - Add end-to-end tests plus two examples (
sensitivity,cpu_budget) to validate “no sensitivity loss” and quantify CPU stage costs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/xng-mode-acars/src/squelch.rs | New envelope squelch implementation (gate decisions + pre-roll buffering + tests). |
| crates/xng-mode-acars/src/demod.rs | Wire squelch into MskDemod so downstream stages run only on gated samples (with pre-roll replay). |
| crates/xng-mode-acars/src/lib.rs | Add public APIs to pin squelch open; hold-open interlock when deframer is mid-block (single + multi channel). |
| crates/xng-mode-acars/src/frame.rs | Expose Deframer::is_collecting() to support the demod hold-open interlock. |
| crates/xng-mode-acars/tests/end_to_end.rs | Add end-to-end tests covering long silence, back-to-back bursts, long gaps, and gated-vs-ungated equivalence. |
| crates/xng-mode-acars/examples/sensitivity.rs | New sensitivity A/B harness that compares gated vs pinned-open through the real decoder. |
| crates/xng-mode-acars/examples/cpu_budget.rs | New CPU attribution example to measure per-stage costs and squelch impact under idle vs busy captures. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
kevinelliott
left a comment
kevinelliott
left a comment
There was a problem hiding this comment.
Review: the design and the verification work are excellent; four detection-path defects should be fixed first
I reviewed this against a build of the branch and reproduced everything below end-to-end through the real AcarsChannelDecoder rather than reasoning from the diff. Where I say "measured", I ran it.
First, credit where it's due, because it materially shaped this review: the PR's own honesty about its verification is unusual and correct. The "the sensitivity harness could not see the gate at all" note, the deaf-mutant check, the "60 trials was too few" retraction, and the explicit "nothing detects PREROLL's removal" flag are all accurate — I re-derived each. I reproduced the quoted sensitivity table exactly on this machine: 393/361/295/175/20 ungated vs 396/360/293/176/22 gated. Committing the harnesses rather than deleting them is the right call.
The four issues below are all in the detection path, and they share one root cause: the gate's notion of "something is present" is a ratio of carrier-inclusive envelope power against a floor that can move the wrong way. The gating architecture itself — pre-roll, hysteresis, hangover, deframer interlock — I checked carefully and it is sound.
Findings (all CONFIRMED with reproductions; details inline)
| # | Severity | Issue |
|---|---|---|
| 1 | Blocker | Detection on carrier-inclusive p goes deaf under a DC/co-channel pedestal the demod strips anyway — drops bursts pinned-open decodes 3/3 |
| 2 | Major | Cold start with signal already on air latches the gate shut for that signal's duration |
| 3 | Major | The floor freeze is self-defeating — it learns up from any signal lasting ≳4 s |
| 4 | Major | floor == 0.0 is absorbing: gate pinned open forever, entire CPU win silently lost |
Finding 1 is the one I'd call blocking: a channel at capture centre (AcarsChannelDecoder::new(rate, 0.0), which the tests use throughout) is exactly where LO leakage lands, and a pedestal only ~9.5 dB above the burst takes decode from 3/3 to 0/3.
Two smaller ones
5. MINOR — interlock ordering and PREROLL sizing. lib.rs:82 and lib.rs:141 set the hold after the chunk is processed, so the interlock can only reopen the gate one chunk late — it can never prevent a mid-chunk close. And PREROLL (512 samples = 21 ms) is smaller than one chunk in the shipping config (READ_CHUNK 65 536 / 2.4 MS/s = 27.3 ms = 655 channel samples), so on the one path where pre-roll would be load-bearing, the replay can't cover the gap. Relatedly, HANGOVER's doc assumes "the ~27 ms chunk", which only holds at 2.4 MS/s — src/main.rs:45 documents -r 24000 as valid for ACARS, where the chunk is 2.73 s, i.e. 27× the hangover, making the interlock inert. That chunk-duration ≤ HANGOVER requirement is real and currently undocumented.
6. MINOR (not a regression) — NaN/Inf permanently latches the gate shut. One NaN sample poisons MskDemod::level (a plain EMA, never reset, demod.rs:140), so every later level > floor * RATIO is false. Measured: after one NaN, 0 open samples over the next 10 s including a 5 s full-scale burst; after one +Inf, exactly one hangover's worth then dead. The seed at line 189 is likewise unguarded. I confirmed the pre-squelch pipeline is already destroyed by a NaN (pinned-open also decodes 0/8), so this isn't new — but the squelch adds a second independent permanent-latch path, and cf32 is a supported input (src/main.rs:257). A p.is_finite() guard is ~free.
On PREROLL (agreeing with the PR)
I tried to break the "nothing detects its removal" claim and couldn't. With PREROLL = 0 all 41 tests pass, and a sweep designed to expose it — inter-burst gaps of 90/95/100/105/110/115/120/130/150/200 ms, i.e. reopening after a short closed run, at both 0.5 and 0.08 amplitude — gives 240/240 both ways. The structural reasoning in the comment is right. Given finding 5, though, 512 is the one value that's neither useful nor honest: either raise it above the maximum chunk length or drop it.
Harness assessment
sensitivity.rs is methodologically sound — TRIALS = 400, LEAD = 48_000 (2 s), correct pairing on the same seed per arm, and hold_squelch_open(!gated) genuinely selects pre-squelch vs squelch. Its blind spot is that it tests exactly one shape: a single burst after a long clean-noise lead-in with a correctly seeded floor. That's why it cannot detect any of findings 1-4, each of which needs a cold start, a pedestal, or sustained signal. Two concrete additions would have caught them: a nonzero-DC arm, and a lead-in that starts mid-signal. Minor: the auto-flag is per-column (gated + margin < ungated, margin 8 = 2%) and doesn't implement the "consistent direction across the middle three" criterion the prose describes.
cpu_budget.rs's idle/busy × gated/pinned matrix is an honest design, and the busy row really is measuring an open gate (I measured 91.3% duty on a back-to-back capture, stable at 91.5% over 6 reps, all 306 frames decoding). But it never asserts that anything still decodes in the busy arm — so a change that made the gate go deaf under sustained traffic would show up there as an improvement.
Verified sound (so this isn't re-litigated)
- Pre-roll ring: doubled-buffer indexing correct at and across wraparound;
newest - ncan't underflow (newest ≥ RING-1,n ≤ PREROLL = RING-1); order is oldest-first and exactly thensamples preceding the decided one;closed_runcounts only dropped samples and saturates atPREROLL, so no sample is handed to the demod twice. For any close shorter thanPREROLLthe replayed stream is bit-for-bit contiguous — no discontinuity for the FIR, NCO or timing loop. Block length is irrelevant (the ring is per-sample). Confirmed with a 300 k-sample randomised run asserting index order. - Threshold math:
OPEN_RATIO1.6 >CLOSE_RATIO1.2 with no path that crosses them; no division instep();p ≥ 0sofloor ≥ 0; the seed divisor is incremented before use, never zero. The ~5% EMA sd claim checks out (a/(2-a)on exponentialp), so 1.6× really is ~12σ on noise. - The central claim — gate more sensitive than demod — holds. For an AM burst
level/floor = 1 + SNR, so the gate opens at −2.2 dB SNR against a demod cliff of ~+3 dB: a ~5 dB margin, and the level EMA crosses threshold ~3 ms into the 53 ms pre-key even at marginal σ. - The interlock cannot latch open forever:
frame.rs:39MAX_CHARS = 250forcesState::Huntwithin ~0.83 s, and bits keep flowing while held open, sois_collectingalways clears. Bounded leak, not permanent. - Metrics are unaffected by gating:
level,dcandnoiseare all computed before the gate (demod.rs:140-159) and run on every sample, solevel_dbfs,noise_dbfsandsnr_db(lib.rs:279) stay correct, and the DC used forvalueis always current. - No stale-state transient on reopen: FIR delay is 121 taps <
PREROLL;Ncophase andFirstate advance only per processed sample, which is the right semantics;acc/prev_disc/prev_bitstaleness costs at most one bit at a splice, inside the pre-key. Nothing needs zero-feeding. - Per-channel state is clean: one
SquelchperMskDemodperChannelBack; no statics, no shared mutable state, no interior mutability. (No retune path exists insrc/runtime.rs— decoders are built once inModeChannel::new— so reset-on-retune isn't reachable today.) holdcomposition is correct:lib.rsORs the user hold with the per-chunk interlock in both back ends, so the interlock can't clear a user-requested disable.if self.mixed.is_empty() { return; }(demod.rs:174) correctly skips the NCO advance, consistent with dropping the samples.- Full suite green (41 tests), and green again with
PREROLL = 0.
Recommendation
Fix 1 and 4 before merge (both are small: detect on DC-blocked power; make floor recovery additive or clamp it off zero). 2 and 3 are the same root cause and worth fixing together. Add the nonzero-DC and mid-signal-start arms to sensitivity.rs so the harness can actually see this class of bug. 5 and 6 are cheap and can ride along.
Also worth stating plainly in the description: the "2.8% live" number is measured with #181 underneath, so this PR can't land or be evaluated independently of it.
Generated by Claude Code
| /// Detection deliberately runs on the *carrier* power `p`, not on the | ||
| /// DC-blocked `value`: the DC blocker exists to strip the carrier and | ||
| /// leave the audio, which throws away the very thing that most cleanly | ||
| /// distinguishes a burst from silence. | ||
| pub(crate) fn step(&mut self, p: f32, level: f32, value: f32) -> Gate { |
There was a problem hiding this comment.
Blocker — detecting on carrier-inclusive power blinds the gate to signals the demod decodes fine.
This comment argues the case for testing p rather than value, and the argument is sound for noise — but it doesn't consider a DC / co-channel carrier pedestal. The demod deliberately strips exactly that (value = env - self.dc, demod.rs:160), so a steady LO leak or co-channel carrier inflates level and floor equally, the ratio test goes deaf, and the burst is dropped — even though the DC blocker downstream would have removed the interferer completely.
Because it's a pure ratio, it scales down to trivially small DC values against weak bursts. Measured end-to-end through the real AcarsChannelDecoder (3 bursts, 2 s lead-in, noise 0.01):
| burst amp | DC offset | dB re burst | gated | pinned open |
|---|---|---|---|---|
| 0.5 | 1.2 | +7.6 | 3/3 | 3/3 |
| 0.5 | 1.5 | +9.5 | 0/3 | 3/3 |
| 0.1 | 0.3 | +9.5 | 0/3 | 3/3 |
| 0.05 | 0.15 | +9.5 | 0/3 | 3/3 |
| 0.05 | 0.2 | +12.0 | 0/3 | 3/3 |
Pinned-open is the pre-squelch pipeline by construction, so this is a regression rather than a pre-existing limit. Note AcarsChannelDecoder::new(rate, 0.0) — a channel at capture centre, which the tests use throughout — is precisely where LO leakage lands.
Suggest detecting on a DC-blocked power (or subtracting the same dc estimate the demod already maintains) so the gate's notion of "present" matches the demod's.
Generated by Claude Code
| if self.seeded < SEED { | ||
| // Still measuring the floor: hold open, and accumulate the mean. | ||
| self.seeded += 1; | ||
| self.floor += (p - self.floor) / self.seeded as f32; | ||
| self.open = true; | ||
| self.hangover = HANGOVER; | ||
| return Gate::Open; | ||
| } |
There was a problem hiding this comment.
Major — a cold start with signal already on air latches the gate shut for the duration of that signal.
The seed is the mean of the first 256 samples of whatever is on air. If a signal is present during those samples, floor ≈ level, so idle (line 201) is true immediately, the floor keeps EMA-ing onto the signal and never falls, and the gate closes SEED + HANGOVER = 2 656 samples (110.7 ms) in — then can never reopen while that signal persists.
All four documented margins fail at once here: the pre-key detection window is already spent (the gate started open), pre-roll is irrelevant, hangover only buys the 100 ms, and the deframer interlock never engages because sync hasn't arrived yet.
Repro — 40 ms of carrier already on air before an otherwise normal burst:
lead 0 ms: gated 1 pinned 1 duty 25.9% (Collect at 85.3 ms → interlock saves it)
lead 40 ms: gated 0 pinned 1 duty 8.9% (closed at 110.7 ms, no OPENING event ever)
lead 160 ms: gated 0 pinned 1 duty 8.1%
The gate-transition trace for the failing case has exactly two events: open @ sample 1, closed @ sample 2657, then nothing.
To be fair on exposure: ordinary ACARS cold starts survive, because sync lands at ~85 ms < 110.7 ms. I confirmed cold start straight into back-to-back traffic (90/90 gated vs 90/90 pinned) and a cold start on a 4-block ETB-chained transmission (4/4 vs 4/4) both pass. It needs signal on air ≳25 ms before the frame's sync — a co-channel carrier, an SDR start-up transient, or joining a transmission already in progress at process start.
A seed that takes a low percentile (or a minimum-of-windows) rather than a mean would be robust to this.
Generated by Claude Code
| // The floor is what the channel reads when nothing is transmitting, so | ||
| // it must not be allowed to learn from a transmission. Freezing it on | ||
| // "not idle" rather than on a fixed spike threshold matters for weak | ||
| // bursts specifically: a burst only a few dB up would otherwise sit | ||
| // below any sane spike gate and drag the floor towards itself, closing | ||
| // the squelch part-way through its own signal. | ||
| let idle = level < self.floor * CLOSE_RATIO; |
There was a problem hiding this comment.
Major — the freeze is self-defeating: the floor does learn up from a sustained signal, which is the exact thing this comment says it prevents.
FLOOR_RECOVER (line 207) runs on every not-idle sample: 2e-5 × 24 kHz = +2.08 dB/s. So for a signal R above the floor, floor * CLOSE_RATIO overtakes level after ln(R/1.2) / 0.48 seconds — 4.4 s at 10 dB — at which point idle flips true and the floor EMAs straight onto the transmission.
Measured with a 10 dB carrier: gate open 100% of the time at 1 s and at 4 s, but 75% over 6 s and 23% over 20 s — it closes at ~4.5 s and stays closed.
Decode impact in the shapes I tested is small (recovery once the signal stops is ~50 ms, alpha = 0.002 → τ = 21 ms; carrier-then-burst at 1/3/6/15 s each decoded 1/1). So this is not a blocker on ACARS traffic shapes. But the invariant claimed here doesn't hold beyond a few seconds, and it's the same underlying mechanism as the two findings above — worth either fixing or restating the comment to match reality (e.g. "must not learn from a transmission shorter than ~4 s").
Generated by Claude Code
| self.floor += FLOOR_ALPHA * (p - self.floor); | ||
| } | ||
| } else { | ||
| self.floor *= 1.0 + FLOOR_RECOVER; |
There was a problem hiding this comment.
Major — floor == 0.0 is an absorbing state, and it silently disables the whole optimisation.
Recovery is multiplicative, so 0.0 * 1.00002 == 0.0.
If the first SEED samples are exactly zero — a muted source, a zero-padded IQ file, a driver's first buffer — then floor = 0. After that, idle = level < 0.0 (line 201) is false for every sample since p ≥ 0, so the EMA branch at line 204 is unreachable forever, and level > 0.0 * OPEN_RATIO is true for every sample → the gate is pinned open permanently with no path back.
Measured duty on 10 s of pure noise: 100.00% with 1024 leading exact zeros, vs 1.11% with noise present from sample 0. Completely silent — no error, no symptom, and the entire CPU win is gone.
An additive floor (+= FLOOR_RECOVER * (something) or a floor.max(TINY) clamp after the seed) fixes it.
Related and worth knowing: several existing tests build silence as exact zeros with no noise (e.g. tests/end_to_end.rs:33 decodes_at_channel_rate), so they run at 100% gate duty and never exercise the squelch at all — the same class of blindness the module doc warns about at lines 79-84.
Generated by Claude Code
Independent verification of the test claims — exact matchBuilt and ran this branch in an isolated worktree off
All 10 new tests were observed passing by name — 6 unit tests in Both new examples compile: No new clippy lints. The workspace
Context for the green check on this PR
To be clear about what this verifies and what it doesn't: the suite is genuinely green and the counts are exactly as stated. The four detection-path defects in my review above are all invisible to it — they need a nonzero-DC arm, a mid-signal cold start, or leading exact zeros, none of which the current tests construct. Generated by Claude Code |
… set ACARS channels are idle 87-99% of the time (a 50-character message is ~254 ms of RF; 30 messages/minute on one channel is 12.7% busy), yet the demod ran its 121-tap audio lowpass, NCO, discriminator, timing loop and the deframer's both-polarity sync hunt on every sample forever, almost all of it on noise. Those stages are ~1.3% of one core across 16 channels. The AM envelope and its EMAs -- already computed for RSSI, and enough to tell whether anything is there -- are ~0.13%. Gate the former on the latter. Measured, 16 ch at 2.4 MS/s, same capture with the gate active vs pinned open (pinned open is the pre-squelch pipeline): all channels idle 1.35% vs 2.83% -52% all channels busy 2.50% vs 2.80% -11% The busy row is every channel transmitting back to back, which no real band does; it bounds the worst case. The gate is never slower than pinned open, so it costs nothing when it cannot help. Four independent margins stop it truncating a burst: a burst opens with ~53 ms of all-ones pre-key and the gate opens ~1.3 ms into it; opening replays a pre-roll; closing needs a lower level than opening did plus a 100 ms hangover; and the deframer holds the gate open outright while mid-block. Sensitivity, 400 trials per sigma, same capture, gate the only variable: ungated 393 / 361 / 295 / 175 / 20 gated 396 / 360 / 293 / 176 / 22 Under 1% either way with no direction to it -- no loss. The squelch is deliberately per-mode and private to this crate. VDL2 acquires on a coherent unique-word correlation that an envelope gate in front of it would only damage, AIS already has a power gate, and STD-C's NCS channels are continuous carriers where a gate that ever closed would drop carrier lock. It must not be lifted into the shared front end. Two notes on the verification, because both nearly went wrong: - The sensitivity harness could not see the gate at all. Its 400-sample lead-in was shorter than the gate's floor seed plus hangover, so the gate was already open when the burst arrived: a deliberately deaf gate (open threshold at 12x the floor, which decodes nothing in the field) scored an unchanged pass through it. The lead-in is now 2 s, and the harness compares gated against pinned-open on one capture rather than one row against a remembered row. - 60 trials was too few to conclude anything. Varying only the noise seed, the sigma = 0.18 column ranges 51-58 of 60 -- the harness's own scatter was the size of the effects it exists to detect, which is also why the previously quoted 60/57/42/23/4 was never a meaningful target. TRIALS is now 400 (~14 s) and the flag threshold scales with it. The pre-roll is kept as insurance and documented as such: nothing in the suite detects its removal, including with the pre-key truncated to 4 of its 128 bits, because the pre-key already supplies the lead-in it re-supplies.
…to see it Review found four defects, all in the detection path rather than in the gating architecture, plus a harness that could not observe any of them. Every fix below is reproduced end-to-end through the real `AcarsChannelDecoder`, and each is covered by something that fails without it. Blocker: detect on DC-blocked power, not carrier-inclusive power ---------------------------------------------------------------- The gate tested raw envelope power on the reasoning that the carrier is what most cleanly separates a burst from silence. That holds for thermal noise and fails for everything else: a DC offset, LO leakage, or a co-channel carrier lifts level and floor together, the ratio test goes deaf, and the burst is dropped — while the demod's own DC blocker removes the interferer completely and decodes it fine with the gate pinned open. A regression, not a pre-existing limit, and `AcarsChannelDecoder::new(rate, 0.0)` puts the channel at the capture centre, which is exactly where LO leakage lands. Detecting on the DC-blocked signal — the same quantity the demod mixes — costs nothing. It is ~3.6 dB *more* sensitive, because blocking DC removes more from the silence floor than from the burst. Pedestal 9.5 dB above the burst, three bursts: 0/3 before, 3/3 after, at every amplitude tried. This also fixes the cold-start-with-signal-on-air case for the same reason: a steady carrier no longer inflates the floor. Carrier already up for 0/40/160/500/2000 ms before the burst now decodes 1/1 at every lead, matching pinned open. A floor of zero is no longer absorbing -------------------------------------- Recovery was multiplicative (`floor *= 1 + eps`), which cannot escape zero. A source starting with digital silence — muted input, zero-padded IQ file, a driver's first buffer — seeded the floor to 0, made `idle` false forever, and pinned the gate open permanently: no error, no dropped frame, the entire CPU saving silently gone. Measured 100.00% gate duty against 1.11% expected. Two changes. The floor now tracks the level additively at one of two rates (fast while idle, ~200x slower otherwise) so it can always escape; and the seed ignores samples with no energy, because digital silence is not a measurement of a noise floor. Leading zeros now cost 1.5% duty against 1.11%, from 100%. The freeze is honest about its limits ------------------------------------- The old comment claimed the floor never learns from a transmission. It does, eventually — the creep is slow, not zero, so a *continuous* carrier wins after roughly seven seconds. Out of reach for ACARS (longest legal block ~0.83 s, and the interlock holds the gate open across a block), but it is exactly why this module must not be reused for STD-C, whose NCS channels are continuous carriers. Documented rather than papered over. Non-finite input no longer latches the gate ------------------------------------------- `level` is an EMA that is never reset, so one NaN made every later comparison false and the channel went deaf for the life of the process. cf32 is a supported input format, so a malformed file reaches this. Interlock granularity no longer depends on the caller ----------------------------------------------------- The interlock can only be evaluated between demod calls, so the caller's chunk size bounded how late it could react. At 2.4 MS/s that is 655 channel samples, fine against a 2 400-sample hangover — but at `-r 24000` a read chunk is 2.73 s, 27x the hangover, leaving the interlock inert. The back end now walks its input in fixed 1 024-sample blocks, so the guarantee holds at any rate. `AcarsChannelDecoder` now holds a `ChannelBack` rather than repeating its fields, so the interlock exists once. It was duplicated at two call sites, and the review found the same defect in both. PREROLL raised 512 -> 2048 -------------------------- Still insurance, still documented as untested — but 512 was smaller than one channel-rate chunk, so on the one path where pre-roll would be load-bearing the replay could not cover the gap. That made it the one size that was neither useful nor honest. Harness: three channel shapes, not one -------------------------------------- `sensitivity.rs` tested a single burst after a clean lead-in, which is why it saw none of the above. It now runs the paired gated/pinned-open comparison over `clean`, `dc pedestal` and `mid-signal start`. The pedestal arm is sized deliberately. For a detector squaring raw envelope, level/floor is ((dc + a)/dc)^2 — 1.78 at 9.5 dB, which is still above the open threshold, so an arm at that level would have passed against the very defect it exists to catch. At 15.6 dB it is 1.36, and the arm fails: reverting detection to carrier-inclusive power now scores 0/0/0/0/0 gated against 394/382/343/274/111 pinned open, and the harness prints GATE COST in: dc pedestal. `cpu_budget.rs` now counts frames on the busy capture and says so. A gate that went deaf under sustained traffic would otherwise have shown up there as the fastest configuration measured. 85 frames both gated and pinned open. Result ------ Idle 1.43% against 2.77% pinned open (-48%); busy 2.41% against 2.78% (-13%), decoding the same 85 frames. Sensitivity shows no gate cost in any of the three scenarios. 1246 tests passing, 0 failing.
91c2eb4 to
0a7486a
Compare
ACARS channels are idle 87–99% of the time — a 50-character message is ~254 ms
of RF, and even 30 messages/minute on one channel is only 12.7% busy. The demod
nonetheless ran its 121-tap audio lowpass, NCO, discriminator, timing loop and
the deframer's both-polarity sync hunt on every sample forever, almost all of it
on noise.
Those stages cost ~1.3% of one core across 16 channels. The AM envelope and its
EMAs — already computed for RSSI, and enough to tell whether anything is there —
cost ~0.13%. This gates the former on the latter.
Real-world result
Measured on the slow host, live off-air, against the same channel set used
for an acarsdec comparison on the same antenna and host:
~9× off the starting point and ~2.9× off the 8% this campaign began quoting.
That also serves as the real-hardware validation of #181, which had only been
measured on a dev box.
We are at 1.87× acarsdec rather than at parity, so the campaign is not finished
— but the remaining gap is now smaller than the measurement noise used to be.
See "What this does not fix" below for where the rest most likely lives.
Bench result
Dev box, 16 channels at 2.4 MS/s, same capture with the gate active versus
pinned open. Pinned open is the pre-squelch pipeline, so this is a controlled
A/B rather than a comparison against a remembered number:
The busy row decodes 85 frames either way, so it is a like-for-like saving
rather than a gate that has gone quiet under load — the harness now asserts
this, because a deaf gate would otherwise appear as the fastest configuration
measured.
"All channels busy" is every channel transmitting back to back, which no real
band does; it bounds the worst case. The gate is never slower than pinned open,
so it costs nothing when it cannot help.
Sensitivity: no loss
400 trials per sigma, same capture, gate the only variable:
Three channel shapes, 400 trials each, gate the only variable:
No gate cost in any scenario.
How it avoids truncating a burst
Four independent margins, in the order they apply:
at 2400 bd (~53 ms) before any frame content; the level EMA crosses the open
threshold ~1.3 ms into it.
100 ms more below it.
outright, so even a signal that genuinely collapses is followed to the end.
Squelch is per-mode, and deliberately private
mod squelchis private toxng-mode-acars. "Is this channel busy" meanssomething different for every waveform:
xng-mode-ais/src/coherent.rs) — do not add a secondWhen the shared front end is fanned out to other modes, the front end
generalises and the presence test does not.
Two notes on the verification, because both nearly went wrong
The sensitivity harness could not see the gate at all. Its 400-sample
lead-in was shorter than the gate's floor seed plus hangover, so the gate was
already open when the burst arrived. A deliberately deaf gate — open threshold
raised to 12× the floor, which decodes nothing whatsoever in the field — scored
an unchanged clean pass through it. The lead-in is now 2 s, and the harness
compares gated against pinned-open on one capture. It scores that same mutant
0 / 0 / 0 / 0 / 0.60 trials was too few to conclude anything. Varying only the noise seed, the
σ = 0.18 column ranges 51–58 of 60 — the harness's own run-to-run scatter was the
same size as the effects it exists to detect. This is also why the previously
quoted
60 / 57 / 42 / 23 / 4was never a meaningful target to compare laterruns against.
TRIALSis now 400 (~14 s) and the flag threshold scales with it.Both harnesses are committed (first commit) rather than written-run-and-deleted,
which is what happened in #178/#181 and left shipped comments pointing at tools
that no longer existed.
On the pre-roll
Kept, and documented as insurance rather than as verified-necessary: nothing
in the suite detects its removal. Setting
PREROLLto 0 leaves every testpassing and the sensitivity A/B unchanged, including in the case it exists for —
the pre-key truncated to 4 of its 128 bits. The reason is structural: the gate
opens ~1.3 ms into a ~53 ms pre-key, so the demod already receives ~51 ms of
constant carrier as lead-in. It is retained because it costs ~512 samples per
burst and synthetic bursts cannot represent a real capture whose pre-key is lost
to a collision or fade. Flagged in the code so nobody later mistakes it for
tested.
Tests
1243 passing, 0 failing (
cargo test --workspace --no-default-features), +10over the base. New coverage: burst after long silence, back-to-back bursts,
bursts separated by long gaps, and gate-on/gate-off frame-set equivalence at low
amplitude. Behavioural mutants (
HANGOVER = 0, deaf threshold, floor learningfrom bursts) are each caught by the unit tests.
What this does not fix
The dev-box decode core predicts ~1.35%, but the slow host measures 2.8%, so
roughly half the live cost is outside anything benchmarked here. The prime
suspect is the ingest path, which has never been profiled: USB transfer,
CU8→CF32 conversion at 2.4 MS/s, per-chunk allocation, and the dashboard's
512-point spectrum FFT — which
src/runtime.rsruns on every 4th chunkunconditionally, whether or not a dashboard subscriber exists. acarsdec is
not computing a spectrum for a web UI, so that is not a like-for-like cost.
That is the next thing to measure, and it is deliberately not in this PR.
Review round 2
All four detection-path findings fixed, each reproduced end-to-end first and
each now covered by something that fails without the fix:
sensitivity.rsdc pedestalarmsensitivity.rsmid-signal startarma_full_length_block_does_not_pull_the_floor_up_onto_itselffloor == 0.0absorbing — gate pinned open forever, 100% dutya_zero_floor_is_not_absorbingPlus: NaN/Inf no longer latches the gate; the deframer interlock now runs on a
fixed 1024-sample timebase instead of inheriting the caller's chunk (it was
inert at
-r 24000, where a read chunk is 2.73 s against a 100 ms hangover);AcarsChannelDecodernow reusesChannelBackso the interlock exists oncerather than duplicated at two sites; and
PREROLLis 2048, above onechannel-rate chunk — 512 was the one size that was neither useful nor honest.
The harness arm is sized to actually fail. For a detector squaring raw
envelope, level/floor is
((dc + a)/dc)^2, which is 1.78 at 9.5 dB — stillabove the open threshold, so an arm at the level originally measured would have
passed against the very defect it exists to catch. At 15.6 dB it is 1.36, and
reverting detection to carrier-inclusive power now scores
0/0/0/0/0gatedagainst
394/382/343/274/111pinned open.1246 tests passing, 0 failing.