fix(posix): drop oldest queued buffer instead of failing when waveOutWrite ring nears full #2115

Merged

Cloudiny merged 1 commit from posix/waveout-ring-guard into main

2026-11-12 03:03:57 +00:00

Cloudiny commented

2026-11-11 03:36:39 +00:00

(Migrated from github.com)

Summary

In host/posix/win32_posix.c, waveOutWrite() still returns MMSYSERR_ERROR once w->count == 64. During shader compilation or heavy frame stalls on Linux, pending audio packets can fill the 64-entry ring and cause subsequent waveOutWrite calls to fail or drop new buffers without recovering.

When w->count >= 60, this change retires the oldest queued WAVEHDR (WHDR_INQUEUE cleared, WHDR_DONE published via __atomic_store_n(..., __ATOMIC_RELEASE)), advances w->head, decrements w->count, and resets w->pos = 0 so newest audio stays live without stalling callers.

NVIDIA & Cross-Platform Compatibility

host/posix/win32_posix.c only compiles on POSIX/Linux hosts; Windows winmm is unaffected.

Files Changed

host/posix/win32_posix.c

Test Plan / Verification

Verified clean git apply --check against main (ca082bf).
Syntax-checked host/posix/win32_posix.c (clang -fsyntax-only -std=gnu11 -Wall -Wextra -Wno-macro-redefined -Wno-unused-parameter): 0 errors, 0 warnings.

## Summary In `host/posix/win32_posix.c`, `waveOutWrite()` still returns `MMSYSERR_ERROR` once `w->count == 64`. During shader compilation or heavy frame stalls on Linux, pending audio packets can fill the 64-entry ring and cause subsequent `waveOutWrite` calls to fail or drop new buffers without recovering. When `w->count >= 60`, this change retires the oldest queued `WAVEHDR` (`WHDR_INQUEUE` cleared, `WHDR_DONE` published via `__atomic_store_n(..., __ATOMIC_RELEASE)`), advances `w->head`, decrements `w->count`, and resets `w->pos = 0` so newest audio stays live without stalling callers. ## NVIDIA & Cross-Platform Compatibility - `host/posix/win32_posix.c` only compiles on POSIX/Linux hosts; Windows `winmm` is unaffected. ## Files Changed - `host/posix/win32_posix.c` ## Test Plan / Verification - [x] Verified clean `git apply --check` against `main` (`ca082bf`). - [x] Syntax-checked `host/posix/win32_posix.c` (`clang -fsyntax-only -std=gnu11 -Wall -Wextra -Wno-macro-redefined -Wno-unused-parameter`): 0 errors, 0 warnings.

hckrman101 (Migrated from github.com) reviewed

2026-11-11 18:24:21 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks for looking at this. The shim change itself is sound, but I don't think it fixes what the description says, and it has no test.

What I checked on main:

The only caller of waveOutWrite is host/nn_audio.c. It has NBLOCKS 10 headers and only reuses one after it reads WHDR_DONE on it ("the device is behind: this frame is dropped"). So at most 10 headers can ever be in the shim's ring. It never reaches 60, let alone 64, and the old MMSYSERR_ERROR path and this new branch are both unreachable from the game. Stalls during shader compilation are already handled in the caller: it drops the frame, and after 1.5 s of no completion it runs output_reopen() (#2190, merged). If you saw a real failure, which log line or waveOutWrite error did you get? That would tell us what is actually happening.
On the points I was asked to check, the code is fine: the dropped header gets WHDR_INQUEUE cleared and WHDR_DONE set under the ring mutex, so nothing can wait on it forever. pos is reset under the same lock as wave_pull, so the ring is thread-safe. The drop could click if the oldest buffer was half-played, but it would only happen in an overflow that can't occur here.
The behaviour differs from Windows, where waveOutWrite just queues and never fails. Dropping at >= 60 while the ring still has 4 free slots is an arbitrary threshold, and it keeps about 60 blocks of latency instead of catching up.
The description says the check was count == 64; the patch changes the capacity to 60, which should be said.
No test. If you keep a change here, a small C test that fills the ring and checks the dropped header's flags would go with it (see tests/audio_output_errors_test.c, tests/audio_wave_buffer_release_test.c).
CI only ran the title check, so nothing exercised this.

Unless there is a path I missed that submits more than 10 headers, I'd suggest closing this or reworking it around the real symptom. #1544 (Aux buffers) is unrelated to this ring.

Thanks for looking at this. The shim change itself is sound, but I don't think it fixes what the description says, and it has no test. What I checked on main: - The only caller of `waveOutWrite` is `host/nn_audio.c`. It has `NBLOCKS 10` headers and only reuses one after it reads `WHDR_DONE` on it ("the device is behind: this frame is dropped"). So at most 10 headers can ever be in the shim's ring. It never reaches 60, let alone 64, and the old `MMSYSERR_ERROR` path and this new branch are both unreachable from the game. Stalls during shader compilation are already handled in the caller: it drops the frame, and after 1.5 s of no completion it runs `output_reopen()` (#2190, merged). If you saw a real failure, which log line or `waveOutWrite` error did you get? That would tell us what is actually happening. - On the points I was asked to check, the code is fine: the dropped header gets `WHDR_INQUEUE` cleared and `WHDR_DONE` set under the ring mutex, so nothing can wait on it forever. `pos` is reset under the same lock as `wave_pull`, so the ring is thread-safe. The drop could click if the oldest buffer was half-played, but it would only happen in an overflow that can't occur here. - The behaviour differs from Windows, where `waveOutWrite` just queues and never fails. Dropping at `>= 60` while the ring still has 4 free slots is an arbitrary threshold, and it keeps about 60 blocks of latency instead of catching up. - The description says the check was `count == 64`; the patch changes the capacity to 60, which should be said. - No test. If you keep a change here, a small C test that fills the ring and checks the dropped header's flags would go with it (see `tests/audio_output_errors_test.c`, `tests/audio_wave_buffer_release_test.c`). - CI only ran the title check, so nothing exercised this. Unless there is a path I missed that submits more than 10 headers, I'd suggest closing this or reworking it around the real symptom. #1544 (Aux buffers) is unrelated to this ring.

Joey-Magnus (Migrated from github.com) reviewed

2026-11-11 21:24:36 +00:00

Joey-Magnus (Migrated from github.com) left a comment

I don't think the new branch can run. The only caller, host/nn_audio.c, has NBLOCKS 10 headers (:738) and only resubmits one after WHDR_DONE (:922), so count never exceeds 10 and >= 60 is never reached. If it did run, it would silently drop queued audio, whereas today a full ring returns an error the caller handles (:955-959). There's no use-after-free, since everything is under w->mu. Suggest closing unless there's a caller I've missed.

I don't think the new branch can run. The only caller, `host/nn_audio.c`, has `NBLOCKS 10` headers (`:738`) and only resubmits one after `WHDR_DONE` (`:922`), so `count` never exceeds 10 and `>= 60` is never reached. If it did run, it would silently drop queued audio, whereas today a full ring returns an error the caller handles (`:955-959`). There's no use-after-free, since everything is under `w->mu`. Suggest closing unless there's a caller I've missed.