Repository navigation
Conversation
There was a problem hiding this comment.
Self-review recommended fixing the four findings below before merging: stream EOF truncates the final buffer, asynchronous device initialization errors are ignored, negative playback gain ramps in the wrong direction, and non-looping samples end without a fade.
Reviewed c0384e7. Three playback bugs were reproduced with headless regression probes; the device-error finding follows the pinned Oto v3.5.0 initialization contract and its asynchronous driver implementation. Normal, race-detector, and audio_oto tests passed for pkg/audio and pkg/api. Physical audio output was not tested.
|
All four self-review items are fixed in 5320aa3.
The three tests fail on c0384e7 and pass here. |
…ains and the device (nooga#255) From the review of nooga#1089: - A stream whose last read is short now mixes every frame it returned before leaving; it used to stop after the first. - A one-shot sample fades over its last 5 ms, or its second half if shorter, so a sample ending on a loud frame no longer clicks. - audio/tone and audio/sample reject a negative or infinite :gain, and audio/gain an infinite one; the mixer's fade in steps by the gain's size, so a negative gain from Go no longer grows without bound. - The oto device checks ctx.Err() once ready, since oto opens the device asynchronously and reports a failure there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The audio half of nooga#255: a handle-based capability with the binding discipline the surface seam uses. rt.Audio (Play, Stream, Stop, Gain, Enabled) is bound at the *audio* root, a silent no-op until a backend binds it with rt.SetAudioRoot or a Run binds one with api.WithAudio. The audio namespace is audio/tone, audio/sample (with :loop-from for an intro then a loop), audio/stop, audio/gain (with a fade) and audio/enabled?. Handles are Ints; with no backend each call returns nil. Stream is Go-only: a native module that synthesizes continuously joins the host's mix as a stream and gets an ordinary handle, so stop and gain work on it; lg code never sees it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ooga#255) pkg/audio is a mixer that implements rt.Audio: samples with a loop point, synthesized tones, and Go-side streams, each with a fade in and out so nothing clicks. Built with -tags audio_oto it plays through oto and binds the *audio* root at init; lg_boot_audio.go links it into lg under the same tag. Released builds set no tag, so lg stays silent and cgo-free. The device opens on the first sound, with the latency budget measured for undertone's native player: a 512-frame device buffer and 40 ms of read-ahead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ains and the device (nooga#255) From the review of nooga#1089: - A stream whose last read is short now mixes every frame it returned before leaving; it used to stop after the first. - A one-shot sample fades over its last 5 ms, or its second half if shorter, so a sample ending on a loud frame no longer clicks. - audio/tone and audio/sample reject a negative or infinite :gain, and audio/gain an infinite one; the mixer's fade in steps by the gain's size, so a negative gain from Go no longer grows without bound. - The oto device checks ctx.Err() once ready, since oto opens the device asynchronously and reports a failure there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nooga
left a comment
There was a problem hiding this comment.
Approving. I checked that the four self-review findings (stream EOF, async oto init error, negative gain, one-shot sample fade) are fixed in the code, with tests. Default builds carry no oto and the wasm/plan9 builds are clean. As project owner I'm fine with the new public surface (rt.Audio, SetAudioRoot, api.WithAudio).
Nits, none blocking:
- Run
go mod tidy:oto/v3 v3.5.0should be a direct require, and go.sum has stale v3.5.1 lines. rendercallsio.ReadFullon a stream while holdingm.mu. A source that returns(0, nil)or short reads forever would spin with the lock held. Consider a singleReadper tick with zero-fill on underrun, and close Closer sources on stop or end.- The tone's end fade sets
targetbut notstopping, so aGain()call during the last 220 frames overwrites it and the tone ends with a click. :freq ##Infpasses validation and turns the phase to NaN;:ms ##Infhas arch-dependent results. Check withmath.IsInf.- Go API only:
Mixer.Playpanics on odd-length PCM or empty PCM withLoopFrom>=0, andMixer.Gaindoesn't check for NaN.
nooga#255) From nooga's review of nooga#1089: - A tone's end fade marks it stopping, so a Gain call in its last 5 ms no longer replaces the fade and the tone ends without a click. - audio/tone rejects an infinite :freq, and :ms, audio/gain's fade-ms and :loop-from must be finite and in range, so no float converts arch-dependently. :loop-from ##Inf used to pass and panic the mixer. - Mixer.Play refuses PCM that is not whole stereo frames, a loop frame past the end (an empty loop included), and a gain or tone frequency that is not finite. Mixer.Gain ignores a gain that is not finite. - go mod tidy: oto is a direct require, and the stale v3.5.1 sums are gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ad on macOS (nooga#255) The comment called the device buffer 512 frames, but the formula, which converts at 4 bytes a frame while oto converts at 8, opens 1024. It is now written as 1024 frames; the duration passed to oto is unchanged. The read-ahead follows undertone's native player: on a MacBook Air (loopback, 2026-10-26) 40 ms gave 150-158 ms trigger to sound and 24 ms gave 119-128 ms. macOS gets 24 ms; other platforms, iOS included, keep 40 ms, since only macOS has been heard there. The old "about 130 ms" was a headless-browser figure, not a native one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The audio half of #255, built from points 1 and 3 of the 2026-10-10 comment.
pkg/rt/audio.go).rt.Audio(Play,Stream,Stop,Gain,Enabled) is bound at*audio*. Until a backend sets the root (rt.SetAudioRoot) or a Run binds one (api.WithAudio), it's a silent no-op, the same discipline as*surface*.audio/tone,audio/samplewith:loop-from,audio/stop,audio/gainwith an optional fade, andaudio/enabled?. Handles are Ints. With no backend, each call returns nil, andstopandgaintake a nil handle as a no-op, so a game runs silent without guarding its calls.Streamis Go-only. A continuously synthesizing module joins the host's mix and gets back an ordinary handle, sostopandgainwork on it.pkg/audio). A pure-Go mixer. With-tags audio_oto, it plays through oto and binds the root at init, andlg_boot_audio.golinks it intolg. Release builds set no tag, solgstays silent and cgo-free.Checked at this head (on 88a86e1, 2026-10-21):
go testpasses onpkg/...,cmd/...andtest/..., and onpkg/audioandpkg/apiwith-tags audio_oto. vet and the wasm build are clean, andcheck-generatedandcheck-generated-manifestare OK.lgplays a tone and fades it through its handle. A stocklgruns the same script silently, andaudio/enabled?returns false.Not here yet: the browser backend (point 5).
Refs #255
🤖 Generated with Claude Code