Fix Input Boost wrapping around instead of clipping#3834
Open
mcfnord wants to merge 1 commit into
Open
Conversation
The Input Boost setting (2x..10x) multiplied each sample and narrowed the int result back to int16_t with a plain cast, so any sample boosted past full scale wrapped to the opposite polarity rather than clipping. Use Float2Short, the same saturating helper already used twice in this function, so boosted peaks clip instead of inverting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Note
📡 STAND BY FOR AN LLM-AUTHORED MESSAGE.
Input Boost multiplies each sample and narrows the
intresult back toint16_twith a plain cast, so a boosted sample that passes full scale wraps to the opposite polarity instead of clipping.iInputBoostis 2..10 (the "2x".."10x" dropdown), so the product routinely exceedsint16_t. Wraparound is a full-amplitude sign flip, which is considerably more audible than the clipping a user would expect from a gain control.The invariant this breaks is written down 20 lines below
The pan block in the same function documents exactly when the plain cast is safe:
and the mono mixdown block, where sums can overload, says:
// note that we need the Float2Short for stereo pan modeInput Boost is the only gain > 1 in the send path, and the only one of the three that does not saturate.
Measured
1 kHz sine at −10 dBFS, 4800 samples. "Sign-flipped" means the current cast returned a value of opposite polarity to the true product.
The threshold is
32767 / iInputBoost. At 10x — the setting a user with a quiet microphone is most likely to reach for — anything above −20 dBFS inverts.Performance
This is a per-sample loop, so: x86-64, gcc 13.3,
-O3, 128-sample frame (2.67 ms), best of 7 runs × 3M reps, loops in a separate translation unit,memcpybaseline subtracted.Float2Short(this PR)std::min/std::maxclampThat is +1000 ns on a 2 670 000 ns budget, or +0.037%, and only on the path where the user has actually enabled the setting (
iInputBoost != 1; the default is 1, which skips the loop entirely). The ratio is large because the plain cast vectorizes topmullwand neither clamped form reachespackssdwon gcc 13 — but the absolute cost is not perceptible.I measured the integer clamp as the alternative and it comes out the same, so I chose
Float2Shortfor consistency with the rest of the function rather than for speed. Happy to switch if you prefer the integer form.Caveat: timings are x86-64 only; I have not measured ARM.
Side note, not fixed here
SignalLevelMeter.Updateruns after the boost, andCStereoSignalLevelMeter::Updatemeasures negative peaks only (a deliberate speed optimization, every third sample). A positive sample that wraps becomes a large negative and reads as a peak; a negative sample that wraps becomes positive and is missed. So the level meter is not a dependable warning for this, in either direction. That seemed like a separate discussion, so I left it alone.History
Introduced with the feature in #1222 (2021-04-01, e5bc010) and untouched since except by clang-format. I could not find any existing issue, PR, or discussion about it.
CHANGELOG: Bugfix: Input Boost now clips instead of wrapping around when the boosted signal exceeds full scale.