Skip to content

Desktop: clamp TX output at unity gain (resample overshoot overdrives the rig)#580

Open
patrickrb wants to merge 1 commit into
devfrom
optio/task-e62f5ce8-2d4c-44c2-a696-7482c2aa6099
Open

Desktop: clamp TX output at unity gain (resample overshoot overdrives the rig)#580
patrickrb wants to merge 1 commit into
devfrom
optio/task-e62f5ce8-2d4c-44c2-a696-7482c2aa6099

Conversation

@patrickrb

Copy link
Copy Markdown
Owner

Root cause

The desktop TX output stage (audio/output.rs::prepare) folded the mandatory full-scale clamp into the gain step and guarded it out at gain == 1.0:

if gain != 1.0 {
    for s in data.iter_mut() { *s = (*s * gain).clamp(-1.0, 1.0); }
}

The guard is inverted relative to need:

  • gain < 1.0 → the product (*s * gain) is already in range, so the clamp is a no-op.
  • gain == 1.0 (TX level 100%) → the clamp is the only thing taming post-resample overshoot — and that is exactly the branch that was skipped.

Why it's a real, reachable bug

FFT resampling (rubato FftFixedIn) of the full-scale GFSK waveform — which peaks at exactly ±1.0 (synth_gfsk) — interpolates a few samples slightly past full scale. Measured empirically with the real resample_buffer:

resample out max samples > 1.0
12k→48k 1.049 85 / 49152
12k→44.1k 1.009 56 / 43953
12k→96k 1.049 153 / 98304

The f32 output path hands samples to the driver verbatim (f32::from_sample is identity) with no saturation net — unlike the i16 cast, which saturates in Rust. So at unity gain those >1.0 samples reach the soundcard unclamped and overdrive the SSB rig's audio input past 0 dBFS → ALC overshoot / distortion / spurious emissions on the air.

Reachability: desktop almost always resamples on TX (hardware rarely offers 12 kHz natively), f32 output devices are the common case (WASAPI/CoreAudio/PulseAudio), and clamp_tx_gain allows exactly 1.0. The default DEFAULT_TX_GAIN = 0.9 clips (the product stays in range), which masks the bug — only a 100% TX-level setting exposes it.

Fix

The clamp is safety-critical and must not depend on gain. Extracted apply_gain_and_clip() and call it unconditionally. This mirrors the Android reference, which clamps unconditionally on the output cast (FT8TransmitSignal.float2Short / floatToInt16NoPad), kept deliberately separate from its applyVolume gain step. For in-range samples the result is byte-identical to the old gain < 1.0 path, so the common case is unchanged.

Testing

4 pure #[cfg(test)] cases added to output.rs:

  • unity-gain overshoot is clamped to ±1.0
  • gain scales then clamps (gain = 0.5)
  • in-range unity gain is identity (happy path unchanged)
  • end-to-end: resample a full-scale tone (asserting the resampler really overshoots), then verify the output stage clamps it at gain = 1.0

The two unity-gain cases fail on the pre-fix guarded code and pass after — verified by reverting only the guard. Ran locally against the real resample.rs via a #[path]-include harness (zig-cc linker; the full Tauri crate can't link webkit2gtk/dbus in a headless sandbox). The #[cfg(test)] tests run in desktop CI under cargo llvm-cov.

Risk

Minimal. One-line behavioral change (clamp now always runs); byte-identical output for every in-range sample at every gain, including the default 0.9. No DSP/decoder/interop impact — this only tightens the TX output to full scale, matching Android and the encoder's own ±1.0 range. No new dependencies.

Platforms

Desktop (Tauri) only. Android/iOS already clamp unconditionally.

🤖 Generated with Claude Code

…rdriving the rig

The desktop TX output stage folded the full-scale clamp into the gain step and
guarded it out whenever `gain == 1.0`:

    if gain != 1.0 {
        for s in data.iter_mut() { *s = (*s * gain).clamp(-1.0, 1.0); }
    }

The guard is inverted relative to need. When `gain < 1.0` the product is already
in range so the clamp is a no-op; at `gain == 1.0` (TX level 100%) the clamp is
the only thing taming post-resample overshoot — and that is exactly the branch
that was skipped.

FFT resampling (rubato `FftFixedIn`) of the full-scale GFSK waveform, which peaks
at exactly ±1.0, interpolates a few samples slightly past full scale — measured
≈1.049 for 12 kHz→48 kHz (85 of 49152 samples over 1.0). The f32 output path
hands samples to the driver verbatim (`f32::from_sample` is identity) with no
saturation net, unlike the i16 cast which saturates in Rust. So at unity gain
those >1.0 samples reach the soundcard unclamped and overdrive the SSB rig's
input past 0 dBFS → ALC overshoot / distortion / spurious emissions. Desktop
almost always resamples on TX (hardware rarely offers 12 kHz), and the default
`DEFAULT_TX_GAIN = 0.9` clips (masking the bug) — only a 100% TX level exposes
it.

Root cause: the clamp is safety-critical and must not depend on gain. Fix mirrors
Android, which clamps unconditionally on the output cast
(`FT8TransmitSignal.float2Short` / `floatToInt16NoPad`), kept separate from its
`applyVolume` gain step. Extracted `apply_gain_and_clip()` and call it
unconditionally; for in-range samples the result is byte-identical to the old
`gain < 1.0` path.

Tests: 4 pure `#[cfg(test)]` cases in output.rs — unity-gain overshoot is clamped,
gain scales-then-clamps, in-range unity gain is identity, and an end-to-end case
that resamples a full-scale tone (asserting the resampler really overshoots) then
verifies the output stage clamps it at gain 1.0. The two unity-gain cases fail on
the pre-fix guarded code, verified by reverting only the guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 29.59%. Comparing base (647b12e) to head (747c678).

Files with missing lines Patch % Lines
desktop/src-tauri/src/audio/output.rs 97.22% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #580      +/-   ##
============================================
+ Coverage     29.36%   29.59%   +0.23%     
  Complexity      197      197              
============================================
  Files           179      179              
  Lines         24068    24099      +31     
  Branches       3263     3263              
============================================
+ Hits           7067     7133      +66     
+ Misses        16780    16745      -35     
  Partials        221      221              
Flag Coverage Δ
desktop 61.91% <97.22%> (+1.07%) ⬆️
native 9.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
desktop/src-tauri/src/audio/output.rs 37.16% <97.22%> (+20.06%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a safety-critical bug in the desktop (Tauri) TX audio output path where full-scale clipping was accidentally skipped at gain == 1.0, allowing FFT-resampler overshoot to reach the audio driver unclamped and potentially overdrive the rig.

Changes:

  • Extracts an apply_gain_and_clip() helper and applies it unconditionally after resampling (including at unity gain).
  • Removes the previous if gain != 1.0 guard that prevented clamping at 100% TX level.
  • Adds unit tests covering clamping behavior at unity gain and basic end-to-end resample → clamp behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +268 to +273
// Guard the test itself: the resampler really does overshoot, so the
// clamp is doing real work (not silently passing on a no-overshoot input).
assert!(
data.iter().any(|&x| x.abs() > 1.0),
"expected resample overshoot past full scale"
);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants