Write the video start bitrate hint once per connection

.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.

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<u64>,
+    // 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<SessionDescription>,
 }
 
@@ -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<Option<SessionDescription>> {
         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<u64>) {
+    pub async fn set_max_send_bitrate_bps(&self, bps: Option<u64>, 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<u64>) -> Option<u32> {
+    /// 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<u64>, is_screen_share: bool) -> Option<u32> {
         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\

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 {