fix(pulseaudio): several correctness and accuracy fixes - #1115
Merged
Conversation
- Fill playback buffer with the correct silence byte per format (0x80 for U8, 0x00 for signed/float); use buf.fill() over a manual loop - Fix sign bug in store_latency: subtract write/read offsets as i64 before clamping to zero, avoiding wrapping on negative values at startup - Use source_usec instead of sink_usec for record-stream latency; sink_usec is 0 for a pure record stream, making capture timestamps wrong - Surface play_all() errors via error_callback instead of silently dropping them; share the callback with the worker thread via Arc<Mutex<>> - Reduce latency poll interval from 100 ms to 5 ms to keep timestamps within roughly one buffer period of true hardware latency - Remove unused return value from store_latency
Channel maps: replace the mono-for-all fallback with correct standard layouts for 1–8 channels (PA_CHANNEL_MAP_DEFAULT convention), 7.1.2 and 7.1.4 Dolby Atmos layouts for 10 and 12 channels, and sequential Aux positions for counts with no agreed-upon layout. Buffer attrs: split into separate playback and record functions. Playback uses double-buffering (target_length = 2N, minimum_request_length = N) so the server always requests exactly N frames per callback. Record now correctly uses fragment_size instead of the playback-only target_length field, which was silently ignored. Set the adjust_latency StreamFlag when BufferSize::Fixed is requested, instructing PulseAudio to configure hardware end-to-end to meet the latency target. Timestamps: switch from SystemTime to Instant (monotonic, immune to NTP adjustments). Add latency interpolation for playback streams between timing polls — store poll_usec before latency_micros to avoid a startup race where stale poll_usec=0 would zero out the interpolated latency.
Member
Author
|
I'm on a merge streak and would like to get this in so it gets more testing. |
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
1 participant
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 few issues I spotted in the PulseAudio backend after it landed. @colinmarc, would you mind casting an eye over these since you know the code best?
First commit contains hot-path and correctness fixes:
U8silence is0x80, not0x00store_latencyhad a sign bug: casting a negativewrite_offset - read_offsettou64wrapped aroundsink_usecinstead ofsource_usecplay_all()were silently droppedSecond commit improves channel maps, buffer configuration, and timestamps:
Mono; now implements standard PA layouts for 1–8 channels, 7.1.2 and 7.1.4 Dolby Atmos for 10 and 12 channels, and sequentialAuxpositions as a fallbackmake_buffer_attrsplit into separate playback and record functions: playback uses a double-buffer (target_length= 2N,minimum_request_length= N) to guarantee N-frame callbacks; record usesfragment_sizeinstead of the playback-onlytarget_lengthfieldadjust_latencyflag now set whenBufferSize::Fixedis requested, so PulseAudio configures the hardware end-to-end to hit the targetSystemTimereplaced withInstant(monotonic, not affected by NTP); playback timestamps now interpolate latency between polls rather than using a stale value for up to 5 ms