Fix S24_LE/S24_BE full-scale constant (playback distortion, capture 6 dB low) - #559
Draft
diamondsea11 wants to merge 1 commit into
Draft
Fix S24_LE/S24_BE full-scale constant (playback distortion, capture 6 dB low)#559diamondsea11 wants to merge 1 commit into
diamondsea11 wants to merge 1 commit into
Conversation
Both capture and playback used 0x00FFFFFF (2^24-1) as the signed 24-bit full-scale reference. The correct value is 0x7FFFFF (2^23-1) -- 0x00FFFFFF is what you'd use for an unsigned 24-bit range, not a signed one. Capture: every sample decoded to half its true level (-6 dBFS), the input appeared quiet rather than distorted. Playback: worse than a level error. Any float above 0.5 (already past -6 dBFS, so reachable by ordinary signal levels) multiplied past the valid signed-24-bit range and wrapped to the opposite polarity in the low 24 bits -- harsh, signal-level-proportional high-frequency distortion, with no xruns and normal CPU load, which makes it an unpleasant one to chase by ear. The existing clamp to [-1, 1] was already present and correct; it just could not help while the scale constant put post-clamp full-scale outside the valid range. Only the SND_PCM_FORMAT_S24_LE/S24_BE paths (24 bits unpacked into a 4-byte container) are affected. The S24_3LE/S24_3BE (packed 3-byte) paths already reconstruct the sample left-justified into the full 32-bit range and were already using the correct scale; left untouched, and confirmed identical to upstream by diff. Extracted as a single, minimal change from the diamondsea11/pipedal fork (feature/multipath-v1) for independent review, per the maintainer's request in rerdavies#555 to evaluate fork changes one topic at a time. Verified against feature/multipath-v1: these four functions are now byte-for-byte identical to the deployed fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
diamondsea11
pushed a commit
to diamondsea11/pipedal
that referenced
this pull request
Aug 6, 2026
All four extracted branches are now open as drafts against rerdavies/pipedal: rerdavies#558 ALSA multichannel, rerdavies#559 S24_LE scaling, rerdavies#560 JUCE sidechain group fallback, rerdavies#561 LV2 category patching. All target main, which is what the branches were cut from; retargeting to dev is offered in each PR body. rerdavies#558 already existed as a non-draft and was converted, with its auto-populated commit-message body replaced by one written for review. Each PR body states the provenance Robin asked about in rerdavies#555 (not a developer, AI-implemented from descriptions) so he can calibrate review effort, and states verification honestly per PR -- only rerdavies#561 has actually been compiled. Co-Authored-By: Claude Opus 4.8 <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
2 participants
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.
You asked in #555: "S24_LE capture/playback scaling. If it's broken, I definitely need a fix for this please."
It is broken, and the playback side is worse than a level error.
Root cause
Both directions used
0x00FFFFFF(2^24 − 1) as the signed 24-bit full-scale reference. The correct value is0x7FFFFF(2^23 − 1) —0x00FFFFFFis the range of an unsigned 24-bit sample, not a signed one.Capture: every sample decoded to half its true level (−6 dBFS). The input just sounds quiet.
Playback: any float above 0.5 — already past −6 dBFS, so reachable by perfectly ordinary signal levels — multiplied past the valid signed-24-bit range and wrapped to the opposite polarity in the low 24 bits. The result is harsh, signal-level-proportional high-frequency distortion, with no xruns and normal CPU load, which makes it an unpleasant one to track down by ear.
The existing clamp to
[-1, 1]was already there and already correct. It simply could not help while the scale constant put post-clamp full-scale outside the representable range.Scope
Four functions:
CopyCaptureS24Le,CopyCaptureS24Be,CopyPlaybackS24Le,CopyPlaybackS24Be— i.e. only theSND_PCM_FORMAT_S24_LE/S24_BEpaths, where 24 bits sit unpacked in a 4-byte container.The packed 3-byte
S24_3LE/S24_3BEpaths are not touched and were confirmed byte-identical to upstream. They reconstruct the sample left-justified into the full 32-bit range — a different convention that correctly calls for a different scale constant.Comments explaining the reasoning are included at each site, since the two directions fail in visibly different ways.
Verification
Diffed byte-for-byte against the deployed fix in
feature/multipath-v1, where this has been running on hardware (RME Babyface Pro FS at 24-bit). I have not built this isolated branch itself — the change is four constants, but that's still a claim about a build I haven't run, so I'd rather say so.Provenance
As discussed in #555: I'm not a developer, and this is AI-implemented from my descriptions. Please review accordingly.
Targeting
mainsince that's what the branch is based on — happy to retarget todev.