fix(audio): align SDL device buffer to audio frame quantum and allow env override #1329
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1329
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/audio-buffer-alignment"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
Aligns the default SDL2 and SDL3 audio device sample frame buffer from 512 to 480 samples, matching the engine's 240-sample audio frame quantum (
FRAME 240inhost/nn_audio.c, 5 ms at 48 kHz) and 480-sample WaveOut buffer blocks. Also respectsSDL_AUDIO_DEVICE_SAMPLE_FRAMESfrom the environment if specified by the user.Why
On Linux systems using SDL3 with PipeWire or PulseAudio, the hardcoded hint
SDL_AUDIO_DEVICE_SAMPLE_FRAMES = 512causes SDL's audio pull callback to request 512 frames (2048bytes for 16-bit stereo).Because the game submits blocks of 480 frames (
1920bytes), whenever the queue drops to 1 block ready,wave_pullinhost/posix/win32_posix.cconsumes the 480 frames and fills the remaining 32 frames (128 bytes) with zero silence (memset(out, 0, len)). This fractional mismatch accumulates across callbacks and produces regular micro-dropouts and audio crackles.Additionally, unconditionally calling
hos_sdl_hintprevented users from overridingSDL_AUDIO_DEVICE_SAMPLE_FRAMESvia environment variables.How it was tested
tests/test_sdl_audio_library.py. Ran full regression suitepython3 tests/run_all.py(166 passed, 0 failed, 17 skipped).Thanks, the env override is a good change and CI is green. I have two questions about the explanation and the SDL2 path.
The PR description says a 512-frame callback against 480-frame blocks leaves 32 frames of silence on every callback. That doesn't match
wave_pullinhost/posix/win32_posix.c. It loops across queued blocks and carriesw->posover, so a partially consumed block is finished in the next callback. Silence is only written when the queue is empty, which is a real underrun and not a rounding artifact. So I don't see the accumulating fractional mismatch. A 480-frame buffer may still help by cutting latency and changing callback timing, and your in-game result may well be real. But a smaller buffer also leaves less margin against the game thread being late, so it would be good to say what you measured, e.g. whether the crackle was gone with SDL_AUDIO_DEVICE_SAMPLE_FRAMES=1024 as well, or only at 480.On the SDL2 path,
want.samplesis now 480, which isn't a power of two. SDL2's docs ask for a power of two, and some backends round it up or handle it badly. I haven't checked which ones do, but worth confirming on SDL2 or using 512 there.Nit: the test only greps the source text, so it would pass even if the env value were ignored in the SDL2 branch.
If the env var is set, SDL3 reads it as a hint itself, so skipping our hint is correct. Just noting the SDL2 branch below uses
atoion the same string, so a non-numeric value silently falls back to 480.This landed in the v2.0.12 branch (merged there for testing; it reaches
mainwith that release), so closing. Thanks!Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.
Thanks @NotRexed for merging and @hckrman101 for the detailed review!
To answer your questions:
wave_pulland measured buffer sizes (480vs960/1024):You are completely right about
w->poscarrying over across blocks when multiple buffers are queued. What we observed in-game on Linux (PipeWire + SDL3) were two combined effects:480-frame block was sitting in the queue at callback time, a512-frame pull immediately underran by 32 frames (memsetsilence), whereas a480-frame pull was satisfied cleanly by that single block without zero-filling.AudioFrameworkThreadsuffered virtual core contention (vcores: ... waits longest 50-60 msin the log, especially sincehos_thread_boostis currently a no-op on POSIX), settingSDL_AUDIO_DEVICE_SAMPLE_FRAMES=960(or1024) gave the extra safety margin needed to absorb those stalls completely. Having the env var respected makes tuning that margin effortless.SDL2
want.samplespower-of-two & test coverage:Good catch on SDL2's documentation requiring a power of two (
512) for legacy SDL2 backends, as well as the test assertion checking the SDL2 assignment explicitly. I'm happy to open a small follow-up PR to keep512as the default fallback on the SDL2 path while preservingenv_samplesand tighteningtest_sdl_audio_library.py.