Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions livekit-ffi/protocol/room.proto
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,13 @@ message AudioEncoding {
required uint64 max_bitrate = 1;
}

// A video preset describing a simulcast layer.
message VideoPreset {
required uint32 width = 1;
required uint32 height = 2;
required VideoEncoding encoding = 3;
}
Comment on lines +308 to +313

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Simulcast layers can never be sent because the option field was never added to the message definition

The new simulcast layer presets are read from the publish options message (opts.simulcast_layers at livekit-ffi/src/conversion/room.rs:357) even though no such field was added to that message definition, so clients have no way to actually supply simulcast layers.
Impact: The advertised ability to choose simulcast layers when publishing a track does not work at all.

Missing repeated VideoPreset field in TrackPublishOptions

The proto change only adds the VideoPreset message (livekit-ffi/protocol/room.proto:308-313); TrackPublishOptions (livekit-ffi/protocol/room.proto:315-336) still ends at field 13 with no repeated VideoPreset simulcast_layers = 14;. Since the Rust structs are generated from this proto at build time (livekit-ffi/src/proto.rs:18), opts.simulcast_layers does not exist and the FFI cannot receive layer presets.

Prompt for agents
The PR adds a VideoPreset proto message and consumes opts.simulcast_layers in livekit-ffi/src/conversion/room.rs, but TrackPublishOptions in livekit-ffi/protocol/room.proto was never given a corresponding field. Add a new repeated VideoPreset simulcast_layers field with the next unused field number (14) to TrackPublishOptions so the conversion has something to read, and verify prost-generated types line up with the conversion code (note prost models proto2 required message fields as Option, so proto::VideoPreset::encoding will likely need handling for the None case).
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


message TrackPublishOptions {
// encodings are optional
optional VideoEncoding video_encoding = 1;
Expand Down
14 changes: 12 additions & 2 deletions livekit-ffi/src/conversion/room.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
},
options::{
AudioEncoding, DegradationPreference, FrameMetadataFeatures, TrackPublishOptions,
VideoEncoderBackend, VideoEncoding,
VideoEncoderBackend, VideoEncoding, VideoPreset,
},
prelude::*,
webrtc::{
Expand Down Expand Up @@ -354,7 +354,11 @@
red: opts.red.unwrap_or(default_publish_options.red),
simulcast: opts.simulcast.unwrap_or(default_publish_options.simulcast),
stream: opts.stream.unwrap_or(default_publish_options.stream),
simulcast_layers: default_publish_options.simulcast_layers,
simulcast_layers: if opts.simulcast_layers.is_empty() {

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Test (aarch64-apple-darwin)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-unknown-linux-gnu)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-apple-ios)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Test (x86_64-unknown-linux-gnu)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-unknown-linux-gnu)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-linux-android)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-linux-android)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-apple-darwin)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-apple-darwin)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (armv7-linux-androideabi)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-apple-ios-sim)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Test (x86_64-pc-windows-msvc)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-pc-windows-msvc)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 357 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-pc-windows-msvc)

no field `simulcast_layers` on type `proto::TrackPublishOptions`
opts.default_publish_options.simulcast_layers

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Test (aarch64-apple-darwin)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-unknown-linux-gnu)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-apple-ios)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Test (x86_64-unknown-linux-gnu)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-unknown-linux-gnu)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-linux-android)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-linux-android)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-apple-darwin)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-apple-darwin)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (armv7-linux-androideabi)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-apple-ios-sim)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Test (x86_64-pc-windows-msvc)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-pc-windows-msvc)

no field `default_publish_options` on type `proto::TrackPublishOptions`

Check failure on line 358 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-pc-windows-msvc)

no field `default_publish_options` on type `proto::TrackPublishOptions`
} else {
Some(opts.simulcast_layers.into_iter().map(Into::into).collect())

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Test (aarch64-apple-darwin)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-unknown-linux-gnu)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-apple-ios)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Test (x86_64-unknown-linux-gnu)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-unknown-linux-gnu)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-linux-android)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-linux-android)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-apple-darwin)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-apple-darwin)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (armv7-linux-androideabi)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-apple-ios-sim)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Test (x86_64-pc-windows-msvc)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (aarch64-pc-windows-msvc)

no field `simulcast_layers` on type `proto::TrackPublishOptions`

Check failure on line 360 in livekit-ffi/src/conversion/room.rs

View workflow job for this annotation

GitHub Actions / Build (x86_64-pc-windows-msvc)

no field `simulcast_layers` on type `proto::TrackPublishOptions`
},
Comment on lines +357 to +361

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Fallback to default simulcast layers reads from the wrong value

When no simulcast layers are provided, the fallback reads the default layers off the incoming request instead of the defaults object (opts.default_publish_options.simulcast_layers at livekit-ffi/src/conversion/room.rs:358), so the default simulcast configuration cannot be resolved.
Impact: Publishing without explicit simulcast layers cannot fall back to the SDK defaults.

Wrong receiver on the default lookup

default_publish_options is a local binding created at livekit-ffi/src/conversion/room.rs:338 (TrackPublishOptions::default()); it is not a field of the proto request opts. Every other branch in this From impl uses the local (e.g. livekit-ffi/src/conversion/room.rs:353-356).

Suggested change
simulcast_layers: if opts.simulcast_layers.is_empty() {
opts.default_publish_options.simulcast_layers
} else {
Some(opts.simulcast_layers.into_iter().map(Into::into).collect())
},
simulcast_layers: if opts.simulcast_layers.is_empty() {
default_publish_options.simulcast_layers
} else {
Some(opts.simulcast_layers.into_iter().map(Into::into).collect())
},
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +357 to +361

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Pull request is missing the required change documentation entry

The change adds a new option without adding the required documentation entry for the release tooling, which the repository mandates for every pull request.
Impact: The release notes and crate version bumps will not reflect this change.

AGENTS.md changeset requirement

AGENTS.md states "Every PR needs a changeset" and that it must list crates to bump. This PR only touches livekit-ffi/protocol/room.proto and livekit-ffi/src/conversion/room.rs; no file was added under /.changeset.

Prompt for agents
Per AGENTS.md, every PR requires a changeset under /.changeset documenting the change and listing crates that need version bumps (here livekit-ffi, and livekit if public API changes). Create one with `knope document-change` or manually.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

preconnect_buffer: opts
.preconnect_buffer
.unwrap_or(default_publish_options.preconnect_buffer),
Expand All @@ -381,6 +385,12 @@
}
}

impl From<proto::VideoPreset> for VideoPreset {
fn from(preset: proto::VideoPreset) -> Self {
Self { width: preset.width, height: preset.height, encoding: preset.encoding.into() }
}
}

#[cfg(test)]
mod tests {
use livekit::options::{TrackPublishOptions, VideoEncoderBackend};
Expand Down
Loading