fix(audio): eliminate macOS CoreAudio crackling#1098
Conversation
the coreaudio ring buffer had three latent bugs that produced clicks and dropouts: - the render callback held a pthread_mutex_t shared with the producer thread, which violates apple's realtime callback guidelines and causes priority inversion -> device underruns -> clicks. - write == read was always treated as empty, so a write that landed exactly on the read position silently wiped the entire ring (~136 ms of audio at 44.1 khz stereo). - doplay discarded the whole incoming chunk on overflow instead of writing what fit. replace the locked ring buffer with a lock-free spsc ring (atomic read and write indices, acquire/release ordering), reserve one frame so write == read unambiguously means empty, and write what fits on overflow. also use getnumoutputchannels() instead of duplicating the stereo/surround logic, and drop the unused maxbuffered local. a standalone reproducer under tools/coreaudio_crackle_repro shows the buggy logic loses 192000 frames over 32 fill/drain cycles while the patched logic preserves everything.
the standalone repro under tools/coreaudio_crackle_repro proved the ring-buffer bugs the previous commit fixes. it is preserved in the branch history for anyone who wants to re-run it; the merged patch should ship as just the audio backend change.
f2b3b6b to
fc0c4f0
Compare
|
Follow-up from the HDMI/internal/USB-C testing after this PR went live:
I also updated the PR description with the live GitHub permalink to the historical repro and removed the now-stale claim that partial writes are the right overflow behavior. |
|
Opened the companion Shipwright producer-side PR here: HarbourMasters/Shipwright#6594 That PR handles the app-side guard: the producer leaves a wake idle when the backend does not have room for the smallest next burst, so the audio engine is not advanced unless the generated buffer can be enqueued whole. This libultraship PR remains the backend-side CoreAudio ring/realtime-safety fix. |
|
FYI, not exactly related, but I just opened #1105 that is also solving some audio glitches but on linux. I'll make sure that your imrovements here are also applied to the SDL and the new PipeWire backend. Thanks for this! |
This PR fixes the libultraship side of macOS CoreAudio crackling by making the CoreAudio backend a realtime-safe, unambiguous producer/consumer ring.
Companion Shipwright producer fix: HarbourMasters/Shipwright#6594
The two PRs address different layers:
CoreAudio Realtime Constraints
Apple's AudioToolbox docs describe audio rendering as a realtime context where blocking can overload the Core Audio HAL (
AUAudioUnit.renderBlock). The AVAudioSourceNode render-block docs make the same rule explicit for device rendering: avoid blocking calls inside the render block (AVAudioSourceNode.init(renderBlock:)).Apple's archived Core Audio QA1715 is older and RemoteIO-focused, but it spells out the practical failure mode: render callbacks run on a realtime thread, must return within their time slice, and blocking or indeterminate-time work can cause clicks and skips. It specifically calls out synchronous I/O, allocation, Objective-C messaging, and APIs that can block as work to keep out of render logic (QA1715).
Root Causes Fixed Here
Realtime Callback Locking
The old CoreAudio render callback took a
pthread_mutex_tshared with the producer thread (pre-patch lock, unlock). A realtime audio callback must not block on a producer-owned mutex; if it stalls, the backend zero-fills the rest of the device request (pre-patch zero-fill), which is audible as a click or dropout.This PR replaces the mutex with an SPSC ring:
mRingBufferWritePosmRingBufferReadPosRing Wrap Ambiguity
The old ring treated
write == readas empty in every calculation (Buffered(), render callback availability,DoPlay()free space). A write ending exactly on the read position could wrapwriteback toread, making a full ring look empty and dropping the queued audio.This PR allocates one extra frame and keeps one frame reserved. Usable capacity stays 6000 frames, but
write == readnow always means empty.Producer Chunk Boundaries
DoPlay()now refuses an incoming chunk if the complete chunk does not fit. Copying only a prefix drops the tail of an already-generated audio-engine buffer and creates a PCM discontinuity. The companion Shipwright PR avoids this condition at the producer by leaving a producer wake idle when the backend does not have room for the smallest next burst, so the audio engine is not advanced unless the resulting buffer can be enqueued whole.Reproducer
The deterministic ring-buffer reproducer is preserved in branch history here:
https://github.com/Kenix3/libultraship/blob/a01c490565711f91212430764bba0d81ea9c0927/tools/coreaudio_crackle_repro/repro.cpp
It compares the old ring logic with the fixed ring logic:
The reproducer was removed from HEAD so this PR merges only the backend implementation change.
Changes
DoPlay()chunk.GetNumOutputChannels()instead of duplicating channel-count logic.Out Of Scope