-
Notifications
You must be signed in to change notification settings - Fork 196
Move data streams into their own crate #1227
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
Changes from all commits
452024f
6904329
2400b0f
4d0c8f4
2168702
7a5f865
4492d6e
4abf39b
0172bfb
5ecee43
433b85d
1a0b979
136124f
ee02f26
4445b08
8e956b4
9141c9b
bf24f85
d2ae930
34661ad
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 |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| livekit: patch | ||
| livekit-ffi: patch | ||
| --- | ||
|
|
||
| refactor: extract data-stream logic and shared types into new `livekit-common` and `livekit-data-stream` crates (public API unchanged; types are re-exported from `livekit`) |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| # AGENTS.md | ||
|
|
||
| ## Architectural overview | ||
|
|
||
| - Holds foundational, broadly-shared items used across multiple LiveKit crates (e.g. [`livekit`](../livekit/), [`livekit-api`](../livekit-api/), [`livekit-data-stream`](../livekit-data-stream/)) | ||
| - Internal crate — not for direct consumption by developers (public APIs live in the [`livekit`](../livekit/) crate) | ||
| - Exists purely to avoid duplication and circular dependencies: an item needed by two or more downstream crates lives here instead of in any single one | ||
| - Current contents set the bar for what fits: `ParticipantIdentity` (newtype), `EncryptionType` (enum + proto conversions), the `CLIENT_PROTOCOL_*` constants, and the `enum_dispatch!` macro | ||
|
|
||
| ## What belongs here | ||
|
|
||
| - Small, self-contained, foundational items shared by **two or more** downstream crates: | ||
| - Newtypes and plain data enums (plus their `From`/`TryFrom` conversions) | ||
| - Simple constants | ||
| - Trivial, stateless helper functions and declarative macros | ||
| - Every addition must be dependency-light (see Dependencies) and free of feature/business logic | ||
|
|
||
| ## What does NOT belong here | ||
|
|
||
| - Feature or business logic — keep it in the feature's own crate (e.g. `livekit-data-stream`) or in `livekit` | ||
| - Items used by only **one** crate — leave them in that crate until a second consumer actually needs them; do not hoist here speculatively | ||
| - Stateful components — managers, actors, services, or anything holding runtime state | ||
| - Wire/protocol types — those belong in `livekit-protocol` (this crate depends on it, never the reverse) | ||
| - Anything that would require a heavy or environment-specific dependency (see Dependencies) | ||
|
|
||
| ## Dependencies | ||
|
|
||
| - Keep the dependency list minimal — today it is only `livekit-protocol` | ||
| - A dependency added here is forced onto **every** downstream crate; treat any new dependency as a red flag and justify it explicitly | ||
| - Never pull in heavy or environment-specific deps (`libwebrtc`, an async runtime, networking, etc.) — a type that needs those belongs in a higher-level crate |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| [package] | ||
| name = "livekit-common" | ||
| description = "Common foundational types shared across LiveKit crates" | ||
| version = "0.1.0" | ||
| readme = "README.md" | ||
| license.workspace = true | ||
| edition.workspace = true | ||
| repository.workspace = true | ||
|
|
||
| [dependencies] | ||
| livekit-protocol = { workspace = true } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # LiveKit Common | ||
|
|
||
| An internal crate which holds shared data structures that many downstream modules all use, like | ||
| `ParticipantIdentity` or `EncryptionType`. | ||
|
|
||
| To build applications with LiveKit, please use the public APIs provided by the [livekit](../livekit) crate. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| // Copyright 2026 LiveKit, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| /// Generates methods on an enum that forward each call to the inner value of every variant. | ||
| /// | ||
| /// Given a list of variants and a set of method signatures, this expands to a `match` over `self` | ||
| /// that dispatches to the identically-named method on each variant's inner type, saving the | ||
| /// boilerplate of writing one `match` arm per variant per method. | ||
| /// | ||
| /// ```ignore | ||
| /// impl AnyStreamInfo { | ||
| /// enum_dispatch!( | ||
| /// [Byte, Text]; | ||
| /// pub fn id(self: &Self) -> &str; | ||
| /// pub fn total_length(self: &Self) -> Option<u64>; | ||
| /// ); | ||
| /// } | ||
| /// ``` | ||
| // TODO(theomonnom): Async methods | ||
| #[macro_export] | ||
| macro_rules! enum_dispatch { | ||
| // This arm is used to avoid nested loops with the arguments | ||
| // The arguments are transformed to $combined_args tt | ||
| (@match [$($variant:ident),+]: $fnc:ident, $self:ident, $combined_args:tt) => { | ||
| match $self { | ||
| $( | ||
| Self::$variant(inner) => inner.$fnc$combined_args, | ||
| )+ | ||
| } | ||
| }; | ||
|
|
||
| // Create the function and extract self fron the $args tt (little hack) | ||
| (@fnc [$($variant:ident),+]: $vis:vis fn $fnc:ident($self:ident: $sty:ty $(, $arg:ident: $t:ty)*) -> $ret:ty) => { | ||
| #[inline] | ||
| $vis fn $fnc($self: $sty, $($arg: $t),*) -> $ret { | ||
| $crate::enum_dispatch!(@match [$($variant),+]: $fnc, $self, ($($arg,)*)) | ||
| } | ||
| }; | ||
|
|
||
| ($variants:tt; $($vis:vis fn $fnc:ident$args:tt -> $ret:ty;)+) => { | ||
| $( | ||
| $crate::enum_dispatch!(@fnc $variants: $vis fn $fnc$args -> $ret); | ||
| )+ | ||
| }; | ||
| } | ||
|
1egoman marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| // Copyright 2026 LiveKit, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| //! Foundational types shared across LiveKit crates: participant identities, the | ||
| //! encryption enum, and client-protocol constants. | ||
|
|
||
| use std::fmt::Display; | ||
|
|
||
| use livekit_protocol as proto; | ||
|
|
||
| mod enum_dispatch; | ||
|
|
||
| // ------------------------------------------------------------------------------------------------- | ||
| // Client protocol | ||
| // ------------------------------------------------------------------------------------------------- | ||
|
|
||
| /// Legacy client. | ||
| pub const CLIENT_PROTOCOL_DEFAULT: i32 = 0; | ||
|
|
||
| /// RPC v2 (see RPC spec). | ||
| pub const CLIENT_PROTOCOL_DATA_STREAM_RPC: i32 = 1; | ||
|
|
||
| // ------------------------------------------------------------------------------------------------- | ||
| // ParticipantIdentity | ||
|
1egoman marked this conversation as resolved.
|
||
| // ------------------------------------------------------------------------------------------------- | ||
|
|
||
| #[derive(Clone, Default, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] | ||
| pub struct ParticipantIdentity(pub String); | ||
|
|
||
| impl From<String> for ParticipantIdentity { | ||
| fn from(value: String) -> Self { | ||
| Self(value) | ||
| } | ||
| } | ||
|
|
||
| impl From<&str> for ParticipantIdentity { | ||
| fn from(value: &str) -> Self { | ||
| Self(value.to_string()) | ||
| } | ||
| } | ||
|
|
||
| impl From<ParticipantIdentity> for String { | ||
| fn from(value: ParticipantIdentity) -> Self { | ||
| value.0 | ||
| } | ||
| } | ||
|
|
||
| impl Display for ParticipantIdentity { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| write!(f, "{}", self.0) | ||
| } | ||
| } | ||
|
|
||
| impl ParticipantIdentity { | ||
| pub fn as_str(&self) -> &str { | ||
| &self.0 | ||
| } | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------------------------------------- | ||
| // EncryptionType | ||
| // ------------------------------------------------------------------------------------------------- | ||
|
|
||
| #[derive(Default, Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum EncryptionType { | ||
| #[default] | ||
| None, | ||
| Gcm, | ||
| Custom, | ||
| } | ||
|
|
||
| impl From<proto::encryption::Type> for EncryptionType { | ||
| fn from(value: proto::encryption::Type) -> Self { | ||
| match value { | ||
| proto::encryption::Type::None => Self::None, | ||
| proto::encryption::Type::Gcm => Self::Gcm, | ||
| proto::encryption::Type::Custom => Self::Custom, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl From<EncryptionType> for proto::encryption::Type { | ||
| fn from(value: EncryptionType) -> Self { | ||
| match value { | ||
| EncryptionType::None => Self::None, | ||
| EncryptionType::Gcm => Self::Gcm, | ||
| EncryptionType::Custom => Self::Custom, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl From<EncryptionType> for i32 { | ||
| fn from(value: EncryptionType) -> Self { | ||
| match value { | ||
| EncryptionType::None => 0, | ||
| EncryptionType::Gcm => 1, | ||
| EncryptionType::Custom => 2, | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| [package] | ||
| name = "livekit-data-stream" | ||
| description = "Data stream core logic for LiveKit" | ||
| version = "0.1.0" | ||
| readme = "README.md" | ||
| license.workspace = true | ||
| edition.workspace = true | ||
| repository.workspace = true | ||
|
|
||
| [features] | ||
| # Exposes test-only hooks used by downstream crates' test suites (e.g. | ||
| # `TextStreamReader::new_for_test`). Forwarded from the `livekit` crate's `__lk-e2e-test` feature. | ||
| test-utils = [] | ||
|
Comment on lines
+9
to
+13
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. Note to reviewers: should these be separate features or just one?
Contributor
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. If we need to keep them, they should be separate (I plan to eventually drop the special feature for E2E testing and instead just skip E2E tests based on env). However, there's the bigger question of if we should be using non-public methods in integration tests in the first place.
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.
Right now, that's how it works so I don't think this makes it worse. IMO I'd rather decouple that conversation and revisit as part of the data streams v2 pull request since I will be making some more extensive updates there.
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've combined them into one feature for now, |
||
|
|
||
| [dependencies] | ||
| livekit-common = { workspace = true } | ||
| livekit-protocol = { workspace = true } | ||
| log = { workspace = true } | ||
| thiserror = { workspace = true } | ||
| parking_lot = { workspace = true } | ||
| bytes = { workspace = true } | ||
| tokio = { workspace = true, default-features = false, features = ["sync", "fs", "io-util", "rt"] } | ||
|
Contributor
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. note: Outside the scope of this PR and a pre-existing issue, but we should only depend on tokio with the sync feature unconditionally.
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. Good to know, I will investigate in the |
||
| futures-util = { workspace = true, default-features = false, features = ["sink"] } | ||
| prost = "0.12" | ||
| chrono = "0.4.38" | ||
| flate2 = "1" | ||
| bmrng = "0.5.2" | ||
| uuid = { version = "1", features = ["v4"] } | ||
|
|
||
| [dev-dependencies] | ||
| tokio = { workspace = true, default-features = false, features = ["macros", "rt", "rt-multi-thread", "time"] } | ||
Uh oh!
There was an error while loading. Please reload this page.