fix(wasapi): left-justify samples in a container wider than they are - #1305
Open
ErwanLegrand wants to merge 1 commit into
Open
fix(wasapi): left-justify samples in a container wider than they are#1305ErwanLegrand wants to merge 1 commit into
ErwanLegrand wants to merge 1 commit into
Conversation
A WAVEFORMATEXTENSIBLE describes a sample with two numbers: wBitsPerSample, the container, and wValidBitsPerSample, how much of it the sample fills. When they differ the valid bits sit at the *top* of the container -- "the valid bits (the actual PCM data) are left-aligned within the container", as ksmedia.h's WAVEFORMATEXTENSIBLE reference puts it. CPAL's I24 is the other way round: a dasp_sample::I24 is an i32 holding -(1 << 23)..=(1 << 23) - 1, at the bottom of its four-byte container. So the backend handed a 24-in-32 device every sample 2^8 too small and read every sample back 2^8 too large. It reaches both share modes: supported_formats reports I24 as supported, and the two fields go to the engine's converter in shared mode and to the driver in exclusive mode alike. Measured on a PreSonus AudioBox 22VSL (USB, Windows 11) at 24-in-32 / 48 kHz stereo, in both share modes and both directions, converting I24 to F32 on input and F32 to I24 on output. Output was 48 dB too quiet -- clean, just far below level -- and input pinned to full scale by anything above near-silence, moving only when the source was nearly silent. Instrumented on the capture side, a shared-mode I24 stream returned samples 2^8 too large, within 0.6 dB of an I32 reference on the same scale. Render shifts up in place, between the data callback and ReleaseBuffer: that buffer is the backend's to write until it is handed back. Capture cannot do the same, since GetBuffer lends WASAPI's own packet and lends it to be read, so the samples are shifted down into a staging buffer on their way to the callback -- sized once at stream build, so the callback still allocates nothing. It is a Vec<i32> rather than a Vec<u8> because that buffer reaches the callback as a Data, whose as_slice casts to the sample type, and a Vec<u8> guarantees no alignment. The shift is wBitsPerSample - wValidBitsPerSample read off the format handed to Initialize, not a test for I24, so every format whose container is exactly full -- I16, I32, F32 -- lands on zero by the same arithmetic and keeps its existing zero-copy path byte for byte. A container that is padded but not four bytes wide is refused rather than passed through unshifted, which would be silently wrong by the width of the padding. The integer half is a platform-neutral module so it can be tested off Windows: the round trip, the -2^23 and 2^23 - 1 boundaries, the sign carried down, padding bits discarded, the shift actually coming from the format, a trailing partial container left alone, and 16- and 32-bit formats passed through untouched.
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.
A WAVEFORMATEXTENSIBLE describes a sample with two numbers: wBitsPerSample, the container, and wValidBitsPerSample, how much of it the sample fills. When they differ the valid bits sit at the top of the container -- "the valid bits (the actual PCM data) are left-aligned within the container", as ksmedia.h's WAVEFORMATEXTENSIBLE reference puts it. CPAL's I24 is the other way round: a dasp_sample::I24 is an i32 holding -(1 << 23)..=(1 << 23) - 1, at the bottom of its four-byte container.
So the backend handed a 24-in-32 device every sample 2^8 too small and read every sample back 2^8 too large. It reaches both share modes: supported_formats reports I24 as supported, and the two fields go to the engine's converter in shared mode and to the driver in exclusive mode alike.
Measured on a PreSonus AudioBox 22VSL (USB, Windows 11) at 24-in-32 / 48 kHz stereo, in both share modes and both directions, converting I24 to F32 on input and F32 to I24 on output. Output was 48 dB too quiet -- clean, just far below level -- and input pinned to full scale by anything above near-silence, moving only when the source was nearly silent. Instrumented on the capture side, a shared-mode I24 stream returned samples 2^8 too large, within 0.6 dB of an I32 reference on the same scale.
Render shifts up in place, between the data callback and ReleaseBuffer: that buffer is the backend's to write until it is handed back. Capture cannot do the same, since GetBuffer lends WASAPI's own packet and lends it to be read, so the samples are shifted down into a staging buffer on their way to the callback -- sized once at stream build, so the callback still allocates nothing. It is a Vec rather than a Vec because that buffer reaches the callback as a Data, whose as_slice casts to the sample type, and a Vec guarantees no alignment.
The shift is wBitsPerSample - wValidBitsPerSample read off the format handed to Initialize, not a test for I24, so every format whose container is exactly full -- I16, I32, F32 -- lands on zero by the same arithmetic and keeps its existing zero-copy path byte for byte. A container that is padded but not four bytes wide is refused rather than passed through unshifted, which would be silently wrong by the width of the padding.
The integer half is a platform-neutral module so it can be tested off Windows: the round trip, the -2^23 and 2^23 - 1 boundaries, the sign carried down, padding bits discarded, the shift actually coming from the format, a trailing partial container left alone, and 16- and 32-bit formats passed through untouched.
This was made with the assistance of AI coding agents.