WebSocket audio backend via AudioIO ABC (updated #189) - #215
Conversation
Integrate the websocket audio backend from #189 (by reisbauer03) into the audio_io package, reframed onto the AudioIO abstract base class so the engine runs identically on local hardware (sounddevice) or a network backend. - Add AudioIO ABC (base.py) and make both backends subclass it; keep AudioProtocol as a back-compat alias. - WebsocketAudioIO: /microphone and /speaker endpoints, 16kHz float32 streaming, VAD per client, playback ack (played/time/sampleRate/reset). - Rooms are opt-in via backend role 'rooms: true' (off by default); when on, multi-mic ownership + segregate_speakers routing. Off => single-source broadcast to all speakers. - get_audio_system()/GladosConfig accept backend_options; add configs/glados_websocket_config.yaml, protocol docs, and browser + Python reference clients. Add websockets>=16.0 dependency.
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a WebSocket audio backend for GLaDOS. It defines a shared ChangesWebSocket Audio IO
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MicClient
participant WebsocketAudioIO
participant SpeakerClient
MicClient->>WebsocketAudioIO: Connect to /microphone and stream Float32 audio
WebsocketAudioIO->>WebsocketAudioIO: Apply VAD and room arbitration
WebsocketAudioIO->>WebsocketAudioIO: Enqueue controlled samples
SpeakerClient->>WebsocketAudioIO: Connect to /speaker and select room
WebsocketAudioIO->>SpeakerClient: Send synchronized audio data
SpeakerClient->>WebsocketAudioIO: Send played acknowledgement
sequenceDiagram
participant GladosConfig
participant get_audio_system
participant WebsocketAudioIO
GladosConfig->>get_audio_system: Pass backend type and audio options
get_audio_system->>WebsocketAudioIO: Construct backend with options
WebsocketAudioIO-->>get_audio_system: Return AudioIO instance
GladosConfig->>WebsocketAudioIO: Close backend during shutdown
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
src/glados/audio_io/websocket_io.py (1)
345-353: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow
_audio_databefore you dereference it.
self._audio_datais typedAudioData | None. Lines 348 and 386-390 access attributes without aNonecheck. The runtime path is currently protected by_is_playing, but a type checker reports these accesses, and a future change to the playback lifecycle turns them intoAttributeError.Add an explicit guard inside the lock in both places.
♻️ Proposed refactor for `set_flags_once`
with self._audio_lock: - if self._audio_data.track_id == track_id: + if self._audio_data is not None and self._audio_data.track_id == track_id: self._playback_was_interrupted = was_interruptedAlso applies to: 385-390
🤖 Prompt for 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. In `@src/glados/audio_io/websocket_io.py` around lines 345 - 353, Within both lock-protected paths in the relevant websocket audio methods, explicitly guard that self._audio_data is not None before accessing its track_id or other attributes. Narrow the optional value inside each guard, preserving the existing playback state updates and one-time track_id reset behavior.
🤖 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 `@configs/glados_websocket_config.yaml`:
- Around line 83-89: Update the shipped prompt in the system entry and its
example assistant responses to remove references to cyanide, firearms, and
Russian roulette, replacing them with harmless dark humor. Add an explicit
constraint prohibiting advice involving self-harm, violence, weapons, or
poisoning while preserving GLaDOS’s concise, sarcastic tone.
In `@docs/audio_websocket.md`:
- Around line 37-38: Update the audio format documentation to limit the 16 kHz
PCM requirement to microphone input, and document that speaker clients must use
the sample rate provided by the backend’s sampleRate:<hz> message when decoding
tracks.
In `@examples/audio_websocket_client.py`:
- Around line 44-58: Update the message handling loop in speaker_client() to
implement complete scheduling and feedback: detect and parse time:<unix_ts>
messages to schedule playback at the specified time rather than immediately,
wait for audio playback to complete using sd.wait() after sd.play() returns,
send a played message back to the websocket after completion to signal server
state clearing, and keep the receive loop active during playback so reset
messages can still interrupt pending audio before completion. This requires
restructuring the current timeout-based loop to allow concurrent receive and
playback operations.
- Around line 25-37: The callback function is invoked from the audio stream's
thread, but asyncio.Queue.put_nowait is not thread-safe. Replace the direct
put_nowait call in callback with get_running_loop().call_soon_threadsafe() to
safely bridge the audio callback into the event loop. Bound the asyncio.Queue
instance by adding a maxsize parameter to out during initialization, and define
explicit drop behavior when the queue reaches capacity (either drop the item or
raise an exception using put_nowait's behavior when the queue is full).
In `@src/glados/audio_io/__init__.py`:
- Around line 20-22: Replace the AudioProtocol = AudioIO alias with a structural
Protocol declaring the documented audio methods, while preserving the legacy
AudioProtocol export and its compatibility with Glados, SpeechListener, and
SpeechPlayer. Update the AudioProtocol documentation to describe it as the
legacy structural interface; do not require backend implementations to subclass
AudioIO.
In `@src/glados/audio_io/base.py`:
- Around line 14-54: Add an abstract close() lifecycle method to AudioIO in
src/glados/audio_io/base.py:14-54, implement it in WebsocketAudioIO to stop and
release the WebSocket server, and invoke it during Glados initialization failure
unwinding after backend creation in src/glados/core/engine.py:852-855 so failed
construction does not leave the backend bound.
In `@src/glados/audio_io/websocket_io.py`:
- Around line 232-235: Update the timeout branch in the playback wait logic to
clear the active playback state before returning: reset _is_playing, clear
_audio_data.track_id, and set _stop_playback so connected speaker tasks exit
their waiting loop and return to idle. Preserve the existing (True, 0) return
value.
- Around line 466-471: In the bytes-handling branch of the microphone websocket
handler, validate that msg has a length divisible by the float32 item size
before calling np.frombuffer. Handle invalid payloads locally with an
appropriate diagnostic and skip that frame, preserving current_data accumulation
and the handler’s control claim for valid audio frames.
- Around line 186-196: Reorder the playback state updates in the scheduling flow
around `_is_playing` so `_stop_playback` and `_playback_was_interrupted` are
initialized before setting `_is_playing = True`. Keep the audio track creation
unchanged, ensuring speaker tasks cannot observe the new track until both flags
are ready.
- Around line 237-239: Adjust the percentage calculation in
measure_percentage_spoken to subtract speaker_sync_delay_ms from elapsed before
converting elapsed time into played_samples. Ensure the adjusted elapsed time
cannot become negative, while preserving the existing 100% cap and return
values.
- Around line 268-298: Update _run_server to store the object returned by
websockets.serve in a distinct server-instance variable, preserving the server
string parameter for address-related use. Broaden startup exception handling to
catch any exception from websockets.serve, set that exception on result_future
before re-raising it, and continue serving with the renamed server instance
after successful startup.
In `@tests/audio-websocket-mic.html`:
- Around line 79-91: Consolidate the two startBtn click listeners into one async
startup sequence: acquire and assign micStream with getUserMedia first, then
call micConnect only after acquisition succeeds, preserving the existing button
state updates. Handle acquisition failures by re-enabling startBtn, and ensure
startMic reuses the acquired stream without requesting or starting a second
microphone stream.
---
Nitpick comments:
In `@src/glados/audio_io/websocket_io.py`:
- Around line 345-353: Within both lock-protected paths in the relevant
websocket audio methods, explicitly guard that self._audio_data is not None
before accessing its track_id or other attributes. Narrow the optional value
inside each guard, preserving the existing playback state updates and one-time
track_id reset behavior.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f1d6a75a-402d-443e-9166-2af6ade59657
📒 Files selected for processing (12)
configs/glados_websocket_config.yamldocs/audio_websocket.mdexamples/audio_websocket_client.pypyproject.tomlsrc/glados/audio_io/__init__.pysrc/glados/audio_io/base.pysrc/glados/audio_io/sounddevice_io.pysrc/glados/audio_io/websocket_io.pysrc/glados/core/engine.pytests/audio-websocket-both.htmltests/audio-websocket-mic.htmltests/audio-websocket-speaker.html
| played_samples = elapsed * sample_rate | ||
| percentage_played = min(int(played_samples * 100 / total_samples), 100) | ||
| return interrupted, percentage_played |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Subtract the speaker sync delay from the elapsed time.
elapsed starts when measure_percentage_spoken begins to wait, but the client starts playback only at play_time, which is speaker_sync_delay_ms later. For an interrupted playback, percentage_played therefore overstates the spoken fraction by up to speaker_sync_delay_ms of audio. The engine uses this percentage to record what the assistant actually said.
🛠️ Proposed fix
- played_samples = elapsed * sample_rate
+ play_seconds = max(elapsed - (self._speaker_sync_delay_ms / 1000.0), 0.0)
+ played_samples = play_seconds * sample_rate
percentage_played = min(int(played_samples * 100 / total_samples), 100)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| played_samples = elapsed * sample_rate | |
| percentage_played = min(int(played_samples * 100 / total_samples), 100) | |
| return interrupted, percentage_played | |
| play_seconds = max(elapsed - (self._speaker_sync_delay_ms / 1000.0), 0.0) | |
| played_samples = play_seconds * sample_rate | |
| percentage_played = min(int(played_samples * 100 / total_samples), 100) | |
| return interrupted, percentage_played |
🤖 Prompt for 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.
In `@src/glados/audio_io/websocket_io.py` around lines 237 - 239, Adjust the
percentage calculation in measure_percentage_spoken to subtract
speaker_sync_delay_ms from elapsed before converting elapsed time into
played_samples. Ensure the adjusted elapsed time cannot become negative, while
preserving the existing 100% cap and return values.
|
@coderabbitai review\n\nImplemented the requested fixes in c4ffb45:\n\n- Restored the structural AudioProtocol while retaining AudioIO for in-tree backends.\n- Added explicit backend cleanup, WebSocket server close/join, normal shutdown cleanup, and failed-initialization unwinding.\n- Made playback state publication atomic, cleared timeout state, guarded optional audio data, and used a monotonic scheduled-start deadline for spoken-percentage accounting.\n- Propagated all WebSocket startup exceptions and removed the server variable shadowing.\n- Rejected malformed float32 microphone frames without dropping the client.\n- Completed the Python speaker protocol with scheduling, concurrent reset handling, playback completion, and played acknowledgements.\n- Bridged the sounddevice callback safely into a bounded asyncio queue with explicit oldest-frame dropping.\n- Consolidated browser microphone acquisition into one failure-aware startup path.\n- Corrected sample-rate documentation, added a trusted-network warning, and replaced unsafe shipped prompt examples.\n- Added automated coverage for timeout cleanup, scheduled timing, close/rebind, startup exception propagation, malformed frames, and an end-to-end speaker acknowledgement.\n\nVerification:\n- 26 focused WebSocket audio, audio resampling, and shutdown tests pass.\n- Ruff and formatting checks pass on the changed audio/client/test files.\n- Strict mypy checks pass on the changed audio modules and new tests.\n- The wider branch run reached 195 passing and 1 skipped test; unrelated existing autonomy failures and local optional-dependency collection problems prevent a clean full-suite result in this environment.\n\nPlease run another review against c4ffb45. |
|
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/glados/audio_io/websocket_io.py (1)
575-592: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce the default single-microphone policy.
When
self._roomsisFalse,has_controlstaysTruefor every/microphoneconnection. Each client then writes samples to_sample_queue. This mixes concurrent microphone streams, despite the default single-client input contract.Assign
MicState.current_idto one microphone, or reject additional microphone connections, even when room routing is disabled. Queue samples only from that owner. Add a regression test with two connected microphones.Proposed fix
async with self._mic_state_lock: if self._rooms: # Existing room-based ownership logic. ... + else: + if self._mic_state.current_id is None: + self._mic_state.current_id = client_id + has_control = self._mic_state.current_id == client_id # If we have control, put sample on queue if has_control:🤖 Prompt for 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. In `@src/glados/audio_io/websocket_io.py` around lines 575 - 592, Update the microphone control flow around MicState.current_id and has_control so single-microphone ownership is enforced when self._rooms is false: assign the first connection as owner or reject later connections, and queue samples only from the owner. Preserve existing room-routing behavior, and add a regression test covering two connected microphones to verify their streams are not mixed.
🤖 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.
Outside diff comments:
In `@src/glados/audio_io/websocket_io.py`:
- Around line 575-592: Update the microphone control flow around
MicState.current_id and has_control so single-microphone ownership is enforced
when self._rooms is false: assign the first connection as owner or reject later
connections, and queue samples only from the owner. Preserve existing
room-routing behavior, and add a regression test covering two connected
microphones to verify their streams are not mixed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ac1b731-23e7-4a20-9597-ae0272937ea7
📒 Files selected for processing (10)
configs/glados_websocket_config.yamldocs/audio_websocket.mdexamples/audio_websocket_client.pysrc/glados/audio_io/__init__.pysrc/glados/audio_io/base.pysrc/glados/audio_io/sounddevice_io.pysrc/glados/audio_io/websocket_io.pysrc/glados/core/engine.pytests/audio-websocket-mic.htmltests/test_websocket_audio.py
🚧 Files skipped from review as they are similar to previous changes (6)
- src/glados/audio_io/base.py
- docs/audio_websocket.md
- src/glados/audio_io/sounddevice_io.py
- examples/audio_websocket_client.py
- tests/audio-websocket-mic.html
- configs/glados_websocket_config.yaml
|
@coderabbitai review\n\nAddressed the follow-up in 6f16834:\n\n- Default rooms=false mode now assigns one microphone owner and permits only that client to enqueue samples.\n- Ownership is released on disconnect or while listening is stopped, and can be reclaimed cleanly after a stop/start cycle.\n- Added a concurrent two-microphone regression test proving the second stream cannot enter the shared queue.\n- Added missing docstrings across the affected WebSocket backend, protocol, reference client, and regression tests.\n\nVerification:\n- 27 focused WebSocket audio, resampling, and shutdown tests pass.\n- Ruff and formatting pass.\n- Strict mypy passes on the changed WebSocket modules and tests.\n- Docstring coverage for the affected scope is 100%, exceeding the required 80%.\n\nPlease review commit 6f16834. |
|
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/glados/audio_io/websocket_io.py (1)
185-187: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRelease microphone ownership when listening stops.
stop_listening()only changes_is_listening. An idle owner remains blocked inwebsocket.recv()and does not callrelinquish(). If listening starts again before that client sends data or disconnects,MicState.current_idstill selects it and blocks other microphones.Schedule an ownership reset on the server loop when listening stops. Reset
silence_chunkswith the owner. Add a stop/start regression test with an idle first client and an active second client.🤖 Prompt for 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. In `@src/glados/audio_io/websocket_io.py` around lines 185 - 187, Update stop_listening() to schedule microphone ownership reset on the server event loop, ensuring the current owner is relinquished and its silence_chunks state is reset. Preserve correct ownership behavior when listening restarts, and add a regression test covering an idle first client stopping, followed by an active second client starting successfully.
🤖 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 `@src/glados/audio_io/websocket_io.py`:
- Around line 624-626: Bound the microphone sample queue used by the VAD path
around _sample_queue.put so it has a finite capacity and cannot grow
indefinitely. Reuse the reference microphone client’s queue size and
non-blocking drop-oldest overflow behavior, while preserving the existing
(vad_data, bool(vad_confidence)) enqueue format.
In `@tests/test_websocket_audio.py`:
- Around line 221-223: Update the first-sample wait after creating first_task in
the websocket audio test to run inside asyncio.wait_for with a finite timeout,
preserving the existing queue-size polling behavior and causing a timeout
failure if no sample is enqueued.
---
Outside diff comments:
In `@src/glados/audio_io/websocket_io.py`:
- Around line 185-187: Update stop_listening() to schedule microphone ownership
reset on the server event loop, ensuring the current owner is relinquished and
its silence_chunks state is reset. Preserve correct ownership behavior when
listening restarts, and add a regression test covering an idle first client
stopping, followed by an active second client starting successfully.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a7fba855-f020-442b-be2d-d805f6c2eb44
📒 Files selected for processing (5)
examples/audio_websocket_client.pysrc/glados/audio_io/__init__.pysrc/glados/audio_io/sounddevice_io.pysrc/glados/audio_io/websocket_io.pytests/test_websocket_audio.py
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/audio_websocket_client.py
- src/glados/audio_io/sounddevice_io.py
|
@coderabbitai review\n\nDeep-reviewed and addressed the three follow-ups in 45532ac.\n\nOwnership reset:\n- Confirmed valid: an idle owner blocked in recv did survive stop_listening.\n- stop_listening now resets owner and silence state under the asyncio lock on the server loop, with a thread-safe handoff for callers outside that loop.\n- Disconnect relinquishment now resets silence state as well.\n- Added a real server stop/start regression where an idle first client remains connected and an active second client successfully owns the restarted session.\n\nQueue bound:\n- Confirmed the unbounded-memory issue.\n- Used a configurable default of 256 VAD chunks rather than the suggested 8. The reference queue handles network jitter, while this queue also spans synchronous ASR stalls; 8 chunks retain only 256 ms and would truncate legitimate speech.\n- The 256-chunk default retains 8.2 seconds (about 0.5 MiB raw), drops oldest non-blockingly, counts drops, and rate-limits warnings.\n- Added overflow-order and invalid-capacity tests and documented the option.\n\nTest deadline:\n- Added a one-second asyncio.wait_for deadline to the first-sample poll.\n\nVerification:\n- 30 focused WebSocket audio, resampling, and shutdown tests pass.\n- Ruff and formatting pass.\n- Strict mypy passes.\n- Docstring coverage remains 100% for the affected scope.\n\nPlease review 45532ac. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Commit metadata and changed files ==='
git show --no-ext-diff --format=fuller --stat --summary 45532ac
git diff --no-ext-diff --name-status 45532ac^ 45532ac
echo '=== Changed Python file outlines ==='
for file in $(git diff --name-only 45532ac^ 45532ac -- '*.py'); do
echo "--- $file ---"
ast-grep outline "$file" --items all || true
done
echo '=== Commit diff ==='
git diff --no-ext-diff --unified=80 45532ac^ 45532ac -- \
src/glados/audio_io/websocket_io.py \
tests/test_websocket_audio.py \
docs/audio_websocket.md \
configs/glados_websocket_config.yamlLength of output: 52275 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Audio factory configuration path ==='
sed -n '1,150p' src/glados/audio_io/__init__.py
echo '=== Default-mode and microphone ownership references ==='
rg -n -C 4 --glob '*.md' --glob '*.yaml' --glob '*.py' \
'(rooms:\s*false|rooms.*false|single[- ]source|one microphone|microphone owner|multiple microphone|mic_queue_max_chunks)' \
.
echo '=== Relevant final implementation sections ==='
sed -n '80,255p' src/glados/audio_io/websocket_io.py
sed -n '660,735p' src/glados/audio_io/websocket_io.pyLength of output: 28327
✏️ Learnings added
|
1 participant
Implementation of a WebSocket audio backend, based on #189 by @reisbauer03 but
reframed onto the project's audio
AudioIOABC so the engine runs identically onlocal hardware (sounddevice) or over the network.
What's included
AudioIOABC (src/glados/audio_io/base.py): single contract;SoundDeviceAudioIOand
WebsocketAudioIOboth subclass it.AudioProtocolkept as a back-compat alias.WebsocketAudioIO:/microphone(16 kHz float32 → VAD → sample queue) and/speaker(TTS playback withtime/sampleRate/played/resetack flow).audio_io_options: rooms: true, default false): multi-micownership arbitration +
segregate_speakersrouting. Default = single-client,broadcast to all speakers.
config.audio_io_options),configs/glados_websocket_config.yaml,protocol docs, and browser (
tests/audio-websocket-*.html) + Python(
examples/audio_websocket_client.py) reference clients.Notes
Closes/supersedes #189 (superseded implementation). Author's unrelated older-fork
changes were intentionally not carried over.
Summary by CodeRabbit
New Features
Documentation
Tests