brainray: add audio -- music streams and one-shot sounds - #302
Conversation
brainray had no audio functions at all, so a Brainrot program could not make a sound. This is B3 from tung-tung-sahur's design doc, and the last engine gap that game had left. Music and Sound are structs that cannot cross the Road A ABI and outlive any statement, so they get the same treatment as Texture2D: C owns them, Brainrot holds an integer index, a failed load returns -1 without consuming a slot, and rl_close_audio_device() unloads everything still live before the device goes away -- a live handle has to imply a live device or a later init plus a stale handle hands raylib a freed stream. Both Music and Sound, because choosing between them is the caller's decision and neither mistake produces an error. An 80-second stereo track loaded as a Sound is tens of megabytes of decoded PCM; a one-shot loaded as Music is a stream you have to remember to pump. The docs say so in a table rather than leaving it to the function names. Three functions exist specifically so the module can be tested rather than asserted. rl_is_audio_device_ready() matters because raylib does not report failure from InitAudioDevice() -- on a machine with no sound device it logs and carries on, and every later call silently does nothing, so without this a program cannot tell "playing quietly" from "not playing". And rl_is_music_playing()/rl_music_time_played() are what make the one dangerous contract observable. That contract is rl_update_music(). It refills the decode buffer, it has to be called every frame, and raylib says nothing if you don't -- the track plays for a fraction of a second and stops, which looks exactly like a broken file. So the test does not assert that updating is required, it measures it: two identical streams over the same wall clock, one pumped and one not, reported 1.60s and 0.00s of playback. If that ever stops being 0.00 the test says so, because at that point it would be proving nothing. Verified against a real 82-second Ogg Vorbis track as well as the generated WAV the test uses, so the fixture stays hermetic and the repo does not depend on a game asset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Stale comment
REQUEST CHANGES
Sound is a thin Road A wrap and covers B3. Music is extra, and it breaks the invariant this PR wrote down: a live handle must imply a live device.
1.
rl_load_musiccan mint a live handle with no audio deviceBLOCKING.
LoadMusicStreamsetsframeCountfrom the file afterdrwav_init_file/ equivalent succeeds.LoadAudioStreamis a separate step and can return a stream withbuffer == NULLwhen the device was never ready or was already closed. This wrapper treatsframeCount > 0as success and stores the slot anyway.
rl_load_sounddoes not have this hole:LoadSoundFromWaveleavesframeCount == 0if the buffer cannot be created, so it already fail-closes without a device.Then
rl_is_music_playing(and failed-loadUnloadMusicStream, which starts withIsMusicStreamPlaying) takema_mutex_lock(&AUDIO.System.lock). AfterCloseAudioDevicethat mutex is destroyed. After a failedInitAudioDeviceit was never initialized.THE COMMENT SAYS: a live handle implies a live device.
THE RUNTIME DOES: load after close, or load after failed init, can create a live handle.Gate both loaders on
IsAudioDeviceReady()and return-1before calling raylib. Keep theframeCount == 0unload — that path is required for an opened 0-frame file — but only after the device is known live.2. Docs invert the handle-reuse contract
MAJOR.
docs/brainray.mdsays a post-unload handle "is not reused, it is inert". The tables recycle the lowest free index. A stale handle aliases the next load. The texture section of this same file already documents that correctly. Say that here too.3. The only audio test skips every assertion without a device
MAJOR.
CI is headless. The program exits before
missing -1, and pytest skips. After the load gate, the!readypath can assertrl_load_music/rl_load_soundreturn-1without a playback device. That is the contract CI can actually pin.The pump measurement is the right test for
rl_update_music. It is not a test of B3 (rl_play_sound) and it will not run on CI.VERDICT
Architecture: the constructor of a music handle does not maintain the live-device invariant the close path was written to protect. Implementation is otherwise a straightforward Road A table, same shape as textures.
Do not merge until load refuses a dead device.
Sent by Cursor Automation: Code Reviewer
Addresses the review on #302. The blocking finding was right about the invariant even though the crash does not reproduce here. On this machine both loaders already returned -1 without a device, because frameCount == 0 happened to catch it. That is the point: the invariant this module's close path was written to protect -- a live handle implies a live device -- was accidental rather than enforced, and it rested on a property of two different raylib functions that do not actually agree. LoadMusicStream() sets frameCount from the FILE once the decoder opens it, while attaching the playback stream is a separate step, so frameCount > 0 is not evidence of a usable stream. LoadSoundFromWave() does fail closed. Relying on that difference is not a contract. Both loaders now check IsAudioDeviceReady() and return -1 before calling raylib, so it is one rule instead of two coincidences. The frameCount == 0 unload stays, since an opened zero-frame file still needs it. Verified by mutation rather than by argument: with the gate removed and the device reported dead, the test fails on the live-handle assertion. With the gate present and a device-less machine simulated end to end, it passes in 0.11s having asserted rather than skipped. Second finding, also right: the docs had the handle-reuse contract backwards. They said a post-unload handle "is not inert, it is reused" the wrong way round -- claiming inertness, when the tables recycle the lowest free index exactly as the texture tables do, so a stale handle aliases the next load. The texture section of that same file already said this correctly. Now both do. Third finding: the test skipped every assertion without a device, which is CI. It now has two halves. Without a device it asserts what CI can actually pin -- that both loaders refuse before init, refuse with no device, and refuse after close. With a device it keeps the pump measurement and adds the one-shot path, since B3 is as much about rl_play_sound as the stream.
|
All three addressed. On the blocking one you were right about the invariant, and I want to be precise about what I found, because the crash does not reproduce here. On this machine both loaders already returned That is the actual problem, though. The invariant the close path was written to protect was accidental, and it rested on a property of two raylib functions that don't agree: So both loaders now check Verified by mutation, not by argument. With the gate removed and the device reported dead, the test fails on the live-handle assertion. With the gate present and a device-less machine simulated end to end, it passes in 0.11 s — having asserted, not skipped. Docs, second finding. I had the handle-reuse contract backwards: claimed inertness when the tables recycle the lowest free index exactly as the texture tables do, so a stale handle aliases the next load. The texture section of that same file already said it correctly. Now both do, and the ownership check is described as the temporary courtesy it is rather than the contract. Test, third finding. You're right that it skipped everything on CI, which made it worthless there. Two halves now. Without a device it asserts what CI can actually pin — both loaders refuse before init, refuse with no device, and refuse after close. With a device it keeps the pump measurement and adds the one-shot path, since B3 is as much about
/review |


