fix(avatar): restore the previous audio route on AvatarSession.aclose() - #7282
Open
dorukdumlu wants to merge 2 commits into
Open
dorukdumlu wants to merge 2 commits into
dorukdumlu wants to merge 2 commits into
Conversation
Avatar plugins route the agent session's audio to the avatar via output.replace_audio_tail() in start(), and aclose() never put it back. On a failed avatar start (wait_for_join timing out, the app giving up on the avatar), the session was left pointed at a dead datastream sink, so the agent went silent instead of degrading to regular audio. With the avatar started before session.start(), the stale route also made session.start() disable RoomIO audio entirely. AvatarSession now installs the route through _attach_audio_output(), which remembers what replace_audio_tail() swapped out, and aclose() restores it: the previous tail goes back under any wrapper chain, a route installed over nothing is cleared so a later session.start() sets up room audio normally, and a route someone else installed after the avatar is left alone. All in-repo avatar plugins are migrated to the helper (a mechanical one-line change each); out-of-tree plugins keep working as before, just without the restore. replace_audio_tail() now returns the sink it replaced, and AgentOutput.audio_tail exposes the current tail. Fixes livekit#7276
…-safe Two review findings. The in-repo sweep missed the one avatar that lives in core rather than livekit-plugins: the inference AvatarSession still installed its route through replace_audio_tail directly, so its aclose() had nothing recorded to restore and the dead datastream sink survived. It now goes through _attach_audio_output like every plugin. And the restore sat behind the participant-removal await, so a cancellation mid-close (a job-shutdown deadline) skipped it. The restore is synchronous and independent of the removal, so it now runs first, before any await.
Author
|
Both findings were right, addressed in acc2670. The red one was a plain miss on my side: I swept livekit-plugins for |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Avatar plugins route the agent session's audio to the avatar with
output.replace_audio_tail(...)instart(), andaclose()never put it back. When the avatar fails to start (wait_for_jointiming out, the app giving up), the session stays pointed at a dead datastream sink and the agent goes silent instead of degrading to regular audio. With the canonical ordering (avatar started beforesession.start()), the stale route also makessession.start()setroom_options.audio_output = False, so no RoomIO audio ever exists to fall back to.Fixes #7276
What changed
AvatarSessiongains_attach_audio_output(sink): it installs the route and remembers whatreplace_audio_tail()swapped out.aclose()restores it:session.start()sets up room audio normally,AgentOutput.replace_audio_tail()now returns the sink it replaced, andAgentOutput.audio_tailexposes the current tail. Both backward compatible.replace_audio_taildirectly keep working exactly as before, just without the restore.One known limit, called out rather than papered over: if the avatar dies after
session.start()already ran, restoring the route can't bring audio back, because RoomIO audio was disabled at startup and there is no_ParticipantAudioOutputto return to. Handling that would mean building the room audio output late, which felt like a separate feature. Happy to take it as a follow-up if there's interest.Test plan
tests/test_avatar_session_close.py(5 tests): restore over nothing, restore of a previous output, restore of the tail under a wrapper proxy, a newer route wins, aclose without an attach is a no-op. The restore tests fail without the fix.pytest tests/test_audio_sink_proxy.py tests/test_inference_avatar.py tests/test_recorder_io.py --unit(90 passed)