-
Notifications
You must be signed in to change notification settings - Fork 66
New Feature: Support sending files #167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5258d54
d005865
ee3a2b6
26eca6e
be5c624
7277f67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ import { | |
| import type { A11ySnapshot, UIJobGroupEnvelope } from "./ui"; | ||
|
|
||
| // Protocol 2.0.0 adds server-driven bot-output progress (spoken_progress, segment_id). | ||
| export const RTVI_PROTOCOL_VERSION = "2.1.0"; | ||
| export const RTVI_PROTOCOL_VERSION = "2.2.0"; | ||
| export const RTVI_MESSAGE_LABEL = "rtvi-ai"; | ||
|
|
||
| /** | ||
|
|
@@ -27,6 +27,7 @@ export enum RTVIMessageType { | |
| // Client-to-server messages | ||
| CLIENT_MESSAGE = "client-message", | ||
| SEND_TEXT = "send-text", | ||
| SEND_FILE = "send-file", | ||
| DTMF = "dtmf", | ||
| // UI Worker Protocol (client-to-server) | ||
| UI_EVENT = "ui-event", | ||
|
|
@@ -262,6 +263,119 @@ export type SendTextOptions = { | |
| audio_response?: boolean; | ||
| }; | ||
|
|
||
| type Serializable = | ||
| | string | ||
| | number | ||
| | boolean | ||
| | null | ||
| | Serializable[] | ||
| | { [key: number | string]: Serializable }; | ||
|
|
||
| export type RTVIImageFormat = | ||
| | "png" | ||
| | "jpg" | ||
| | "jpeg" | ||
| | "webp" | ||
| | "gif" | ||
| | "heic" | ||
| | "heif"; | ||
| export type RTVIDocFormat = | ||
| | "pdf" | ||
| | "csv" | ||
| | "txt" | ||
| | "md" | ||
| | "doc" | ||
| | "docx" | ||
| | "xls" | ||
| | "xlsx" | ||
| | "json" | ||
| | "html" | ||
| | "css" | ||
| | "javascript"; | ||
| export type RTVIMediaFormat = | ||
| | "mp3" | ||
| | "wav" | ||
| | "ogg" | ||
| | "aac" | ||
| | "mp4" | ||
| | "webm" | ||
| | "avi"; | ||
| export type RTVIFileFormat = RTVIImageFormat | RTVIDocFormat | RTVIMediaFormat; | ||
|
|
||
| export const MimeTypeMapping: Record<RTVIFileFormat, string> = { | ||
| // Images | ||
| png: "image/png", | ||
| jpg: "image/jpeg", | ||
| jpeg: "image/jpeg", | ||
| webp: "image/webp", | ||
| gif: "image/gif", | ||
| heic: "image/heic", | ||
| heif: "image/heif", | ||
| // Documents | ||
| pdf: "application/pdf", | ||
| csv: "text/csv", | ||
| txt: "text/plain", | ||
| md: "text/markdown", | ||
| doc: "application/msword", | ||
| docx: "application/vnd.openxmlformats-officedocument.wordprocessingml.document", | ||
| xls: "application/vnd.ms-excel", | ||
| xlsx: "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", | ||
| json: "application/json", | ||
| html: "text/html", | ||
| css: "text/css", | ||
| javascript: "application/javascript", | ||
| // Media | ||
| mp3: "audio/mpeg", | ||
| wav: "audio/wav", | ||
| ogg: "audio/ogg", | ||
| aac: "audio/aac", | ||
| mp4: "video/mp4", | ||
| webm: "video/webm", | ||
| avi: "video/x-msvideo", | ||
| }; | ||
|
|
||
| export type FileSourceType = "bytes" | "url" | "id"; | ||
|
|
||
| export type FileBytes = { | ||
| type: Extract<FileSourceType, "bytes">; | ||
| bytes: string; | ||
| }; | ||
|
|
||
| export type ImageFileBytes = FileBytes & { | ||
| width?: number; | ||
| height?: number; | ||
| }; | ||
|
Comment on lines
+339
to
+347
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does feels a bit odd having width and height inside Maybe do something like this: or What do you think ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i like it, though, even for images width/height is optional. TBH, maybe they can just go away? I originally had it because I thought it was required in some scenarios. In pipecat, In the RTVI processor, I just default the size to 0x0 if it's not provided. Maybe we remove this as part of the spec and always use 0x0?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be nice to provide a way to send the size to keep consistency with the server side API, even if this isn't currently needed for Pipecat.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done! |
||
| export type FileUrl = { | ||
| type: Extract<FileSourceType, "url">; | ||
| url: string | URL; | ||
| public?: boolean; | ||
| }; | ||
| export type FileId = { | ||
| type: Extract<FileSourceType, "id">; | ||
| id: string; | ||
| }; | ||
|
|
||
| export type RTVIFile = { | ||
| name?: string; | ||
| // RTVI definition takes the Mime type here, but in client-js, we support | ||
| // clients providing shorthands defined above and we map them to Mime types | ||
| format: string; | ||
| source: FileBytes | FileUrl | FileId; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In summary, we're basically sending either the file itself (as bytes), a URL to a file, or the ID of a file that was previously uploaded somewhere. Is that correct ? I don't see
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. Right now I think
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But for now, i'm explicitly not supporting it. The id must start with
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Let's keep it like this for now. We should probably just document it or create an example in |
||
| }; | ||
|
|
||
| export type SendFileOptions = { | ||
| run_immediately?: boolean; | ||
| audio_response?: boolean; | ||
| // for things like 'detail' in openAI or 'citations' in Bedrock | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a TODO to actually support. Right now, these options fall on the floor. |
||
| custom_options?: { [key: number | string]: Serializable }; | ||
| }; | ||
|
|
||
| export type FileSupport = { | ||
| formats: string[]; | ||
| sources: FileSourceType[]; | ||
| maxSize: number; // bytes | ||
| }; | ||
|
Comment on lines
+373
to
+377
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we are not using this anywhere. Should we keep it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my original spec, I wanted a way for clients to get support requirements from the server. I haven't built it out yet. So I dunno. Should I? Should I comment it and leave as a todo? The general idea was so that UIs could pre-filter out files that won't work and avoid unnecessary hefty uploads.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think so. At least it would make it clear that we are not supporting it yet. |
||
|
|
||
| /** Valid DTMF keypad keys. */ | ||
| export type DTMFButton = | ||
| | "0" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is a bit hard to follow. It is doing three things at once: normalizing the input, deciding inline vs. upload, and sending.
Would it be worth restructuring it ? Maybe something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i like it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!