Description
brainray had no audio functions at all — a Brainrot program could not make a sound. This is B3 from tung-tung-sahur's design doc, and the last engine gap that game had left.
Eighteen wrappers: device open/close/ready, music (load, play, update, stop, volume, looping, playing, time played, time length, unload), and sound (load, play, playing, volume, unload).
MusicandSoundare structs that can't cross the Road A ABI and outlive any statement, so they get the same treatment asTexture2D— C owns them, Brainrot holds an integer index, a failed load returns-1without consuming a slot, andrl_close_audio_deviceunloads everything still live before the device goes away. A live handle has to imply a live device, or a later init plus a stale handle hands raylib a freed stream.The one contract that fails silently
rl_update_music()refills the decode buffer. It must be called every frame, and raylib says nothing if you don't — the track plays for a fraction of a second and stops, which looks exactly like a broken file.So the test doesn't assert that updating is required. It measures it: two identical streams over the same wall clock, one pumped and one not.
And it asserts
idle == 0.0explicitly, with a message saying that if this ever stops being zero thenrl_update_musicisn't what advances a stream and the test is proving nothing.Three functions that exist to make this testable
rl_is_audio_device_ready()— raylib does not report failure fromInitAudioDevice(). On a machine with no sound device, or in a container, it logs and carries on, and every later call silently does nothing. Without this a program cannot distinguish playing quietly from not playing, and the test cannot tell a real failure from a headless runner.rl_is_music_playing()/rl_music_time_played()— otherwise "did the music play?" has no answer inside Brainrot.Music or Sound is a real choice
Both are included because choosing is the caller's decision and neither mistake produces an error. An 80-second stereo track loaded as a
Soundis tens of megabytes of decoded PCM; a one-shot loaded asMusicis a stream you have to remember to pump. The docs put that in a table rather than leaving it to the function names.Verification
Tested against a real 82-second Ogg Vorbis track (the game's actual background music) — device ready, handle 0, reported length 82.3 s which matches the file's Ogg granule position exactly, play/stop/unload all correct, stale handle inert, clean ASan exit.
The committed test uses a generated sine WAV instead, hand-rolled from a RIFF header, so the fixture stays hermetic and this repo doesn't depend on a game asset or an encoder dependency.
Skips when no audio device is present, which is the normal case on a headless runner — the same shape as the existing windowed tests.
Related Issue
Implements B3 from tung-tung-sahur's DESIGN.md §15.1. No issue filed here — say the word and I'll open one, as with #291 for B1.
Type of Change
Checklist
make format-checklocally (ormake formatto fix)make test425 passed.make valgrind407 cases, 0 errors, exit 0.make format-checkclean.make cppchecknot run — not installed here; CI'sstatic-analysisjob covers it.🤖 Generated with Claude Code