Repository navigation
Conversation
|
Heads-up on merging this one, and it is my doing rather than yours. This branch predates #560, which stopped tracking compiled bytecode. It has a Git leaves the file sitting in the working tree, so resolving with git rm <path>.disOr just rebase onto current master, where the file no longer exists and the Nothing else in the PR is affected — for d in appl appl/mpeg appl/veltro tests; do (cd $d && mk install); done
Sorry for the friction — five open PRs are in this position because of the |
pdfinn
left a comment
pdfinn
left a comment
There was a problem hiding this comment.
I cannot exercise CoreAudio from here, so this covers the parts that are
testable on Linux plus a read of the C. Someone on macOS should confirm the
backend itself and the SDL re-init behaviour — I am not signing off on those.
What I did check
The shared files do not break other platforms. emu/port/devaudio.c,
audio.h and audio-tbls.c are built everywhere, so I applied the PR onto
current master and built both Linux targets:
headless build exit=0
sdl3 build exit=0
audiodevwrite is careful C. Bounded at 256 with Ebadarg above it,
smalloc(count + 1) with an explicit NUL, memmove rather than a string
copy, and waserror()/poperror() around the allocation so the buffer is
freed on the error path. The quote handling matches the read format, so a line
from cat can be written straight back, which is a nice property and the
comment says so.
It is not agent-reachable. #A does not appear in
appl/veltro/nsconstruct.b, so a confined agent cannot enumerate or select
host audio devices. Worth stating explicitly in the PR, because "select the
microphone" is exactly the capability someone will ask about, and the answer
is good.
The new test skips honestly. On Linux:
this audio backend registers no buffer-cap accessor
PASS 0 passed, 1 skipped
That is the right shape — a clean SKIP naming the reason, rather than passing
while testing nothing. It is the pattern #563 is trying to establish and the
opposite of what #565's test currently does.
One thing to add
dis/tests/audioctl_readback_test.dis is in tests/mkfile TARG but not in
tools/dis-manifest.txt. Since #560 the manifest is the tracked record of
what the build must produce, so a new module needs both, in the same commit.
One line.
Not reviewed
emu/MacOSX/audio.c, the CoreAudio backend — needs a macOS reader- the SDL "came up with zero devices, re-initialise" path — I cannot reproduce
the condition - whether 16000 Hz in
audio-tbls.cis right for the speech capture path;
the addition is obviously harmless, the claim that speech needs it I am
taking on trust
Note on scope
This is three changes in one PR — device selection, SDL re-init, and ctl
readback — plus a new CoreAudio backend. They are related by subject but
independent in risk: the readback is trivial, the device file is a new
namespace surface, and the backend is 1000+ lines of platform code. If the
macOS review stalls on the backend, the first two are worth landing without
it. Not a blocker, just an option if it sits.
Add the manifest line, and resolve the dis/tests/audioctl_readback_test.dis
conflict by deletion — see my other comment.
fd1e519 to
0af4d14
Compare
|
Manifest line added and the 16000 Hz is right, and you were right to only take it on trust. Verified: I had the macOS parts reviewed here — the ones you said you were not signing off
The round-trip property you praised did not work. The comment and the doc The parser stripped the direction word but not The device-select test never exercised its own verdict. Step 5 captured to Also bounded the On scope: you offered landing the first two without the backend. With the |
No macOS configuration compiles emu/MacOSX/audio.c: the emu config lists 'audio audio-sdl3', emu-g has audio commented out, and both builds link audio-sdl3.o. It registers no Audiodevops, so it could not perform device selection even if it were wired in. Remove it and the -framework CoreAudio/-framework AudioToolbox flags added for it — link flags for an object that is never linked. audio_macos_test.sh grepped for coreerror() text only that file could emit; that branch was dead too and is gone. The empty-capture skip now exits 77 like every other skip, and the capture asserts a size floor instead of passing on a handful of bytes.
The comment in audiodevwrite and docs/SPEECH-REMOTE-AUDIO.md claimed a line from a read of #A/audiodev can be written straight back. It could not: the parser stripped the direction word but not the device/selected keyword, so every form the read actually emits was refused with 'no device of that name'. Teach the parser to skip that keyword (only when a name follows it, so a device literally called device stays reachable) and restate the accepted forms in the doc. Also document INFERNODE_AUDIO_IN / INFERNODE_AUDIO_OUT there, which were shipped undocumented.
ensure_sdl_audio re-ran its 20x50 ms SDL_Init/Quit probe on every call when the host enumerates no audio devices, so each /dev/audio open and audioctl read blocked a second or more, forever. Probe once and remember the result: sdl_quiet_host short-circuits the churn while the subsystem stays up, so SDL still sees devices that appear later. SDL_QuitSubSystem(SDL_INIT_AUDIO) was called without checking for live streams, in both the re-probe path and the 'not initialized' recovery in open_stream. Quitting under an open stream leaves in_stream / out_stream dangling and the next SDL_DestroyAudioStream is a use-after-free — the unplug-mid-capture race. Both sites now check sdl_streams_live() and leave the subsystem up instead.
Step 5 captured to /tmp/audiodev-cap.pcm, but /tmp does not exist
inside the emulator namespace: dd failed, cap_bytes stayed 0, and the
read always ended 'capture idle', so the active/silent verdicts this
feature exists to surface were never verified. Capture to a path under
the emu root instead and assert the verdict — 'idle' after a capture
to a writable file is now a failure, not a skip.
Also add the round-trip property the parser fix enables: the exact
line a read of #A/audiodev produced ('in device <name>') is written
back unchanged and must select.
0af4d14 to
fd51797
Compare
|
Rebased onto current master (df34b02). Force-pushed 0af4d14 -> fd51797.
Re-verified on macOS after the rebase. Device enumeration and select-by-name verified in both directions against a #578 fixed the ClusterFuzzLite link break, so |
#A could not be shared. Two programs writing /dev/audio interleaved their bytes, two reading the microphone split its samples (SDL) or the second was refused (OSS), and the format was one setting under everyone, so a program setting it changed it under the others. A call, a key's tone, speech and a notice at once -- a phone's ordinary case -- could not happen. This is layer 1 of the speech design. Every open of #A/audio, and every stream made by opening #A/mix/new (the fid becomes mix/n/ctl, a read of it gives n), is a stream of its own. What streams write is mixed into what the speaker plays, each converted from its format; every stream reading gets the whole microphone, in its format, from its open on. mix/n/ctl takes audioctl's format lines, a role (media, tone, notify, speech, call: media is a quarter under speech, notify and call, speech a third under call, ramped) and a queue (0: a writer waits for room, so a copy plays at the speaker's pace; N ms: never waits, only the newest kept, as a call needs). The streams are in mix/ because #A is bound onto /dev. #A/audio is a stream in audioctl's format, following it as it changes, as the device did; audioctl's play_buffer_ms and rec_buffer_ms (INFR- 194) are its streams' queues, and its readback is unchanged. The platform drivers are untouched: the mixer opens each one once each way, in its own format (48 kHz stereo 16-bit), when a stream first needs it, from the opener's process, so a stream that cannot have the hardware is refused at its open with the driver's error rather than writing into nothing; a driver that opens once (OSS, Einuse) serves both ways from one ORDWR open. Output is let go once the streams drain and nothing has played for 3 s, through the driver's own close (SDL's drain- on-close kept); what a stream writes before closing is played to its end. One kproc mixes, writing at most 40 ms ahead of the clock (SDL's queue never grows); one reads the microphone and gives it out. alk, a spin lock, is held only to move samples; clients wait in Sleep, which a kill interrupts, serialized per stream by a QLock no close takes. emu/port/audiomix.c is the arithmetic, with no emulator in it: G.711, 8- and 16-bit linear, mono and stereo, rate conversion by linear interpolation counted exactly (a 16.16 step drifted: 8 kHz to 48 kHz gave 48003 frames a second), queues, the mix. 16 and 48 kHz join the rate table. $INFERNODE_AUDIO_FAKE gives the device no hardware: the speaker is read back from #A/fakeout and #A/fakein is the microphone, for tests. tests/host/audiomix_test.sh: the arithmetic on the host (G.711 known values and round trips, tones through every rate pair within linear interpolation's own error, pieces equal to the whole, queues, mix). tests/host/audiodev_test.sh runs tests/audiodev_test.b against the fake: two formats mixed at their levels, #A/audio's writers mixed not interleaved, ducking down and back, the microphone whole to two readers, pacing, queues, drain, ctl; it fails with ducking or the microphone's fan-out broken. Both in CI's shard 2. Settings: nothing here a person tunes (roles and queues are a program's; the input and output device is #554's, with its panel). The native kernel's audio (os/) is still exclusive-use; man/3/audio says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195u6z7i448Ladv2piAEL5f
What this changes
The emulator opened whatever the operating system called the default audio
device. On a developer machine that default is often a virtual device belonging
to an application, which may not even be running. Capture opens, every sample is
zero, and no layer reports a fault. Diagnosing it meant leaving InferNode
entirely.
#A/audiodevmakes the choice and the verdict visible. Read it to enumeratewhat the host offers and what is selected; write a name to select one:
A line from the read can be written straight back —
audiodevwriteskips aleading
deviceorselectedword, soin device 'X'selectsX.SDL audio is re-initialised when it comes up with no devices. The subsystem
can start before the host has enumerated anything, in which case it reports zero
devices permanently rather than recovering. The probe runs once per host rather
than on every open, and
SDL_QuitSubSystemis guarded against live streams so adevice disappearing mid-capture cannot leave a dangling stream.
audioctlreads back the buffer capabilities. They were write-only, so acaller could set
play_buffer_msand had no way to confirm it took — and afailed
echo >to a ctl file does not set$status. Reading now returns thelive values.
audio_parse_buffer_msalso now accepts single-digit values, soplay_buffer_ms 0parses instead of falling through.emu/port/audio-tbls.cgains 16000 Hz, which speech capture needs and the tabledid not offer.
Not agent-reachable.
#Adoes not appear inappl/veltro/nsconstruct.b, soa confined agent cannot enumerate or select host audio devices.
Tests
tests/audioctl_readback_test.bexercises the capability round trip over 9P andis wired into
tests/mkfileTARG andtools/dis-manifest.txt.tests/host/audio_device_select_test.shcovers device selection, the read/writeround trip, and the capture verdict — the capture writes under the emu root, so
capture active/capture silentare asserted rather than skipped.tests/host/audio_macos_test.shfails when an open fails while devices arepresent, rather than passing quietly.
On macOS ARM64 (SDL3 build), and red against an emulator built from the merge
base:
The readback test skips rather than fails on a backend with no accessor, such as
Linux, so it discriminates by moving from skipped to passing.
Not included
The Alt+V keyboard chord in
emu/port/draw-sdl3.cbelongs to the desktop voicemode, not to audio device handling, and follows separately.