wgpu: collapse audio onto shared labelle-audio mixer (Phase 2 fan-out) - #389
Conversation
Fan-out of the Phase 2 audio pilot (bgfx). wgpu has no real OS audio device, so src/audio.zig now instantiates labelle_audio.Mixer(NullSink) and forwards every public fn to it, software-pumped via mixOutput. The duplicated overflow-safe WAV parser (wav_parser.zig, now in labelle-audio/src/wav.zig) is deleted along with the slot arrays and the f32 mixOutput mixer. - build.zig.zon: add labelle_audio v0.2.1 dep - build.zig: wire labelle-audio into the audio module + host audio test (replaces the now-removed wav_parser test) - src/audio.zig: thin adapter, public API preserved byte-for-byte; the shared mixer is i16, mixOutput converts to normalized f32 (the old f32 output had no consumer anywhere) - delete src/wav_parser.zig
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe wgpu audio backend now uses wgpu audio adapter migration
Sequence Diagram(s)sequenceDiagram
participant build.zig.zon
participant build.zig
participant src/audio.zig
participant labelle_audio
participant audio_tests
build.zig.zon->>build.zig: provides labelle_audio dependency
build.zig->>src/audio.zig: imports labelle-audio module
src/audio.zig->>labelle_audio: forwards load/play/mix calls
build.zig->>audio_tests: builds host-targeted smoke tests
audio_tests->>src/audio.zig: exercises Audio.resetForTest() and mixOutput
Changeswgpu audio adapter migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the WebGPU audio backend to delegate WAV decoding, PCM mixing, and slot management to the shared labelle-audio package, allowing the deletion of the local wav_parser.zig file. The backend now instantiates labelle_audio.Mixer(labelle_audio.NullSink) and converts the mixed i16 samples to f32 in mixOutput. A potential issue was identified in mixOutput where an odd output.len can cause the loop to break early, leaving the last sample uninitialized. It is recommended to zero out any remaining samples in this scenario.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // stereo interleave stays correct across chunk boundaries. | ||
| var chunk: usize = @min(remaining, scratch.len); | ||
| chunk -= chunk % CHANNELS; | ||
| if (chunk == 0) break; |
There was a problem hiding this comment.
If mix_samples is odd (which can happen if output.len is odd), chunk will become 0 and the loop will break early. This leaves the last sample in output uninitialized/untouched, which could lead to uninitialized memory reads or minor audio artifacts. Consider zeroing out any remaining samples in output[done..mix_samples] before breaking.
if (chunk == 0) {
@memset(output[done..mix_samples], 0.0);
break;
}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backends/wgpu/src/audio.zig`:
- Around line 185-186: The mix sample calculation in audio mixing is clamping
too late, which can overflow when frame_count is large. Update the logic around
the total_samples and mix_samples computation in the audio path to clamp by
frames first using output.len / CHANNELS, then multiply by CHANNELS, and keep
the loop in the same full-stereo-frame invariant so the mix routine never reads
or writes a partial frame.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 12faf714-e212-4be1-81ed-994b3bf196a3
📒 Files selected for processing (4)
backends/wgpu/build.zigbackends/wgpu/build.zig.zonbackends/wgpu/src/audio.zigbackends/wgpu/src/wav_parser.zig
💤 Files with no reviewable changes (1)
- backends/wgpu/src/wav_parser.zig
- Derive frames from output.len/CHANNELS before multiplying, so frame_count * CHANNELS can't overflow/trap for a large caller value (CodeRabbit); also makes mix_samples always frame-aligned, eliminating the odd-length early-break. - Zero any tail we don't mix so the host never reads uninitialized samples — silence, not garbage (Gemini).
The zero-tail addition broke mixOutput's deliberate 'only writes the frames it is asked for' contract (and its test). CodeRabbit's overflow fix alone (clamp frames before multiply) also resolves Gemini's odd-sample break — mix_samples is now always even, so the loop fully writes [0..mix_samples] with no early break. Tail stays the caller's, per contract.
1 participant
Fan-out of the Phase 2 audio pilot (after bgfx #388). Collapses wgpu's software audio onto
labelle-audio'sMixer(NullSink)(wgpu is software-only — no OS device, so the NullSink/manual-pump path).src/audio.zig: 297 → 233 lines — gone: slot arrays,loadWav, themixOutputmixer body. Remains:Mixer(NullSink)+ libc file-read shim + thin forwarders.src/wav_parser.zig(416 lines) DELETED — it was the overflow-safe parser already ported intolabelle-audio/src/wav.zig; the bug: wgpu audio WAV parser vulnerable to integer overflow on corrupt input #12 overflow regression-lock moved with it.mixOutput(output: []f32, frame_count)— the adapter mixes into an i16 scratch and converts to normalized f32; i16 is safe since the output is consumed nowhere outside audio.zig).labelle-audiov0.2.1 (url+hash, copied from bgfx).Build + test green (
backends/wgpu). raylib/sdl assessed separately → leave as-is (they delegate to raudio/SDL_mixer, no duplication), so the fan-out ends at sokol+wgpu.Summary by CodeRabbit
New Features
Bug Fixes
Tests