AGT-2669: Add turn detection and interruption protobufs#1485
AGT-2669: Add turn detection and interruption protobufs#1485chenghao-mou wants to merge 16 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 9670cbd The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR 💥 An error occurred when fetching the changed packages and changesets in this PR |
| message TdServerMessage { | ||
| oneof message { | ||
| TdSessionCreated session_created = 1; | ||
| TdInferenceStarted inference_started = 2; | ||
| TdInferenceStopped inference_stopped = 3; | ||
| TdEouPrediction eou_prediction = 4; | ||
| TdSessionClosed session_closed = 5; | ||
| TdError error = 6; | ||
| } | ||
| optional string request_id = 7; | ||
| google.protobuf.Timestamp server_created_at = 8; | ||
| optional google.protobuf.Timestamp client_created_at = 9; | ||
| } |
There was a problem hiding this comment.
Should we split this into a Request/Response message?
There was a problem hiding this comment.
For the RemoteSession, I have Request/Response and events
https://github.com/livekit/protocol/blob/main/protobufs/agent/livekit_agent_session.proto
There was a problem hiding this comment.
I guess it depends on how you look at them: e.g. TdInferenceStart and TdInferenceStarted is a Request-and-Response pair, we just wrap all the requests and responses in TdClientMessage and TdServerMessage.
davidzhao
left a comment
davidzhao
left a comment
There was a problem hiding this comment.
is this only for the turn detector? or more designed to be the generic websocket protocol?
They are only for the turn detector. Barge-in or STT have somewhat different control messages or different formats: Bargein (ws binary+json): in: audio frame (pcm), inference start/end, out: prediction Do you think we should try to unify them? |
…om/livekit/protocol into chenghao/feat/turn-detection-proto
chenghao-mou
changed the title
chenghao-mou
changed the title
| message InferenceStats { | ||
| // server-side e2e latency (server input to server output) | ||
| google.protobuf.Duration e2e_latency = 1; | ||
| google.protobuf.Duration preprocessing_duration = 2; | ||
| google.protobuf.Duration inference_duration = 3; | ||
| } | ||
|
|
||
| message ProcessingStats { | ||
| google.protobuf.Timestamp earliest_client_created_at = 1; | ||
| google.protobuf.Timestamp latest_client_created_at = 2; | ||
| // client-side e2e latency (client send to client receive) | ||
| google.protobuf.Duration e2e_latency = 3; | ||
| InferenceStats inference_stats = 4; | ||
| } |
There was a problem hiding this comment.
maybe?
| message InferenceStats { | |
| // server-side e2e latency (server input to server output) | |
| google.protobuf.Duration e2e_latency = 1; | |
| google.protobuf.Duration preprocessing_duration = 2; | |
| google.protobuf.Duration inference_duration = 3; | |
| } | |
| message ProcessingStats { | |
| google.protobuf.Timestamp earliest_client_created_at = 1; | |
| google.protobuf.Timestamp latest_client_created_at = 2; | |
| // client-side e2e latency (client send to client receive) | |
| google.protobuf.Duration e2e_latency = 3; | |
| InferenceStats inference_stats = 4; | |
| } | |
| message ProcessingStats { | |
| message InferenceStats { | |
| // server-side e2e latency (server input to server output) | |
| google.protobuf.Duration e2e_latency = 1; | |
| google.protobuf.Duration preprocessing_duration = 2; | |
| google.protobuf.Duration inference_duration = 3; | |
| } | |
| google.protobuf.Timestamp earliest_client_created_at = 1; | |
| google.protobuf.Timestamp latest_client_created_at = 2; | |
| // client-side e2e latency (client send to client receive) | |
| google.protobuf.Duration e2e_latency = 3; | |
| InferenceStats inference_stats = 4; | |
| } |
There was a problem hiding this comment.
The model-to-Inference response needs to include InferenceStats but not ProcessingStats (EotInferenceResponse and InterruptionInferenceResponse)
| message Error { | ||
| string message = 1; | ||
| // error code follows the HTTP status code convention | ||
| // 4xx for client errors | ||
| // 5xx for server errors | ||
| uint32 code = 2; | ||
| } |
There was a problem hiding this comment.
This Error message is too generic. Can we have another name?
Fwiw, we only do a string error in other services
There was a problem hiding this comment.
The code here is the convention from Inference. We use that for the status_code in APIStatusError.
Renamed it to InferenceError.
3 participants
Add protobuf messages for both client <-> Inference and Inference <-> GPU host.