fix(livekit): deliver inbound app messages to RTVI instead of back to the client - #5224
Closed
XinZhou0417 wants to merge 2 commits into
Closed
fix(livekit): deliver inbound app messages to RTVI instead of back to the client#5224XinZhou0417 wants to merge 2 commits into
XinZhou0417 wants to merge 2 commits into
Conversation
… the client
Over LiveKitTransport, a message sent from a client to the bot never reached
RTVIProcessor. Three defects compound, the first masking the second:
push_app_message wrapped the message in a LiveKitOutputTransportMessageUrgentFrame
— an output frame. RTVIProcessor dispatches on InputTransportMessageFrame, and the
two are siblings under SystemFrame with neither extending the other, so the
isinstance check never matched. The frame rode past RTVI and was then matched by
BaseOutputTransport, which did its job and sent it back to the client.
_on_data_received forwarded the payload as a str, where
RTVIProcessor._handle_transport_message needs a mapping for
transport_message.get("label"). Unreachable while the frame class was wrong.
_async_on_data_received reported the sender as data.participant.sid, which is
carried through to publish_data(destination_identities=[...]). LiveKit resolves
that against identity, so the misrouted reply above was addressed to a
participant that does not exist and was dropped without error — and any directed
send_message on this transport has the same problem, independent of RTVI.
The effect was that client-ready never completed, so a session never became
ready, and send-text, output-mode switching and every other client-to-server
RTVI message were inoperative. Bot-to-client messaging was unaffected, so a
session looked healthy until the client tried to say something.
SmallWebRTCTransport does all three correctly; this brings LiveKit in line with
it. Fixes pipecat-ai#5218.
The four regression tests fail on the unpatched transport and pass with the fix.
The identity change is separable from the other two if you would prefer it in its
own PR.
Author
|
Parallel fix #5297 merged, hence closing this PR. |
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
Over
LiveKitTransport, a message sent from a client to the bot never reachesRTVIProcessor.client-readynever completes, so a session never becomes ready;send-text, output-mode switching and every other client-to-server RTVI message are inoperative. Bot-to-client messaging is unaffected, so a session looks healthy right up until the client tries to say something.SmallWebRTCTransporthandles this correctly. This brings LiveKit in line with it.Fixes #5218.
Cause
Three defects compound, the first masking the second.
1 —
push_app_messageemits an output frame.RTVIProcessordispatches onInputTransportMessageFrame. The two are siblings underSystemFrameand neither extends the other, so theisinstancecheck never matches. The frame rides past RTVI and is then matched byBaseOutputTransport:— which does its job and sends the client's own message back to the client.
2 — the payload is forwarded undecoded.
_on_data_receivedpassesdata.decode(), astr, whereRTVIProcessor._handle_transport_messageneeds a mapping fortransport_message.get("label"). Unreachable while defect 1 stands, and immediate once it is fixed.3 — the sender is reported as a SID.
_async_on_data_receivedpassesdata.participant.sid, which is carried through topublish_data(destination_identities=[...]). LiveKit resolves that against identity, so the misrouted reply above is addressed to a participant that does not exist and is dropped without error. This one is independent of RTVI — it makes any directedsend_messageon this transport silently undeliverable.Fix
Decode the payload, broadcast an
InputTransportMessageFrame— whatSmallWebRTCInputTransportalready does — and report the sender by identity. A non-JSON packet is logged and ignored rather than raising, since the data channel is shared.No changes to constructor arguments, defaults, or public API.
Verification
Four regression tests in
tests/test_livekit_transport.py. All four fail onmainand pass with the fix:The full file passes with the fix applied (14 tests, including the 10 already there).
Found and confirmed against a live LiveKit server before writing the patch — a real pipeline (
[transport.input(), rtvi, transport.output()]) plus a barertc.Roomclient publishing oneclient-readypacket. The runnable reproduction is in #5218. Measured there:RTVIProcessorfiredon_client_readyNote
Defect 3 is separable from the other two if you would prefer it in its own PR — say the word and I will split it out.