freedv/HARQ: gate combined-decode acceptance on parity count, not CRC16 alone

[spinkham]

The HARQ Chase fallback re-decodes the averaged LLRs and then accepts the
result on freedv_check_crc16_unpacked() alone; parityCheckCount is computed
but only printed in the verbose path. A combine that has NOT converged still
emits a codeword, and that codeword still has a ~2^-16 chance of passing CRC16
by itself. On the single-shot path that stray pass is just a rare undetected
bit error. On the combine path it is reached on purpose at the fade cliff --
roughly half the combines fail to converge there by design -- so the roll is
taken far more often than 2^-16-per-frame suggests. Today burst_frames==1
everywhere bounds the blast radius (a same-codeword combine, one DATA frame
per session), but the moment burst_frames>1 lets the RX admit more than one
DATA frame per session, a CRC-passing non-converged combine parses in-window
and is delivered as new data instead of being retransmitted -- silent
corruption, not a retry.

Fix: require the LDPC parity-check count to clear a wide margin (>= 90% of
NumberParityBits, via the new ldpc_harq_combine_parity_ok()) in ADDITION to
CRC16 before adopting a combined decode. A converged decode satisfies
essentially all of its mother-code parity checks; a non-converged one sits
near the ~50% binomial floor, so 90% cleanly separates the two populations.
Deliberately not a strict all-checks gate: at the marginal combining point
residual errors can live in parity bits, so a genuine decode need not reach
100% and a strict gate would reject it. The single-shot decode is untouched,
so clean-channel goodput is unchanged; RX-only, no wire-format change.

Measured on Mercury's own H_256_768_22 (DATAC15) decoder + CRC16 with a
fixed-seed AWGN Chase model: 3,000,000 sub-threshold trials produced 47
CRC16-passing NON-converged decodes -- the silent forgeries CRC-only
acceptance would deliver as data (rate ~2^-16, as expected). ZERO of the 47
cleared the 90% parity gate (worst forgery 402/512, gate 460/512). Separately,
200,000 genuine near-threshold decodes ALL cleared the gate (min 507/512), so
it never false-rejects a real decode. The threshold sits cleanly between the
two populations: 402 (worst forgery) < 460 (gate) < 507 (worst genuine).
Corroborated by OpenARQ's reference implementation at the same decode point.

Test (tests/modem/test_freedv_harq.c) drives Mercury's own DATAC15 decoder
(run_ldpc_decoder on H_256_768_22) with a fixed-seed AWGN model and asserts
the separation: every genuine correct decode clears the gate (no false-reject),
a non-converged garbage decode never does (forgery blocked), and the threshold
is exact at the boundary. ldpc_harq_combine_parity_ok is the single source of
the threshold shared by the decoder and the test.

Complements d356833 (the LLR-averaging half of the same combining path):
d356833 fixed the combined LLR scale, this fixes its acceptance.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com

---

Opened via Claude Code (Opus 4.8) working with N4FPV; provenance in the commit trailer.

[spinkham]

Another option: Someone else pushed an independent fix for the same issue today: xmutantson@ffcb1f4

They way their fix is designed has a slightly tighter gate, which since there is no stream CRC in Mercury, it may be a better solution for you to adopt instead of this one. I'd recommend you evaluate both.

[rafael2k]

Reviewed + validated locally — approving.

The bug is real. In freedv_comp_short_rx_ofdm() the combined decode was accepted on freedv_check_crc16_unpacked() alone (parityCheckCount was computed but only used in the verbose print). At the fade cliff the combine path is entered on purpose and ~half the combines haven't converged, so the ~2⁻¹⁶ stray-CRC roll is taken far more often than per-frame odds suggest. Today burst_frames==1 bounds the blast radius, but it turns into silent corruption the moment burst_frames>1 admits >1 DATA frame/session — i.e. exactly when we resume multi-frame bursts.

The fix is sound and minimal: the 90% parity-count gate is applied only to the combine path (single-shot untouched → clean-channel goodput unchanged), it's additive to CRC16, and 90% cleanly separates converged (~all checks) from non-converged (~50% binomial floor) while staying below 100% so genuine-but-marginal combines aren't rejected. ldpc_harq_combine_parity_ok() as a shared inline (RX + test single source of truth) is nice.

Validation:

• test_freedv_harq builds and passes 3/3 (boundary; correct-decode-always-clears; non-converged-never-clears), driving the real DATAC15 H_256_768_22 decoder.
• Goodput-neutrality A/B — decoded identical CCIR-poor faded DATAC17 files (across the whole HARQ-active cliff) with gate-OFF (mercuryv2) vs gate-ON, both --harq. Delivered counts are identical at every SNR (18/38, 14/38, 12/37, 7/32, 6/31, 3/24, 0/10 — MATCH throughout), confirming the gate rejects zero genuine combined deliveries. (Channel: OpenARQ skywave/hfchan, noise axis calibrated to codec2 ch.)

Nice catch — forward-defense on precisely the path our roadmap lights up. Note: the parallel mfsk-arq-integration branch (#125) also carries this HARQ code, so it'll need to pick up this gate on its next rebase onto mercuryv2.