Report finished audio buffers at the renderer update, so a speaker no longer goes silent #1181

Merged
janvitos merged 1 commit from fix/audio-wave-buffer-release into main 2026-11-09 18:59:37 +00:00
janvitos commented 2026-11-09 16:32:51 +00:00 (Migrated from github.com)

What

A wave buffer the mixer finishes is now reported to the game at the next RequestUpdateAudioRenderer (commit()), as on the console, instead of the moment the mixer thread finishes it. Until that update, GetReleasedWaveBuffer does not return it and GetWaveBufferCount still counts it. Only host/nn_audio.c changes (a nrel_seen count per voice).

Why

In about one launch in five, one speaker channel went silent and stayed silent for the rest of the session. This happens in stereo too: one stereo run lost the left channel's own sound and kept only the centre's share in it.

Ocarina's sound thread feeds its six speaker voices one after another. It reads each voice's buffer count, then takes released buffers and appends to any voice that looks short. Hookshot's mixer released buffers from its own thread at any time. When a buffer finished between two of those reads, one voice looked a buffer short and got an extra append. From then on the game kept topping that voice up whenever its count was off, until its queue reached MAX_WB and that channel died.

I found this while testing #1124 (5.1 surround). It has no issue of its own that I could find.

How it was tested

  • Machine: Linux (CachyOS), RTX 4070 SUPER, NVIDIA 620.70.06.
  • Build: a full game build with this change on 8b2e1d3.
  • Runs: repeated launches of about 40 s each, going from the title screen into gameplay on a loaded save, alternating stereo and 5.1. Each run's output was checked channel by channel.
  • Result: before the change, 3 of 8 runs lost a channel; after it, 0 of 9 did.
  • New test: tests/test_audio_wave_buffer_release.py compiles the actual append, released, count and play-state calls, the update queue, commit() and the mixer's buffer step, then runs a six-voice pass through them. It fails on the current main (count(2) == 4) and passes with the change.
  • Suite: on this branch (rebased on 1f6fbdb), python -I -B tests/run_all.py gives 132 passed, 0 failed, 17 skipped on Linux.

🤖 Generated with Claude Code

## What A wave buffer the mixer finishes is now reported to the game at the next `RequestUpdateAudioRenderer` (`commit()`), as on the console, instead of the moment the mixer thread finishes it. Until that update, `GetReleasedWaveBuffer` does not return it and `GetWaveBufferCount` still counts it. Only `host/nn_audio.c` changes (a `nrel_seen` count per voice). ## Why In about one launch in five, one speaker channel went silent and stayed silent for the rest of the session. This happens in stereo too: one stereo run lost the left channel's own sound and kept only the centre's share in it. Ocarina's sound thread feeds its six speaker voices one after another. It reads each voice's buffer count, then takes released buffers and appends to any voice that looks short. Hookshot's mixer released buffers from its own thread at any time. When a buffer finished between two of those reads, one voice looked a buffer short and got an extra append. From then on the game kept topping that voice up whenever its count was off, until its queue reached `MAX_WB` and that channel died. I found this while testing #1124 (5.1 surround). It has no issue of its own that I could find. ## How it was tested - **Machine:** Linux (CachyOS), RTX 4070 SUPER, NVIDIA 620.70.06. - **Build:** a full game build with this change on 8b2e1d3. - **Runs:** repeated launches of about 40 s each, going from the title screen into gameplay on a loaded save, alternating stereo and 5.1. Each run's output was checked channel by channel. - **Result:** before the change, 3 of 8 runs lost a channel; after it, 0 of 9 did. - **New test:** `tests/test_audio_wave_buffer_release.py` compiles the actual append, released, count and play-state calls, the update queue, `commit()` and the mixer's buffer step, then runs a six-voice pass through them. It fails on the current `main` (`count(2) == 4`) and passes with the change. - **Suite:** on this branch (rebased on 1f6fbdb), `python -I -B tests/run_all.py` gives 132 passed, 0 failed, 17 skipped on Linux. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hckrman101 (Migrated from github.com) approved these changes 2026-11-09 18:51:42 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. Reporting finished buffers at the next RequestUpdateAudioRenderer is the safe direction (late, never early, so the game can't reuse memory the mixer is still reading), and it fits what the console does.

Checked:

  • nrel_seen is only moved in commit(), which runs under au_lock from the update HLE and from the 50 ms no-update fallback, so nothing is lost if the game stops updating.
  • Stop releases its buffers inside apply(), so they are seen by that same commit(). Looping buffers never release. Pause is untouched.
  • The MAX_REL overflow path in release_wb keeps nrel_seen consistent, and GetReleasedWaveBuffer decrements both counters. A fresh voice is memset, so nrel_seen starts at 0.
  • GetWaveBufferCount now includes the finished-but-unreported buffers, which is exactly what stops the half-updated count mid-pass.
  • No textual conflict that I can see with #1182 or #1063, both touch other parts of nn_audio.c.

One nit: the new test isn't in ARGS in tests/run_all.py. Every other C fixture has an entry like "--cc", "gcc" there, so the runner can skip it when no compiler is around. Without one it falls back to the bare cc default, which won't exist on the Windows runner and would fail instead of skipping. Something like "tests/test_audio_wave_buffer_release.py": ["--cc", "gcc"], next to the other audio entries. (Also worth noting CI hasn't run on this head yet, it's waiting for approval, so the test hasn't been exercised there.)

Looks good. Reporting finished buffers at the next RequestUpdateAudioRenderer is the safe direction (late, never early, so the game can't reuse memory the mixer is still reading), and it fits what the console does. Checked: - `nrel_seen` is only moved in `commit()`, which runs under au_lock from the update HLE and from the 50 ms no-update fallback, so nothing is lost if the game stops updating. - Stop releases its buffers inside `apply()`, so they are seen by that same `commit()`. Looping buffers never release. Pause is untouched. - The `MAX_REL` overflow path in `release_wb` keeps `nrel_seen` consistent, and `GetReleasedWaveBuffer` decrements both counters. A fresh voice is memset, so `nrel_seen` starts at 0. - `GetWaveBufferCount` now includes the finished-but-unreported buffers, which is exactly what stops the half-updated count mid-pass. - No textual conflict that I can see with #1182 or #1063, both touch other parts of nn_audio.c. One nit: the new test isn't in `ARGS` in `tests/run_all.py`. Every other C fixture has an entry like `"--cc", "gcc"` there, so the runner can skip it when no compiler is around. Without one it falls back to the bare `cc` default, which won't exist on the Windows runner and would fail instead of skipping. Something like `"tests/test_audio_wave_buffer_release.py": ["--cc", "gcc"],` next to the other audio entries. (Also worth noting CI hasn't run on this head yet, it's waiting for approval, so the test hasn't been exercised there.)
janvitos commented 2026-11-09 19:06:51 +00:00 (Migrated from github.com)

Thanks for the review. This was merged just before I pushed 3e7ee9a, so the run_all.py entry isn't in main. It isn't urgent: main's Windows CI (fe611e4) ran test_audio_wave_buffer_release.py and it passed, so that runner has a cc. I can open a one-line follow-up PR for the ["--cc", "gcc"] entry if you'd still like it for consistency.

Thanks for the review. This was merged just before I pushed 3e7ee9a, so the `run_all.py` entry isn't in main. It isn't urgent: main's Windows CI (fe611e4) ran `test_audio_wave_buffer_release.py` and it passed, so that runner has a `cc`. I can open a one-line follow-up PR for the `["--cc", "gcc"]` entry if you'd still like it for consistency.
Commenting is not possible because the repository is archived.
No description provided.