diff --git a/.changeset/video_start_bitrate_connection_level.md b/.changeset/video_start_bitrate_connection_level.md new file mode 100644 index 000000000..ff1fc181c --- /dev/null +++ b/.changeset/video_start_bitrate_connection_level.md @@ -0,0 +1,11 @@ +--- +livekit: patch +livekit-ffi: patch +livekit-capture: patch +--- + +Write the `x-google-start-bitrate` hint once per publisher connection, and exempt screen share from the 1 Mbps cap. + +libwebrtc reads this fmtp parameter per m-section but applies it to the shared `Call` (`WebRtcVideoSendChannel::ApplyChangedParams` -> `SetSdpBitrateParameters`), where `RtpBitrateConfigurator` holds one config for the whole peer connection. It retains `start_bitrate_bps` and re-applies it on network route changes (`RtpTransportControllerSend::OnNetworkRouteChanged`), so a WiFi-to-cellular handover re-seeds the estimator from the original hint with no renegotiation. Rewriting the value on later offers was therefore at best a no-op and at worst a restart of a converged bandwidth estimator; it is now written only on the first offer that carries local video, and only once that offer is accepted locally. A full reconnect builds a new peer connection and seeds the new estimator again. The initial offer sent with the JoinRequest in single PC mode never carries the hint: it is created before any track is published, so no target bitrate exists yet. + +Screen share is no longer capped at 1 Mbps, matching client-sdk-js and client-sdk-android: unlike camera content, a screen share is published at a high bitrate so text stays legible, and a conservative start costs more than a brief overshoot. diff --git a/livekit/src/rtc_engine/peer_transport.rs b/livekit/src/rtc_engine/peer_transport.rs index 4eea3df3a..9a1b09c98 100644 --- a/livekit/src/rtc_engine/peer_transport.rs +++ b/livekit/src/rtc_engine/peer_transport.rs @@ -33,6 +33,12 @@ struct TransportInner { single_pc_mode: bool, // Publish-side target bitrate (bps) for offer munging max_send_bitrate_bps: Option, + // Whether the target above belongs to a screen share, which is exempt from the + // start bitrate cap. + max_send_bitrate_is_screen_share: bool, + // Whether an offer carrying `x-google-start-bitrate` has been accepted locally. The + // hint is written once per peer connection; see `compute_start_bitrate_kbps`. + start_bitrate_applied: bool, pending_initial_offer: Option, } @@ -65,6 +71,8 @@ impl PeerTransport { restarting_ice: false, single_pc_mode, max_send_bitrate_bps: None, + max_send_bitrate_is_screen_share: false, + start_bitrate_applied: false, pending_initial_offer: None, })), } @@ -153,8 +161,18 @@ impl PeerTransport { /// The offer is stored as pending and will be applied when the server's answer arrives. /// /// In single PC mode, this initial offer is sent with the JoinRequest before any track - /// is published. We apply both `inactive→recvonly` munging and `x-google-start-bitrate` - /// munging when a target bitrate is known. + /// is published, so only the `inactive→recvonly` munging applies. + /// + /// It deliberately carries no `x-google-start-bitrate`. The hint is derived from + /// `max_send_bitrate_bps`, which only `SessionInner::create_sender` sets, when a track is + /// published — always after this runs — so there is never a target to write here. Writing + /// one would also have to consume the one-shot latch before the offer becomes the local + /// description, which happens later in `set_remote_description`, and the offer can be + /// dropped without ever being applied: when the server declines single PC mode, + /// `RtcSession` calls `clear_pending_initial_offer` and reuses this transport for the rest + /// of the session. The latch would then be spent on an offer that never existed, silently + /// skipping the hint for the life of the connection. `create_and_send_offer` owns the hint + /// and latches only once `set_local_description` has succeeded. pub async fn create_initial_offer(&self) -> EngineResult> { let inner = self.inner.lock().await; if !inner.single_pc_mode { @@ -163,39 +181,13 @@ impl PeerTransport { drop(inner); let mut offer = self.peer_connection.create_offer(OfferOptions::default()).await?; - let mut sdp = offer.to_string(); + let sdp = offer.to_string(); // Apply inactive→recvonly munging for single PC mode let recvonly_munged = Self::munge_inactive_to_recvonly_for_media(&sdp); if recvonly_munged != sdp { if let Ok(parsed) = SessionDescription::parse(&recvonly_munged, offer.sdp_type()) { offer = parsed; - sdp = recvonly_munged; - } - } - - // Apply x-google-start-bitrate munging for video codecs if we have a target bitrate. - // In initial offers (before track is published), max_send_bitrate_bps is None, - // so no munging is applied and WebRTC uses its default conservative start bitrate. - let has_video = sdp.contains(" VP8/90000") - || sdp.contains(" VP9/90000") - || sdp.contains(" AV1/90000") - || sdp.contains(" H264/90000") - || sdp.contains(" H265/90000"); - if has_video { - let start_kbps = { - let inner = self.inner.lock().await; - Self::compute_start_bitrate_kbps(inner.max_send_bitrate_bps) - }; - if let Some(start_kbps) = start_kbps { - log::info!("Initial offer: applying x-google-start-bitrate={} kbps", start_kbps); - - let munged = Self::munge_x_google_start_bitrate(&sdp, start_kbps); - if munged != sdp { - if let Ok(parsed) = SessionDescription::parse(&munged, offer.sdp_type()) { - offer = parsed; - } - } } } @@ -209,30 +201,44 @@ impl PeerTransport { inner.pending_initial_offer = None; } - pub async fn set_max_send_bitrate_bps(&self, bps: Option) { + pub async fn set_max_send_bitrate_bps(&self, bps: Option, is_screen_share: bool) { let mut inner = self.inner.lock().await; inner.max_send_bitrate_bps = bps; + inner.max_send_bitrate_is_screen_share = is_screen_share; } /// Maximum x-google-start-bitrate (kbps). /// 1 Mbps is a reasonable ceiling that prevents BWE from starting too aggressively. const MAX_START_BITRATE_KBPS: u32 = 1000; + /// Minimum target bitrate (kbps) worth hinting. Below this, seeding above the real + /// capacity costs more than the ramp it saves, so libwebrtc's default is left alone. + const MIN_TARGET_BITRATE_KBPS: u32 = 300; + /// Compute the x-google-start-bitrate value for SDP munging. /// - /// Returns min(90% of target, 1 Mbps). Returns None if no target bitrate is set - /// (initial offer before track publish) or if the target is too low. - fn compute_start_bitrate_kbps(target_bps: Option) -> Option { + /// 90% of the target leaves ~10% headroom for the estimator to settle. The same + /// multiplier is used for every codec because the target already reflects the codec's + /// efficiency. Camera is capped at [`Self::MAX_START_BITRATE_KBPS`] so the estimator does + /// not open too aggressively on a high-bitrate track; screen share is exempt, because its + /// content needs the bitrate immediately to stay legible. + /// + /// Returns None if no target bitrate is set (initial offer before track publish) or if + /// the target is below [`Self::MIN_TARGET_BITRATE_KBPS`]. + fn compute_start_bitrate_kbps(target_bps: Option, is_screen_share: bool) -> Option { let target_bps = target_bps?; let target_kbps = (target_bps / 1000) as u32; - if target_kbps == 0 || target_kbps < 300 { + if target_kbps < Self::MIN_TARGET_BITRATE_KBPS { return None; } - // Use 90% of target bitrate as start bitrate, capped at 1 Mbps let start_kbps = (target_kbps as f64 * 0.9).round() as u32; - Some(start_kbps.min(target_kbps).min(Self::MAX_START_BITRATE_KBPS)) + if is_screen_share { + Some(start_kbps.min(target_kbps)) + } else { + Some(start_kbps.min(target_kbps).min(Self::MAX_START_BITRATE_KBPS)) + } } /// Munge SDP to change a=inactive to a=recvonly for RTP media m-lines in single PC mode. @@ -511,14 +517,29 @@ impl PeerTransport { } // Apply x-google-start-bitrate for all video codecs to improve initial quality. - // Uses min(90% of target, 1 Mbps) to prevent BWE from starting too aggressively. + // + // The same value goes on every video codec, and only on the first offer that carries + // local video. libwebrtc reads this fmtp parameter per m-section but applies it to the + // shared Call (`WebRtcVideoSendChannel::ApplyChangedParams` -> `SetSdpBitrateParameters`), + // where `RtpBitrateConfigurator` keeps one config for the whole peer connection: it + // retains `start_bitrate_bps` and re-applies it on network route changes, so rewriting + // it later is at best a no-op and at worst restarts a converged bandwidth estimator. A + // full reconnect builds a new peer connection and seeds the new estimator again. + // + // x-google-max-bitrate is deliberately never written: the same Call-level promotion + // would turn a per-track cap into a ceiling on total send bandwidth, starving concurrent + // tracks. Per-track and per-layer caps belong in the encodings' max_bitrate. + let mut applied_start_bitrate = false; let has_video = sdp.contains(" VP8/90000") || sdp.contains(" VP9/90000") || sdp.contains(" AV1/90000") || sdp.contains(" H264/90000") || sdp.contains(" H265/90000"); - if has_video { - if let Some(start_kbps) = Self::compute_start_bitrate_kbps(inner.max_send_bitrate_bps) { + if has_video && !inner.start_bitrate_applied { + if let Some(start_kbps) = Self::compute_start_bitrate_kbps( + inner.max_send_bitrate_bps, + inner.max_send_bitrate_is_screen_share, + ) { log::info!( "Applying x-google-start-bitrate={} kbps (target_bps={:?})", start_kbps, @@ -529,7 +550,10 @@ impl PeerTransport { if munged != sdp { log::debug!("SDP munged successfully for video codec"); match SessionDescription::parse(&munged, offer.sdp_type()) { - Ok(parsed) => offer = parsed, + Ok(parsed) => { + offer = parsed; + applied_start_bitrate = true; + } Err(e) => log::warn!( "Failed to parse munged SDP, falling back to original offer: {e}" ), @@ -542,6 +566,11 @@ impl PeerTransport { self.peer_connection.set_local_description(offer.clone()).await?; + // Only consume the one-shot hint once the offer carrying it is accepted locally. + if applied_start_bitrate { + inner.start_bitrate_applied = true; + } + if let Some(handler) = self.on_offer_handler.lock().as_mut() { handler(offer); } @@ -664,6 +693,49 @@ a=fmtp:111 minptime=10;useinbandfec=1\n"; assert_eq!(out, sdp, "should not change SDP if no video codec present"); } + #[test] + fn start_bitrate_is_ninety_percent_of_target() { + assert_eq!( + PeerTransport::compute_start_bitrate_kbps(Some(1_000_000), false), + Some(900), + "should be 90% of a 1 Mbps target" + ); + } + + #[test] + fn start_bitrate_caps_camera_but_not_screen_share() { + // 90% of 3 Mbps is 2700, above the 1 Mbps camera ceiling. + assert_eq!( + PeerTransport::compute_start_bitrate_kbps(Some(3_000_000), false), + Some(PeerTransport::MAX_START_BITRATE_KBPS), + "camera should be capped" + ); + assert_eq!( + PeerTransport::compute_start_bitrate_kbps(Some(3_000_000), true), + Some(2700), + "screen share should not be capped; its content needs the bitrate to stay legible" + ); + } + + #[test] + fn start_bitrate_skipped_below_target_floor() { + assert_eq!( + PeerTransport::compute_start_bitrate_kbps(Some(299_000), false), + None, + "below the floor libwebrtc's own default is left in place" + ); + assert_eq!( + PeerTransport::compute_start_bitrate_kbps(Some(300_000), false), + Some(270), + "at the floor the hint applies" + ); + assert_eq!( + PeerTransport::compute_start_bitrate_kbps(None, false), + None, + "no target means no hint (initial offer before any publish)" + ); + } + #[test] fn vp8_with_fmtp_appends_start_bitrate() { let sdp = "v=0\n\ diff --git a/livekit/src/rtc_engine/rtc_session.rs b/livekit/src/rtc_engine/rtc_session.rs index b5af188a9..611c6ba5d 100644 --- a/livekit/src/rtc_engine/rtc_session.rs +++ b/livekit/src/rtc_engine/rtc_session.rs @@ -60,7 +60,7 @@ use crate::{ peer_transport::PeerTransport, rtc_events::{RtcEvent, RtcEvents}, }, - track::LocalTrack, + track::{LocalTrack, TrackSource}, DataPacketKind, }; @@ -1939,7 +1939,8 @@ impl SessionInner { let sum: u64 = encodings.iter().filter_map(|e| e.max_bitrate).sum(); (sum > 0).then_some(sum) }; - self.publisher_pc.set_max_send_bitrate_bps(ultimate_bps).await; + let is_screen_share = options.source == TrackSource::Screenshare; + self.publisher_pc.set_max_send_bitrate_bps(ultimate_bps, is_screen_share).await; } let init = RtpTransceiverInit {