Conversation
c767c0f to
ecdbb87
Compare
The agent must know if a client can rebuild transcriptions from data streams. A client tells the other participants about this ability with a client protocol number. Client protocol 3 means that the client rebuilds transcription events from the `lk.transcription` text streams. Such a client ignores the deprecated `rtc.Transcription` data packet. This commit adds the `CLIENT_PROTOCOL_TRANSCRIPTION_STREAMS` constant. The client SDKs set the value of this constant. This repository does not set it. This commit also adds the `_client_protocol` function. The function reads the number from a participant. The function returns 0 if the number is not available. A value of 0 means a legacy client. The function reads the private `_info` field on purpose. The client protocol is an internal signal between participants. It is not part of the public participant API.
The agent sends each transcription two times. It sends a deprecated `rtc.Transcription` data packet. It also sends an `lk.transcription` text stream. The two copies fill the reliable data channel. Other reliable traffic becomes slow. This commit stops the legacy packet when no client needs it. Before each legacy publish, the agent examines the remote participants. If all applicable participants have client protocol 3 or higher, the agent does not send the packet. The agent always sends the text stream. Only STANDARD participants are applicable. These are the client SDK instances that users create. SIP, INGRESS, AGENT, CONNECTOR and BRIDGE participants do not show legacy transcripts. EGRESS participants are hidden and do not appear in the participant list. The agent also ignores its own avatar worker. The gate is in `_publish_transcription`. This is the only function that sends the packet. Both `capture_text` and `flush` use this function. Do not move the gate into `capture_text`. Each legacy packet contains the full segment text with a stable identifier. The accumulated text must stay correct. If a legacy client joins during a segment, the next packet gives it the full segment. The agent calculates the result again for each publish. Do not put the result in a cache. The SDK changes the participant data without a Python event. A cache can keep an incorrect result for the full session. The `_legacy_status_logged` field does not control the gate. It holds only the last status written to the log. The code writes a new log message when the status changes. This prevents a log message for each transcription chunk.
This commit adds three test helpers. It adds no tests. `_fake_remote` makes a stand-in for a remote participant. The `attributes` field must be a true dictionary, because the gate calls `get` on it. The `client_protocol` value is on the `_info` field. This is the same shape as a livekit-rtc participant. `_make_legacy_output` makes a legacy output and sets its fields directly. Do not use `set_participant` here. That function reads `track_publications`, and the fakes do not have this attribute. `_capture_and_flush` sends text through an output and then waits for the flush task.
This commit adds three tests for the client protocol comparison. The first test shows that the agent sends no legacy packet when all clients have client protocol 3. The second test shows that the agent sends the legacy packets when one client is older. The test also examines the packets. The partial packet and the final packet must have the same segment identifier and the full text. The third test shows that an unknown client protocol counts as a legacy client. The agent then sends the packet. This is the safe result.
This commit adds six tests. These tests hold the rules about applicable participants. Change these rules only with care. Two tests show that the agent sends no legacy packet when the room has no STANDARD participant. A room with only a SIP participant gives the same result, because a SIP participant does not show transcripts. One test shows that SIP, INGRESS, AGENT and CONNECTOR participants do not control the result. A second test shows that a STANDARD participant with an old client protocol does control the result. Two tests show the avatar worker rules. The agent ignores its own avatar worker. The agent does not ignore the avatar worker of a different agent. The last test is a guard. The exclusion must not use `_is_local_proxy_participant`. That function also agrees with the participant of the output. For the user output, that participant is the user. The agent must not stop the transcriptions of the user.
This commit adds two tests. The first test shows that the gate obeys changes in the room. All clients are modern at the start, and the agent sends no packet for the first text. A legacy client then joins. The agent sends one packet for the second text. This packet contains the full text of both parts, not only the second part. The final packet has the same segment identifier. This test fails if a person moves the gate into `capture_text`. The second test shows that the `lk.transcription` text stream does not change. The stream always goes to all participants.
ecdbb87 to
66ee8de
Compare
| logger.debug( | ||
| "legacy transcription publishing %s", | ||
| "enabled" if needed else "disabled", | ||
| extra={"participant": self._participant_identity}, | ||
| ) |
There was a problem hiding this comment.
Ah, interesting - should I get rid of this logging behavior all together? Or if not - I'm assuming that in other places participant ids are logged within the framework's logs. Are there any templates which I can follow on how best to handle this?
xianshijing-lk
left a comment
There was a problem hiding this comment.
one question, lgtm if you address it.
| reach ``remote_participants``. | ||
| """ | ||
| local_identity = room.local_participant.identity | ||
| for p in room.remote_participants.values(): |
There was a problem hiding this comment.
how often this _legacy_transcription_needed will be called ?
If it is called frequently, can we improve the code to reduce the overhead ?
There was a problem hiding this comment.
This is called on every transcription event, which in practice for most agent interactions will be under 10 times a second. This function does have an O(n) loop, but n is fairly small (n is ~number of remote participants, so for most agent interactions, this will probably be 1), and the loop has an early bail out if client protocol is under 3 for any participant so it's likely n will be smaller in practice for larger rooms.
Given this context - Is there something you have in mind here to reduce overhead? I'm not sure there's an obvious lever I am seeing which would cause a significant impact.
| capability signal used for feature detection between participants, not part of the | ||
| public participant API. | ||
| """ | ||
| value = getattr(getattr(participant, "_info", None), "client_protocol", 0) |
There was a problem hiding this comment.
any chance that we can add some tests to make sure _info and proto filed never gets renamed or changed ?
otherwise the value will default to 0 and fail silently.
There was a problem hiding this comment.
I did some thinking about this, and I think in practice actually this was more complicated than it needs to be. All modern participant implementations should contain ._info.client_protocol as this generated directly from protobuf. So this "fallback to zero of no field exists" logic makes more sense to be handled at the protobuf level.
The minor risk worth mentioning: in theory if somebody were to use an older livekit package version than is mentioned in pyproject.toml, this would throw a hard error. But this seems like a pretty unlikely thing to occur, and even if so, the client_protocol field has been around for almost a year at this point.
Given this I've dropped this _client_protocol function and inlined all participant._info.client_protocol accesses where each previous call site was located in 970231a.
The `_client_protocol` function used `getattr` with a default value of 0. If livekit-rtc renames or removes the `_info.client_protocol` field, the function gives 0 for each participant. The agent then treats all clients as legacy clients and continues to send the legacy packets. Nothing fails, and no test finds the problem. This commit reads the field directly. The field is `required` in the FFI protobuf, and the `livekit` dependency has an exact version, so the field is always present. A rename now causes an immediate error. The function had only one caller, and the body became one line. This commit removes the function and puts the read in `_legacy_transcription_needed`. The reasons for the direct read are now in the docstring of that function. This commit also removes the test for an unknown client protocol. That condition can no longer occur.
The agent sends each transcription two times. It sends a deprecated `Transcription` data packet. It also sends an `lk.transcription` text stream. The two copies fill the reliable data channel. Other reliable traffic becomes slow. This commit stops the legacy packet when no client needs it. Before each legacy publish, the agent examines the remote participants. If all applicable participants have client protocol 3 or higher, the agent does not send the packet. The agent always sends the text stream. Only STANDARD participants are applicable. These are the client SDK instances that users create. SIP, INGRESS, AGENT, CONNECTOR and BRIDGE participants do not show legacy transcripts. EGRESS participants are hidden and do not appear in the participant list. The agent also ignores its own avatar worker. An absent client protocol counts as 0, which means a legacy client. This decision is necessary, not defensive. protobuf-es gives the field the optional type, and in JavaScript all comparisons with `undefined` are false. Without the coalesce, a missing field reads as a modern client, and the agent stops the transcripts of a client that still needs them. The avatar worker test compares two values only when the attribute is present. An absent attribute and an absent local identity are both `undefined`. A direct comparison is true for each participant that has no such attribute. The gate is in `publishTranscription`. This is the only function that sends the packet. Both `handleCaptureText` and `handleFlush` use this function. Do not move the gate into `handleCaptureText`. Each legacy packet contains the full segment text with a stable identifier. The accumulated text must stay correct. If a legacy client joins during a segment, the next packet gives it the full segment. The agent calculates the result again for each publish. Do not put the result in a cache. `legacyStatusLogged` holds only the last status written to the log. It prevents a log message for each transcription chunk. This is a port of livekit/agents#7240 to node.
The agent sends each transcription two times. It sends a deprecated `Transcription` data packet. It also sends an `lk.transcription` text stream. The two copies fill the reliable data channel. Other reliable traffic becomes slow. This commit stops the legacy packet when no client needs it. Before each legacy publish, the agent examines the remote participants. If all applicable participants have client protocol 3 or higher, the agent does not send the packet. The agent always sends the text stream. Only STANDARD participants are applicable. These are the client SDK instances that users create. SIP, INGRESS, AGENT, CONNECTOR and BRIDGE participants do not show legacy transcripts. EGRESS participants are hidden and do not appear in the participant list. The agent also ignores its own avatar worker. An absent client protocol counts as 0, which means a legacy client. This decision is necessary, not defensive. protobuf-es gives the field the optional type, and in JavaScript all comparisons with `undefined` are false. Without the coalesce, a missing field reads as a modern client, and the agent stops the transcripts of a client that still needs them. The avatar worker test compares two values only when the attribute is present. An absent attribute and an absent local identity are both `undefined`. A direct comparison is true for each participant that has no such attribute. The gate is in `publishTranscription`. This is the only function that sends the packet. Both `handleCaptureText` and `handleFlush` use this function. Do not move the gate into `handleCaptureText`. Each legacy packet contains the full segment text with a stable identifier. The accumulated text must stay correct. If a legacy client joins during a segment, the next packet gives it the full segment. The agent calculates the result again for each publish. Do not put the result in a cache. `legacyStatusLogged` holds only the last status written to the log. It prevents a log message for each transcription chunk. This is a port of livekit/agents#7240 to node.
The agents part of livekit/client-sdk-js#2093. A high level summary of the transcription deduplication project:
In livekit/client-sdk-js#2093, I added a new client protocol of 3. Any client which advertises this new client protocol version ignores all legacy transcriptions, and back-converts modern transcriptions into in memory legacy transcriptions.
This pull request implements the converse of this behavior on the agent end: if any connected client sdk advertises support for the client protocol of 3, then the agents sdk now will skip sending legacy transcriptions to all participants.
A few important caveats:
destination_identitieswouldn't only deliver the packet to the listed participants.