From d0413db864a59f30b88b753605b7cb62a6d3347d Mon Sep 17 00:00:00 2001 From: shijing xian Date: Tue, 7 Jul 2026 15:37:23 +0800 Subject: [PATCH 1/8] fix x-google-start-bitrate for vp8 and other codex and make it adaptive based on network conditions --- livekit-api/src/signal_client/mod.rs | 2 +- .../src/signal_client/signal_stream.rs | 15 +- livekit-runtime/src/dispatcher.rs | 11 ++ livekit/src/rtc_engine/mod.rs | 61 +++++- livekit/src/rtc_engine/peer_transport.rs | 181 ++++++++++++++++-- livekit/src/rtc_engine/rtc_session.rs | 107 ++++++++++- 6 files changed, 359 insertions(+), 18 deletions(-) diff --git a/livekit-api/src/signal_client/mod.rs b/livekit-api/src/signal_client/mod.rs index e2448d1d1..a6acebd4d 100644 --- a/livekit-api/src/signal_client/mod.rs +++ b/livekit-api/src/signal_client/mod.rs @@ -148,7 +148,7 @@ impl Default for SignalOptions { auto_subscribe: true, adaptive_stream: false, sdk_options: SignalSdkOptions::default(), - single_peer_connection: false, + single_peer_connection: true, connect_timeout: SIGNAL_CONNECT_TIMEOUT, } } diff --git a/livekit-api/src/signal_client/signal_stream.rs b/livekit-api/src/signal_client/signal_stream.rs index 7394b86a0..d9f7b71a1 100644 --- a/livekit-api/src/signal_client/signal_stream.rs +++ b/livekit-api/src/signal_client/signal_stream.rs @@ -407,7 +407,20 @@ impl SignalStream { } Ok(Message::Frame(_)) => {} Err(WsError::Protocol(ProtocolError::ResetWithoutClosingHandshake)) => { - break; // Ignore + log::debug!("websocket reset without closing handshake"); + break; // Treat as normal close + } + Err(WsError::Io(ref io_err)) + if io_err.kind() == std::io::ErrorKind::UnexpectedEof => + { + // TLS connection closed without close_notify - treat as normal close + // This happens when the server closes the connection abruptly + log::debug!("websocket closed with unexpected EOF (TLS close without close_notify)"); + break; + } + Err(WsError::ConnectionClosed) | Err(WsError::AlreadyClosed) => { + log::debug!("websocket connection already closed"); + break; } _ => { log::error!("unhandled websocket message {:?}", msg); diff --git a/livekit-runtime/src/dispatcher.rs b/livekit-runtime/src/dispatcher.rs index 024a4e89f..eee1acc9e 100644 --- a/livekit-runtime/src/dispatcher.rs +++ b/livekit-runtime/src/dispatcher.rs @@ -62,6 +62,17 @@ where JoinHandle { task: Some(task) } } +impl JoinHandle { + /// Aborts the task associated with this handle. + /// + /// The task will be cancelled at the next await point. + pub fn abort(&self) { + if let Some(task) = &self.task { + task.cancel(); + } + } +} + impl Future for JoinHandle { type Output = T; diff --git a/livekit/src/rtc_engine/mod.rs b/livekit/src/rtc_engine/mod.rs index a1a26f4ae..8bbcd094c 100644 --- a/livekit/src/rtc_engine/mod.rs +++ b/livekit/src/rtc_engine/mod.rs @@ -908,6 +908,34 @@ impl EngineInner { let mut resuming_emitted = false; let mut restarting_emitted = false; + // # Set network hint on reconnection trigger + // + // The need to reconnect itself is a strong signal of network instability. Rather + // than waiting for slow connection detection (>5 seconds), we proactively set + // a conservative bitrate hint immediately. + // + // This helps in two scenarios: + // 1. Resume path: The hint is applied to subsequent offers on the SAME session + // 2. Full restart path: The old session's hint is lost, but try_restart_connection() + // passes the hint to the new session via connect_with_hint() + // + // Note: For the resume path, setting the hint here is sufficient. For full restart, + // this hint on the old session is lost (new session is created), so we also pass + // the hint via connect_with_hint() in try_restart_connection(). + { + let session = self.running_handle.read().session.clone(); + log::info!( + "Reconnection triggered: setting constrained network hint to {} kbps", + rtc_session::CONSTRAINED_NETWORK_START_BITRATE_KBPS + ); + session + .publisher() + .set_network_quality_start_bitrate_kbps(Some( + rtc_session::CONSTRAINED_NETWORK_START_BITRATE_KBPS, + )) + .await; + } + for i in 1..=RECONNECT_ATTEMPTS { let (is_closed, full_reconnect) = { let running_handle = self.running_handle.read(); @@ -1052,8 +1080,37 @@ impl EngineInner { let _ = engine_task.await; } - let (new_session, join_response, session_events) = - RtcSession::connect(url, token, options, e2ee_manager).await?; + // # Problem: Network hint is lost during full restart + // + // A full restart creates a completely NEW RtcSession, which means: + // - New PeerTransport with network_quality_start_bitrate_kbps = None + // - Any hint set on the old session is lost + // - The first offer of the new session would use aggressive bitrate (e.g., 4500 kbps) + // - On poor networks, this overwhelms the connection and causes immediate failure + // + // # Solution: Pass hint to new session via connect_with_hint() + // + // Since a full restart is triggered by connection failures (which often indicate + // poor network conditions), we proactively pass CONSTRAINED_NETWORK_START_BITRATE_KBPS + // (300 kbps) to the new session. This ensures: + // 1. The initial offer uses conservative bitrate + // 2. Track republishing after restart also uses conservative bitrate + // 3. WebRTC's BWE can ramp up if the network improves + // + // This is more reliable than waiting for slow connection detection (which only + // triggers after 5+ seconds of connection time). + log::info!( + "Full restart: creating new session with network hint {} kbps", + rtc_session::CONSTRAINED_NETWORK_START_BITRATE_KBPS + ); + let (new_session, join_response, session_events) = RtcSession::connect_with_hint( + url, + token, + options, + e2ee_manager, + Some(rtc_session::CONSTRAINED_NETWORK_START_BITRATE_KBPS), + ) + .await?; // On SignalRestarted, the room will try to unpublish the local tracks // NOTE: Doing operations that use rtc_session will not use the new one diff --git a/livekit/src/rtc_engine/peer_transport.rs b/livekit/src/rtc_engine/peer_transport.rs index 18a1285e9..8ad3811fa 100644 --- a/livekit/src/rtc_engine/peer_transport.rs +++ b/livekit/src/rtc_engine/peer_transport.rs @@ -34,6 +34,8 @@ struct TransportInner { // Publish-side target bitrate (bps) for offer munging max_send_bitrate_bps: Option, pending_initial_offer: Option, + // Network quality hint: caps x-google-start-bitrate on poor networks + network_quality_start_bitrate_kbps: Option, } pub struct PeerTransport { @@ -66,6 +68,7 @@ impl PeerTransport { single_pc_mode, max_send_bitrate_bps: None, pending_initial_offer: None, + network_quality_start_bitrate_kbps: None, })), } } @@ -147,20 +150,67 @@ impl PeerTransport { /// Create an initial offer without setting it as local description. /// The offer is stored as pending and will be applied when the server's answer arrives. + /// + /// # Problem Solved + /// + /// In single PC mode, this initial offer is sent with the JoinRequest before any track + /// is published. Previously, this function only applied `inactive→recvonly` munging but + /// NOT the `x-google-start-bitrate` munging that `create_and_send_offer()` applies. + /// + /// This was a problem during full restart after network issues: + /// 1. Full restart creates a new session with network_hint=300 kbps + /// 2. Initial offer is created here (with recv-only video transceivers) + /// 3. The SDP contains video codecs (VP8/90000 etc.) from recv-only transceivers + /// 4. But without bitrate munging, the first offer after restart didn't get the hint + /// + /// Now we apply the same x-google-start-bitrate munging as `create_and_send_offer()`, + /// so the initial offer respects the network quality hint if one is set. pub async fn create_initial_offer(&self) -> EngineResult> { let mut inner = self.inner.lock().await; if !inner.single_pc_mode { return Ok(None); } - let offer = self.peer_connection.create_offer(OfferOptions::default()).await?; - let sdp = offer.to_string(); + let mut offer = self.peer_connection.create_offer(OfferOptions::default()).await?; + let mut 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()) { - inner.pending_initial_offer = Some(parsed.clone()); - return Ok(Some(parsed)); + offer = parsed; + sdp = recvonly_munged; + } + } + + // Apply x-google-start-bitrate munging for video codecs. + // Even though no track is published yet, the recv-only transceivers include video + // codecs in the SDP. If we have a network quality hint (e.g., from a previous + // connection that detected poor network), apply it to avoid overwhelming the + // network when video is eventually published. + 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, + inner.network_quality_start_bitrate_kbps, + ) { + log::info!( + "Initial offer: applying x-google-start-bitrate={} kbps (ultimate_bps={:?}, network_hint={:?})", + start_kbps, + inner.max_send_bitrate_bps, + inner.network_quality_start_bitrate_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; + } + } } } @@ -178,11 +228,60 @@ impl PeerTransport { inner.max_send_bitrate_bps = bps; } - fn compute_start_bitrate_kbps(ultimate_bps: Option) -> Option { - let ultimate_kbps = (ultimate_bps? / 1000) as u32; + /// Set a network quality hint that caps x-google-start-bitrate on poor networks. + /// + /// This hint persists across all subsequent offers until explicitly changed or the + /// session is replaced. It's set when: + /// - Connection time exceeds SLOW_CONNECTION_THRESHOLD (5 seconds) + /// - Reconnection is triggered (network instability indicator) + /// - Full restart is initiated (passed via connect_with_hint) + /// + /// The hint value (e.g., 300 kbps) is conservative enough to work on poor networks + /// while allowing WebRTC's BWE to ramp up on good networks. + pub async fn set_network_quality_start_bitrate_kbps(&self, kbps: Option) { + let mut inner = self.inner.lock().await; + inner.network_quality_start_bitrate_kbps = kbps; + } + + /// Compute the x-google-start-bitrate value for SDP munging. + /// + /// # Problem Solved + /// + /// Previously, this function required `ultimate_bps` (the track's target bitrate) to + /// compute any start bitrate. But during initial offer creation in single PC mode, + /// no track is published yet, so `ultimate_bps` is None. This meant: + /// 1. Initial offers never got bitrate munging applied + /// 2. After full restart with network_hint=300, the first offer still used default + /// + /// # Solution + /// + /// When `ultimate_bps` is None but `network_hint_kbps` is Some, return the hint + /// directly. This allows initial offers to respect network quality constraints even + /// before any track is published. + /// + /// # Priority + /// + /// The final start bitrate is: min(90% of ultimate, network_hint) + /// - If only ultimate_bps: use 90% of it + /// - If only network_hint: use the hint directly + /// - If both: use the smaller of (90% of ultimate, hint) + fn compute_start_bitrate_kbps( + ultimate_bps: Option, + network_hint_kbps: Option, + ) -> Option { + // If we have a network hint but no ultimate bitrate (e.g., initial offer before + // track is published), use the network hint directly as the start bitrate. + // This is critical for full restart recovery where we want to cap bitrate even + // before the track is republished. + let Some(ultimate_bps) = ultimate_bps else { + return network_hint_kbps; + }; + + let ultimate_kbps = (ultimate_bps / 1000) as u32; if ultimate_kbps == 0 { - return None; + return network_hint_kbps; } + // Use 90% of target bitrate as start bitrate for all codecs. // Why 90%: Gives ~10% headroom for bandwidth estimation while starting close to target. // Why same for all codecs: Target bitrate already accounts for codec efficiency @@ -192,10 +291,19 @@ impl PeerTransport { // A low start-bitrate hint is more likely to hurt than help for VP9/AV1. // If the max is too low, don't inject a start-bitrate hint at all. if ultimate_kbps < 300 { - return None; + return network_hint_kbps; + } + + let mut result = start_kbps.min(ultimate_kbps); + + // Apply network quality cap if a hint is provided (e.g., 300 kbps on poor network). + // This ensures that even with a high ultimate bitrate (e.g., 5 Mbps → 4500 kbps start), + // we respect the network constraint and start lower. + if let Some(hint) = network_hint_kbps { + result = result.min(hint); } - Some(start_kbps.min(ultimate_kbps)) + Some(result) } /// Munge SDP to change a=inactive to a=recvonly for RTP media m-lines in single PC mode. @@ -366,8 +474,51 @@ impl PeerTransport { out.push(rewritten); } + // 3) For video codecs without fmtp lines (VP8/VP9), create new fmtp lines + // This is needed because VP8/VP9 don't have fmtp lines by default + let pts_with_fmtp: std::collections::HashSet = out + .iter() + .filter_map(|line| { + let l = line.trim(); + if let Some(rest) = l.strip_prefix("a=fmtp:") { + rest.split_whitespace().next().map(|s| s.to_string()) + } else { + None + } + }) + .collect(); + + // Find rtpmap lines and insert fmtp after them for codecs without fmtp + let mut final_out: Vec = Vec::with_capacity(out.len() + target_pts.len()); + for line in out.iter() { + final_out.push(line.clone()); + + // Check if this is an rtpmap line for a video codec without fmtp + let l = line.trim(); + if let Some(rest) = l.strip_prefix("a=rtpmap:") { + let mut it = rest.split_whitespace(); + let pt = it.next().unwrap_or(""); + let codec = it.next().unwrap_or(""); + if Self::is_video_codec(codec) + && !pt.is_empty() + && !pts_with_fmtp.contains(pt) + { + // Create fmtp line with x-google-start-bitrate + let fmtp_line = + format!("a=fmtp:{pt} x-google-start-bitrate={start_bitrate_kbps}"); + log::debug!( + "Creating fmtp line for {} (pt={}): {}", + codec, + pt, + fmtp_line + ); + final_out.push(fmtp_line); + } + } + } + // Re-join using same EOL, and ensure trailing EOL (some parsers are picky) - let mut munged = out.join(eol); + let mut munged = final_out.join(eol); if !munged.ends_with(eol) { munged.push_str(eol); } @@ -443,11 +594,15 @@ impl PeerTransport { || 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 let Some(start_kbps) = Self::compute_start_bitrate_kbps( + inner.max_send_bitrate_bps, + inner.network_quality_start_bitrate_kbps, + ) { log::info!( - "Applying x-google-start-bitrate={} kbps (ultimate_bps={:?})", + "Applying x-google-start-bitrate={} kbps (ultimate_bps={:?}, network_hint={:?})", start_kbps, - inner.max_send_bitrate_bps + inner.max_send_bitrate_bps, + inner.network_quality_start_bitrate_kbps ); let munged = Self::munge_x_google_start_bitrate(&sdp, start_kbps); diff --git a/livekit/src/rtc_engine/rtc_session.rs b/livekit/src/rtc_engine/rtc_session.rs index 4bb9ff54a..056755d83 100644 --- a/livekit/src/rtc_engine/rtc_session.rs +++ b/livekit/src/rtc_engine/rtc_session.rs @@ -65,6 +65,14 @@ use crate::{ pub const ICE_CONNECT_TIMEOUT: Duration = Duration::from_secs(15); pub const TRACK_PUBLISH_TIMEOUT: Duration = Duration::from_secs(10); + +/// Total connection time (signal + ICE) taking longer than this indicates a +/// constrained network. Used to cap the initial video bitrate hint. +pub const SLOW_CONNECTION_THRESHOLD: Duration = Duration::from_secs(5); + +/// x-google-start-bitrate cap (kbps) for constrained networks detected via slow ICE +/// or reconnection events. 300 kbps is conservative enough to work on poor networks. +pub const CONSTRAINED_NETWORK_START_BITRATE_KBPS: u32 = 300; pub const LOSSY_DC_LABEL: &str = "_lossy"; pub const RELIABLE_DC_LABEL: &str = "_reliable"; pub const DATA_TRACK_DC_LABEL: &str = "_data_track"; @@ -424,6 +432,8 @@ struct SessionInner { e2ee_manager: Option, subscriber_primary: bool, pc_state_notify: Notify, + /// Time when the connection process started (for network quality detection) + connect_start_time: std::time::Instant, } /// Information about the local participant needed for outgoing @@ -468,12 +478,45 @@ struct SessionHandle { } impl RtcSession { + /// Connect to a LiveKit room (fresh connection with no network hint). pub async fn connect( url: &str, token: &str, options: EngineOptions, e2ee_manager: Option, ) -> EngineResult<(Self, proto::JoinResponse, SessionEvents)> { + Self::connect_with_hint(url, token, options, e2ee_manager, None).await + } + + /// Connect with an optional network quality hint for the initial offer bitrate. + /// + /// # Problem Solved + /// + /// When a connection fails and requires a full restart (new session), we lose any + /// network quality hints that were set on the old session. This meant: + /// 1. Poor network detected → hint set to 300 kbps + /// 2. Full restart creates NEW session + /// 3. New session has hint=None (lost the 300 kbps constraint) + /// 4. First offer uses aggressive 4500 kbps → overwhelms poor network → fails again + /// + /// # Solution + /// + /// Accept an optional `initial_network_hint_kbps` parameter that is passed from + /// `try_restart_connection()`. This hint is set on the new publisher_pc BEFORE + /// any offers are created, so the first offer of the new session respects the + /// network constraint. + /// + /// The hint is set in TWO places to handle both connection paths: + /// 1. Early in single PC mode (before initial offer is created) + /// 2. After publisher_pc is finalized (covers v0 fallback path where early PC isn't used) + pub async fn connect_with_hint( + url: &str, + token: &str, + options: EngineOptions, + e2ee_manager: Option, + initial_network_hint_kbps: Option, + ) -> EngineResult<(Self, proto::JoinResponse, SessionEvents)> { + let connect_start_time = std::time::Instant::now(); let (emitter, session_events) = mpsc::unbounded_channel(); let lk_runtime = LkRuntime::instance(); @@ -487,6 +530,17 @@ impl RtcSession { true, ); + // Set network hint BEFORE creating initial offer so it gets applied. + // In single PC mode (v1 path), the initial offer is sent with JoinRequest. + // This hint ensures the first offer respects network constraints. + if let Some(hint_kbps) = initial_network_hint_kbps { + log::info!( + "Setting initial network hint to {} kbps for new session (v1 path)", + hint_kbps + ); + publisher_pc.set_network_quality_start_bitrate_kbps(Some(hint_kbps)).await; + } + let dcs = Self::create_data_channels(&publisher_pc, &emitter)?; Self::add_recv_media_sections(&publisher_pc.peer_connection(), 3, 3)?; @@ -588,6 +642,32 @@ impl RtcSession { }; let (dt_sender, dt_packet_tx) = DataChannelSender::new(dt_sender_options); + // Set network quality hint on publisher_pc if provided (for full restart after network issues). + // + // # Why This Is Needed (v0 Fallback Path Problem) + // + // There are two code paths for creating publisher_pc: + // 1. Single PC mode (v1): early_publisher_pc is created and hint is set at line ~536 + // 2. v0 fallback: A NEW publisher_pc is created at line ~607 WITHOUT the hint + // + // In v0 fallback, the signal client falls back from the v1 WebSocket path to the + // v0 HTTP-upgrade path. When this happens, `single_pc_mode` becomes false and a + // different PeerTransport is created that doesn't have the hint set. + // + // By setting the hint HERE (after publisher_pc is finalized), we ensure the hint + // is applied regardless of which code path was taken. This is belt-and-suspenders: + // - For v1 path: hint was already set, setting it again is harmless (same value) + // - For v0 path: hint is set for the first time here + // + // This MUST happen BEFORE any offers are created (e.g., when track is republished). + if let Some(hint_kbps) = initial_network_hint_kbps { + log::info!( + "Setting network hint to {} kbps on publisher_pc (covers v0 fallback path)", + hint_kbps + ); + publisher_pc.set_network_quality_start_bitrate_kbps(Some(hint_kbps)).await; + } + let inner = Arc::new(SessionInner { has_published: Default::default(), fast_publish: AtomicBool::new(join_response.fast_publish), @@ -625,6 +705,7 @@ impl RtcSession { e2ee_manager, subscriber_primary, pc_state_notify: Notify::new(), + connect_start_time, }); // Log when a publisher data channel closes without the engine or peer @@ -2170,13 +2251,37 @@ impl SessionInner { Ok(()) }; - tokio::select! { + let result = tokio::select! { res = wait_connected => res, _ = sleep(ICE_CONNECT_TIMEOUT) => { let err = EngineError::Connection("wait_pc_connection timed out".into()); Err(err) } + }; + + // On successful connection, check if total connection time was slow and set network quality hint + if result.is_ok() { + let elapsed = self.connect_start_time.elapsed(); + log::info!( + "Network quality check: connection_time={:?}, threshold={:?}, slow={}", + elapsed, + SLOW_CONNECTION_THRESHOLD, + elapsed > SLOW_CONNECTION_THRESHOLD + ); + if elapsed > SLOW_CONNECTION_THRESHOLD { + log::info!( + "Slow network detected! Setting start_bitrate hint to {} kbps", + CONSTRAINED_NETWORK_START_BITRATE_KBPS + ); + self.publisher_pc + .set_network_quality_start_bitrate_kbps(Some( + CONSTRAINED_NETWORK_START_BITRATE_KBPS, + )) + .await; + } } + + result } /// Start publisher negotiation From 8c27abf08267809bb11facdca583d1d73cb0e197 Mon Sep 17 00:00:00 2001 From: shijing xian Date: Tue, 7 Jul 2026 15:53:08 +0800 Subject: [PATCH 2/8] remove some unneeded changes --- livekit-api/src/signal_client/mod.rs | 2 +- livekit-runtime/src/dispatcher.rs | 11 ----------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/livekit-api/src/signal_client/mod.rs b/livekit-api/src/signal_client/mod.rs index a6acebd4d..e2448d1d1 100644 --- a/livekit-api/src/signal_client/mod.rs +++ b/livekit-api/src/signal_client/mod.rs @@ -148,7 +148,7 @@ impl Default for SignalOptions { auto_subscribe: true, adaptive_stream: false, sdk_options: SignalSdkOptions::default(), - single_peer_connection: true, + single_peer_connection: false, connect_timeout: SIGNAL_CONNECT_TIMEOUT, } } diff --git a/livekit-runtime/src/dispatcher.rs b/livekit-runtime/src/dispatcher.rs index eee1acc9e..024a4e89f 100644 --- a/livekit-runtime/src/dispatcher.rs +++ b/livekit-runtime/src/dispatcher.rs @@ -62,17 +62,6 @@ where JoinHandle { task: Some(task) } } -impl JoinHandle { - /// Aborts the task associated with this handle. - /// - /// The task will be cancelled at the next await point. - pub fn abort(&self) { - if let Some(task) = &self.task { - task.cancel(); - } - } -} - impl Future for JoinHandle { type Output = T; From 038f0f7315e1ac56c6b01cebb93711346ff72b94 Mon Sep 17 00:00:00 2001 From: shijing xian Date: Tue, 7 Jul 2026 15:59:15 +0800 Subject: [PATCH 3/8] fix the comment --- livekit/src/rtc_engine/peer_transport.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/livekit/src/rtc_engine/peer_transport.rs b/livekit/src/rtc_engine/peer_transport.rs index 8ad3811fa..39bd3fecc 100644 --- a/livekit/src/rtc_engine/peer_transport.rs +++ b/livekit/src/rtc_engine/peer_transport.rs @@ -474,8 +474,10 @@ impl PeerTransport { out.push(rewritten); } - // 3) For video codecs without fmtp lines (VP8/VP9), create new fmtp lines - // This is needed because VP8/VP9 don't have fmtp lines by default + // 3) For video codecs that don't already have fmtp lines, create new ones + // with x-google-start-bitrate. This handles cases where the browser/WebRTC + // didn't generate an fmtp line for a particular payload type (e.g., VP8 + // typically has no fmtp, while H.264/VP9-SVC/AV1 usually do). let pts_with_fmtp: std::collections::HashSet = out .iter() .filter_map(|line| { @@ -488,12 +490,12 @@ impl PeerTransport { }) .collect(); - // Find rtpmap lines and insert fmtp after them for codecs without fmtp + // Find rtpmap lines and insert fmtp after them for video codecs without existing fmtp let mut final_out: Vec = Vec::with_capacity(out.len() + target_pts.len()); for line in out.iter() { final_out.push(line.clone()); - // Check if this is an rtpmap line for a video codec without fmtp + // Check if this is an rtpmap line for a video codec without an existing fmtp line let l = line.trim(); if let Some(rest) = l.strip_prefix("a=rtpmap:") { let mut it = rest.split_whitespace(); From 07905566ae872f9f85a1079effe713e970ba3adb Mon Sep 17 00:00:00 2001 From: shijing xian Date: Wed, 8 Jul 2026 22:45:16 +0800 Subject: [PATCH 4/8] use max 1Mbps as start-bitrate, and change the default of degrade preference --- livekit/src/room/options.rs | 52 ++++----- livekit/src/rtc_engine/mod.rs | 61 +--------- livekit/src/rtc_engine/peer_transport.rs | 141 +++++------------------ livekit/src/rtc_engine/rtc_session.rs | 108 +---------------- 4 files changed, 62 insertions(+), 300 deletions(-) diff --git a/livekit/src/room/options.rs b/livekit/src/room/options.rs index 01e56793c..f443f6087 100644 --- a/livekit/src/room/options.rs +++ b/livekit/src/room/options.rs @@ -142,12 +142,14 @@ pub struct TrackPublishOptions { /// Controls how the encoder trades off between resolution and framerate /// when bandwidth is constrained. /// - /// - `MaintainResolution`: Prioritizes resolution, drops frames if needed /// - `MaintainFramerate`: Prioritizes framerate, reduces resolution if needed + /// - `MaintainResolution`: Prioritizes resolution, drops frames if needed /// - `Balanced`: Balances between both /// - /// If not set, the SDK will use a smart default based on the track source - /// and resolution (MaintainResolution for screenshare or video >= 540p). + /// If not set, the SDK uses defaults based on track source: + /// - Camera: `MaintainFramerate` (smoother video for real-time communication) + /// - Screen share: `MaintainResolution` (clarity is critical for text/UI) + /// - Other/unknown: `Balanced` pub degradation_preference: Option, } @@ -175,14 +177,10 @@ impl Default for TrackPublishOptions { /// Returns the appropriate degradation preference for a video track. /// /// If the user explicitly set a preference in `TrackPublishOptions`, that is returned. -/// Otherwise, defaults to `MaintainResolution` for all video tracks. -/// -/// `MaintainResolution` ensures video clarity is preserved during bandwidth constraints -/// by dropping frames rather than reducing resolution. This prevents the "blurry video" -/// issue that users commonly report during initial connection or network fluctuations. -/// -/// Users who prefer smoother video over clarity can explicitly set `Balanced` or -/// `MaintainFramerate` in their `TrackPublishOptions`. +/// Otherwise, defaults based on track source: +/// - `MaintainFramerate` for camera tracks (smoother video for real-time communication) +/// - `MaintainResolution` for screen share (clarity is critical for reading text/UI) +/// - `Balanced` for other/unknown sources pub fn get_default_degradation_preference( options: &TrackPublishOptions, _height: u32, @@ -192,9 +190,12 @@ pub fn get_default_degradation_preference( return pref; } - // Default to MaintainResolution for all video tracks to prevent blurry video - // during bandwidth ramp-up or network constraints - DegradationPreference::MaintainResolution + // Default based on track source + match options.source { + TrackSource::Camera => DegradationPreference::MaintainFramerate, + TrackSource::Screenshare => DegradationPreference::MaintainResolution, + _ => DegradationPreference::Balanced, + } } impl VideoPreset { @@ -571,29 +572,28 @@ mod tests { } #[test] - fn degradation_preference_defaults_to_maintain_resolution() { - // All sources should default to MaintainResolution + fn degradation_preference_defaults_by_source() { + // Camera defaults to MaintainFramerate (smoother video) let camera_options = TrackPublishOptions { source: TrackSource::Camera, ..Default::default() }; - let screenshare_options = - TrackPublishOptions { source: TrackSource::Screenshare, ..Default::default() }; - let default_options = TrackPublishOptions::default(); - assert_eq!( get_default_degradation_preference(&camera_options, 1080), - DegradationPreference::MaintainResolution + DegradationPreference::MaintainFramerate ); + + // Screenshare defaults to MaintainResolution (clarity for text/UI) + let screenshare_options = + TrackPublishOptions { source: TrackSource::Screenshare, ..Default::default() }; assert_eq!( get_default_degradation_preference(&screenshare_options, 1080), DegradationPreference::MaintainResolution ); + + // Unknown/default source defaults to Balanced + let default_options = TrackPublishOptions::default(); assert_eq!( get_default_degradation_preference(&default_options, 720), - DegradationPreference::MaintainResolution - ); - assert_eq!( - get_default_degradation_preference(&default_options, 360), - DegradationPreference::MaintainResolution + DegradationPreference::Balanced ); } diff --git a/livekit/src/rtc_engine/mod.rs b/livekit/src/rtc_engine/mod.rs index 8bbcd094c..a1a26f4ae 100644 --- a/livekit/src/rtc_engine/mod.rs +++ b/livekit/src/rtc_engine/mod.rs @@ -908,34 +908,6 @@ impl EngineInner { let mut resuming_emitted = false; let mut restarting_emitted = false; - // # Set network hint on reconnection trigger - // - // The need to reconnect itself is a strong signal of network instability. Rather - // than waiting for slow connection detection (>5 seconds), we proactively set - // a conservative bitrate hint immediately. - // - // This helps in two scenarios: - // 1. Resume path: The hint is applied to subsequent offers on the SAME session - // 2. Full restart path: The old session's hint is lost, but try_restart_connection() - // passes the hint to the new session via connect_with_hint() - // - // Note: For the resume path, setting the hint here is sufficient. For full restart, - // this hint on the old session is lost (new session is created), so we also pass - // the hint via connect_with_hint() in try_restart_connection(). - { - let session = self.running_handle.read().session.clone(); - log::info!( - "Reconnection triggered: setting constrained network hint to {} kbps", - rtc_session::CONSTRAINED_NETWORK_START_BITRATE_KBPS - ); - session - .publisher() - .set_network_quality_start_bitrate_kbps(Some( - rtc_session::CONSTRAINED_NETWORK_START_BITRATE_KBPS, - )) - .await; - } - for i in 1..=RECONNECT_ATTEMPTS { let (is_closed, full_reconnect) = { let running_handle = self.running_handle.read(); @@ -1080,37 +1052,8 @@ impl EngineInner { let _ = engine_task.await; } - // # Problem: Network hint is lost during full restart - // - // A full restart creates a completely NEW RtcSession, which means: - // - New PeerTransport with network_quality_start_bitrate_kbps = None - // - Any hint set on the old session is lost - // - The first offer of the new session would use aggressive bitrate (e.g., 4500 kbps) - // - On poor networks, this overwhelms the connection and causes immediate failure - // - // # Solution: Pass hint to new session via connect_with_hint() - // - // Since a full restart is triggered by connection failures (which often indicate - // poor network conditions), we proactively pass CONSTRAINED_NETWORK_START_BITRATE_KBPS - // (300 kbps) to the new session. This ensures: - // 1. The initial offer uses conservative bitrate - // 2. Track republishing after restart also uses conservative bitrate - // 3. WebRTC's BWE can ramp up if the network improves - // - // This is more reliable than waiting for slow connection detection (which only - // triggers after 5+ seconds of connection time). - log::info!( - "Full restart: creating new session with network hint {} kbps", - rtc_session::CONSTRAINED_NETWORK_START_BITRATE_KBPS - ); - let (new_session, join_response, session_events) = RtcSession::connect_with_hint( - url, - token, - options, - e2ee_manager, - Some(rtc_session::CONSTRAINED_NETWORK_START_BITRATE_KBPS), - ) - .await?; + let (new_session, join_response, session_events) = + RtcSession::connect(url, token, options, e2ee_manager).await?; // On SignalRestarted, the room will try to unpublish the local tracks // NOTE: Doing operations that use rtc_session will not use the new one diff --git a/livekit/src/rtc_engine/peer_transport.rs b/livekit/src/rtc_engine/peer_transport.rs index 39bd3fecc..7c3e2c10f 100644 --- a/livekit/src/rtc_engine/peer_transport.rs +++ b/livekit/src/rtc_engine/peer_transport.rs @@ -34,8 +34,6 @@ struct TransportInner { // Publish-side target bitrate (bps) for offer munging max_send_bitrate_bps: Option, pending_initial_offer: Option, - // Network quality hint: caps x-google-start-bitrate on poor networks - network_quality_start_bitrate_kbps: Option, } pub struct PeerTransport { @@ -68,7 +66,6 @@ impl PeerTransport { single_pc_mode, max_send_bitrate_bps: None, pending_initial_offer: None, - network_quality_start_bitrate_kbps: None, })), } } @@ -151,25 +148,15 @@ impl PeerTransport { /// Create an initial offer without setting it as local description. /// The offer is stored as pending and will be applied when the server's answer arrives. /// - /// # Problem Solved - /// /// In single PC mode, this initial offer is sent with the JoinRequest before any track - /// is published. Previously, this function only applied `inactive→recvonly` munging but - /// NOT the `x-google-start-bitrate` munging that `create_and_send_offer()` applies. - /// - /// This was a problem during full restart after network issues: - /// 1. Full restart creates a new session with network_hint=300 kbps - /// 2. Initial offer is created here (with recv-only video transceivers) - /// 3. The SDP contains video codecs (VP8/90000 etc.) from recv-only transceivers - /// 4. But without bitrate munging, the first offer after restart didn't get the hint - /// - /// Now we apply the same x-google-start-bitrate munging as `create_and_send_offer()`, - /// so the initial offer respects the network quality hint if one is set. + /// is published. We apply both `inactive→recvonly` munging and `x-google-start-bitrate` + /// munging when a target bitrate is known. pub async fn create_initial_offer(&self) -> EngineResult> { - let mut inner = self.inner.lock().await; + let inner = self.inner.lock().await; if !inner.single_pc_mode { return Ok(None); } + drop(inner); let mut offer = self.peer_connection.create_offer(OfferOptions::default()).await?; let mut sdp = offer.to_string(); @@ -183,26 +170,21 @@ impl PeerTransport { } } - // Apply x-google-start-bitrate munging for video codecs. - // Even though no track is published yet, the recv-only transceivers include video - // codecs in the SDP. If we have a network quality hint (e.g., from a previous - // connection that detected poor network), apply it to avoid overwhelming the - // network when video is eventually published. + // 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 inner = self.inner.lock().await; 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, - inner.network_quality_start_bitrate_kbps, - ) { + if let Some(start_kbps) = Self::compute_start_bitrate_kbps(inner.max_send_bitrate_bps) { log::info!( - "Initial offer: applying x-google-start-bitrate={} kbps (ultimate_bps={:?}, network_hint={:?})", + "Initial offer: applying x-google-start-bitrate={} kbps (target_bps={:?})", start_kbps, - inner.max_send_bitrate_bps, - inner.network_quality_start_bitrate_kbps + inner.max_send_bitrate_bps ); let munged = Self::munge_x_google_start_bitrate(&sdp, start_kbps); @@ -214,6 +196,7 @@ impl PeerTransport { } } + let mut inner = self.inner.lock().await; inner.pending_initial_offer = Some(offer.clone()); Ok(Some(offer)) } @@ -228,82 +211,25 @@ impl PeerTransport { inner.max_send_bitrate_bps = bps; } - /// Set a network quality hint that caps x-google-start-bitrate on poor networks. - /// - /// This hint persists across all subsequent offers until explicitly changed or the - /// session is replaced. It's set when: - /// - Connection time exceeds SLOW_CONNECTION_THRESHOLD (5 seconds) - /// - Reconnection is triggered (network instability indicator) - /// - Full restart is initiated (passed via connect_with_hint) - /// - /// The hint value (e.g., 300 kbps) is conservative enough to work on poor networks - /// while allowing WebRTC's BWE to ramp up on good networks. - pub async fn set_network_quality_start_bitrate_kbps(&self, kbps: Option) { - let mut inner = self.inner.lock().await; - inner.network_quality_start_bitrate_kbps = kbps; - } + /// 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; /// Compute the x-google-start-bitrate value for SDP munging. /// - /// # Problem Solved - /// - /// Previously, this function required `ultimate_bps` (the track's target bitrate) to - /// compute any start bitrate. But during initial offer creation in single PC mode, - /// no track is published yet, so `ultimate_bps` is None. This meant: - /// 1. Initial offers never got bitrate munging applied - /// 2. After full restart with network_hint=300, the first offer still used default - /// - /// # Solution - /// - /// When `ultimate_bps` is None but `network_hint_kbps` is Some, return the hint - /// directly. This allows initial offers to respect network quality constraints even - /// before any track is published. - /// - /// # Priority - /// - /// The final start bitrate is: min(90% of ultimate, network_hint) - /// - If only ultimate_bps: use 90% of it - /// - If only network_hint: use the hint directly - /// - If both: use the smaller of (90% of ultimate, hint) - fn compute_start_bitrate_kbps( - ultimate_bps: Option, - network_hint_kbps: Option, - ) -> Option { - // If we have a network hint but no ultimate bitrate (e.g., initial offer before - // track is published), use the network hint directly as the start bitrate. - // This is critical for full restart recovery where we want to cap bitrate even - // before the track is republished. - let Some(ultimate_bps) = ultimate_bps else { - return network_hint_kbps; - }; - - let ultimate_kbps = (ultimate_bps / 1000) as u32; - if ultimate_kbps == 0 { - return network_hint_kbps; - } - - // Use 90% of target bitrate as start bitrate for all codecs. - // Why 90%: Gives ~10% headroom for bandwidth estimation while starting close to target. - // Why same for all codecs: Target bitrate already accounts for codec efficiency - // (e.g., users set lower targets for VP9/AV1 knowing they're more efficient). - let start_kbps = (ultimate_kbps as f64 * 0.9).round() as u32; - - // A low start-bitrate hint is more likely to hurt than help for VP9/AV1. - // If the max is too low, don't inject a start-bitrate hint at all. - if ultimate_kbps < 300 { - return network_hint_kbps; - } - - let mut result = start_kbps.min(ultimate_kbps); - - // Apply network quality cap if a hint is provided (e.g., 300 kbps on poor network). - // This ensures that even with a high ultimate bitrate (e.g., 5 Mbps → 4500 kbps start), - // we respect the network constraint and start lower. - if let Some(hint) = network_hint_kbps { - result = result.min(hint); + /// 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 { + let target_bps = target_bps?; + let target_kbps = (target_bps / 1000) as u32; + + if target_kbps == 0 || target_kbps < 300 { + return None; } - Some(result) + // 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)) } /// Munge SDP to change a=inactive to a=recvonly for RTP media m-lines in single PC mode. @@ -589,27 +515,24 @@ impl PeerTransport { } } - // Apply x-google-start-bitrate for all video codecs to improve initial quality + // 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. 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, - inner.network_quality_start_bitrate_kbps, - ) { + if let Some(start_kbps) = Self::compute_start_bitrate_kbps(inner.max_send_bitrate_bps) { log::info!( - "Applying x-google-start-bitrate={} kbps (ultimate_bps={:?}, network_hint={:?})", + "Applying x-google-start-bitrate={} kbps (target_bps={:?})", start_kbps, - inner.max_send_bitrate_bps, - inner.network_quality_start_bitrate_kbps + inner.max_send_bitrate_bps ); let munged = Self::munge_x_google_start_bitrate(&sdp, start_kbps); if munged != sdp { - log::info!("SDP munged successfully for video codec"); + log::debug!("SDP munged successfully for video codec"); match SessionDescription::parse(&munged, offer.sdp_type()) { Ok(parsed) => offer = parsed, Err(e) => log::warn!( diff --git a/livekit/src/rtc_engine/rtc_session.rs b/livekit/src/rtc_engine/rtc_session.rs index 056755d83..aa9c00cea 100644 --- a/livekit/src/rtc_engine/rtc_session.rs +++ b/livekit/src/rtc_engine/rtc_session.rs @@ -65,14 +65,6 @@ use crate::{ pub const ICE_CONNECT_TIMEOUT: Duration = Duration::from_secs(15); pub const TRACK_PUBLISH_TIMEOUT: Duration = Duration::from_secs(10); - -/// Total connection time (signal + ICE) taking longer than this indicates a -/// constrained network. Used to cap the initial video bitrate hint. -pub const SLOW_CONNECTION_THRESHOLD: Duration = Duration::from_secs(5); - -/// x-google-start-bitrate cap (kbps) for constrained networks detected via slow ICE -/// or reconnection events. 300 kbps is conservative enough to work on poor networks. -pub const CONSTRAINED_NETWORK_START_BITRATE_KBPS: u32 = 300; pub const LOSSY_DC_LABEL: &str = "_lossy"; pub const RELIABLE_DC_LABEL: &str = "_reliable"; pub const DATA_TRACK_DC_LABEL: &str = "_data_track"; @@ -432,8 +424,6 @@ struct SessionInner { e2ee_manager: Option, subscriber_primary: bool, pc_state_notify: Notify, - /// Time when the connection process started (for network quality detection) - connect_start_time: std::time::Instant, } /// Information about the local participant needed for outgoing @@ -478,45 +468,13 @@ struct SessionHandle { } impl RtcSession { - /// Connect to a LiveKit room (fresh connection with no network hint). + /// Connect to a LiveKit room. pub async fn connect( url: &str, token: &str, options: EngineOptions, e2ee_manager: Option, ) -> EngineResult<(Self, proto::JoinResponse, SessionEvents)> { - Self::connect_with_hint(url, token, options, e2ee_manager, None).await - } - - /// Connect with an optional network quality hint for the initial offer bitrate. - /// - /// # Problem Solved - /// - /// When a connection fails and requires a full restart (new session), we lose any - /// network quality hints that were set on the old session. This meant: - /// 1. Poor network detected → hint set to 300 kbps - /// 2. Full restart creates NEW session - /// 3. New session has hint=None (lost the 300 kbps constraint) - /// 4. First offer uses aggressive 4500 kbps → overwhelms poor network → fails again - /// - /// # Solution - /// - /// Accept an optional `initial_network_hint_kbps` parameter that is passed from - /// `try_restart_connection()`. This hint is set on the new publisher_pc BEFORE - /// any offers are created, so the first offer of the new session respects the - /// network constraint. - /// - /// The hint is set in TWO places to handle both connection paths: - /// 1. Early in single PC mode (before initial offer is created) - /// 2. After publisher_pc is finalized (covers v0 fallback path where early PC isn't used) - pub async fn connect_with_hint( - url: &str, - token: &str, - options: EngineOptions, - e2ee_manager: Option, - initial_network_hint_kbps: Option, - ) -> EngineResult<(Self, proto::JoinResponse, SessionEvents)> { - let connect_start_time = std::time::Instant::now(); let (emitter, session_events) = mpsc::unbounded_channel(); let lk_runtime = LkRuntime::instance(); @@ -530,17 +488,6 @@ impl RtcSession { true, ); - // Set network hint BEFORE creating initial offer so it gets applied. - // In single PC mode (v1 path), the initial offer is sent with JoinRequest. - // This hint ensures the first offer respects network constraints. - if let Some(hint_kbps) = initial_network_hint_kbps { - log::info!( - "Setting initial network hint to {} kbps for new session (v1 path)", - hint_kbps - ); - publisher_pc.set_network_quality_start_bitrate_kbps(Some(hint_kbps)).await; - } - let dcs = Self::create_data_channels(&publisher_pc, &emitter)?; Self::add_recv_media_sections(&publisher_pc.peer_connection(), 3, 3)?; @@ -642,32 +589,6 @@ impl RtcSession { }; let (dt_sender, dt_packet_tx) = DataChannelSender::new(dt_sender_options); - // Set network quality hint on publisher_pc if provided (for full restart after network issues). - // - // # Why This Is Needed (v0 Fallback Path Problem) - // - // There are two code paths for creating publisher_pc: - // 1. Single PC mode (v1): early_publisher_pc is created and hint is set at line ~536 - // 2. v0 fallback: A NEW publisher_pc is created at line ~607 WITHOUT the hint - // - // In v0 fallback, the signal client falls back from the v1 WebSocket path to the - // v0 HTTP-upgrade path. When this happens, `single_pc_mode` becomes false and a - // different PeerTransport is created that doesn't have the hint set. - // - // By setting the hint HERE (after publisher_pc is finalized), we ensure the hint - // is applied regardless of which code path was taken. This is belt-and-suspenders: - // - For v1 path: hint was already set, setting it again is harmless (same value) - // - For v0 path: hint is set for the first time here - // - // This MUST happen BEFORE any offers are created (e.g., when track is republished). - if let Some(hint_kbps) = initial_network_hint_kbps { - log::info!( - "Setting network hint to {} kbps on publisher_pc (covers v0 fallback path)", - hint_kbps - ); - publisher_pc.set_network_quality_start_bitrate_kbps(Some(hint_kbps)).await; - } - let inner = Arc::new(SessionInner { has_published: Default::default(), fast_publish: AtomicBool::new(join_response.fast_publish), @@ -705,7 +626,6 @@ impl RtcSession { e2ee_manager, subscriber_primary, pc_state_notify: Notify::new(), - connect_start_time, }); // Log when a publisher data channel closes without the engine or peer @@ -2251,37 +2171,13 @@ impl SessionInner { Ok(()) }; - let result = tokio::select! { + tokio::select! { res = wait_connected => res, _ = sleep(ICE_CONNECT_TIMEOUT) => { let err = EngineError::Connection("wait_pc_connection timed out".into()); Err(err) } - }; - - // On successful connection, check if total connection time was slow and set network quality hint - if result.is_ok() { - let elapsed = self.connect_start_time.elapsed(); - log::info!( - "Network quality check: connection_time={:?}, threshold={:?}, slow={}", - elapsed, - SLOW_CONNECTION_THRESHOLD, - elapsed > SLOW_CONNECTION_THRESHOLD - ); - if elapsed > SLOW_CONNECTION_THRESHOLD { - log::info!( - "Slow network detected! Setting start_bitrate hint to {} kbps", - CONSTRAINED_NETWORK_START_BITRATE_KBPS - ); - self.publisher_pc - .set_network_quality_start_bitrate_kbps(Some( - CONSTRAINED_NETWORK_START_BITRATE_KBPS, - )) - .await; - } } - - result } /// Start publisher negotiation From 501a6b7d450e15a83b7e6d3c4b84aeac41683e6f Mon Sep 17 00:00:00 2001 From: shijing xian Date: Wed, 8 Jul 2026 22:58:49 +0800 Subject: [PATCH 5/8] cargo fmt --- livekit-api/src/signal_client/signal_stream.rs | 4 +++- livekit/src/rtc_engine/peer_transport.rs | 12 ++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/livekit-api/src/signal_client/signal_stream.rs b/livekit-api/src/signal_client/signal_stream.rs index d9f7b71a1..db437fed8 100644 --- a/livekit-api/src/signal_client/signal_stream.rs +++ b/livekit-api/src/signal_client/signal_stream.rs @@ -415,7 +415,9 @@ impl SignalStream { { // TLS connection closed without close_notify - treat as normal close // This happens when the server closes the connection abruptly - log::debug!("websocket closed with unexpected EOF (TLS close without close_notify)"); + log::debug!( + "websocket closed with unexpected EOF (TLS close without close_notify)" + ); break; } Err(WsError::ConnectionClosed) | Err(WsError::AlreadyClosed) => { diff --git a/livekit/src/rtc_engine/peer_transport.rs b/livekit/src/rtc_engine/peer_transport.rs index 7c3e2c10f..e54cd5ee4 100644 --- a/livekit/src/rtc_engine/peer_transport.rs +++ b/livekit/src/rtc_engine/peer_transport.rs @@ -427,19 +427,11 @@ impl PeerTransport { let mut it = rest.split_whitespace(); let pt = it.next().unwrap_or(""); let codec = it.next().unwrap_or(""); - if Self::is_video_codec(codec) - && !pt.is_empty() - && !pts_with_fmtp.contains(pt) - { + if Self::is_video_codec(codec) && !pt.is_empty() && !pts_with_fmtp.contains(pt) { // Create fmtp line with x-google-start-bitrate let fmtp_line = format!("a=fmtp:{pt} x-google-start-bitrate={start_bitrate_kbps}"); - log::debug!( - "Creating fmtp line for {} (pt={}): {}", - codec, - pt, - fmtp_line - ); + log::debug!("Creating fmtp line for {} (pt={}): {}", codec, pt, fmtp_line); final_out.push(fmtp_line); } } From a1c6b4fd48b89e2c6ce8053eb93122b562b245f2 Mon Sep 17 00:00:00 2001 From: shijing xian Date: Wed, 8 Jul 2026 23:34:13 +0800 Subject: [PATCH 6/8] fix the lock --- livekit/src/rtc_engine/peer_transport.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/livekit/src/rtc_engine/peer_transport.rs b/livekit/src/rtc_engine/peer_transport.rs index e54cd5ee4..ac9935c25 100644 --- a/livekit/src/rtc_engine/peer_transport.rs +++ b/livekit/src/rtc_engine/peer_transport.rs @@ -173,18 +173,20 @@ impl PeerTransport { // 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 inner = self.inner.lock().await; 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) { + 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 (target_bps={:?})", - start_kbps, - inner.max_send_bitrate_bps + "Initial offer: applying x-google-start-bitrate={} kbps", + start_kbps ); let munged = Self::munge_x_google_start_bitrate(&sdp, start_kbps); From aa8e8c2e9a11af397e5c5d23e4919f2ed1adec31 Mon Sep 17 00:00:00 2001 From: shijing xian Date: Wed, 8 Jul 2026 23:36:01 +0800 Subject: [PATCH 7/8] adding changeset --- ...implify-start-bitrate-and-degradation-defaults.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/simplify-start-bitrate-and-degradation-defaults.md diff --git a/.changeset/simplify-start-bitrate-and-degradation-defaults.md b/.changeset/simplify-start-bitrate-and-degradation-defaults.md new file mode 100644 index 000000000..7db12aa17 --- /dev/null +++ b/.changeset/simplify-start-bitrate-and-degradation-defaults.md @@ -0,0 +1,12 @@ +--- +"livekit": patch +--- + +Simplify x-google-start-bitrate logic and update degradation preference defaults + +- Start bitrate: use min(90% of target, 1 Mbps) instead of adaptive network hints +- Remove slow connection detection and network quality hints on reconnect +- Default degradation preference by track source: + - Camera: MaintainFramerate (smoother video) + - Screenshare: MaintainResolution (clarity for text/UI) + - Other: Balanced From 50cd4aa8f455daed02ae6c3a455340b2c443699b Mon Sep 17 00:00:00 2001 From: shijing xian Date: Thu, 9 Jul 2026 15:53:24 +0800 Subject: [PATCH 8/8] fixed the test --- livekit/src/rtc_engine/peer_transport.rs | 83 +++++++++++++++++++++--- 1 file changed, 75 insertions(+), 8 deletions(-) diff --git a/livekit/src/rtc_engine/peer_transport.rs b/livekit/src/rtc_engine/peer_transport.rs index ac9935c25..749b6ccd0 100644 --- a/livekit/src/rtc_engine/peer_transport.rs +++ b/livekit/src/rtc_engine/peer_transport.rs @@ -184,10 +184,7 @@ impl PeerTransport { 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 - ); + 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 { @@ -634,8 +631,9 @@ a=fmtp:104 x-google-start-bitrate=1000;foo=bar\n"; } #[test] - fn vp9_without_fmtp_line_is_noop() { - // VP9 rtpmap exists, but no fmtp: function intentionally does not insert a new fmtp line. + fn vp9_without_fmtp_line_creates_one() { + // VP9 rtpmap exists, but no fmtp: function creates a new fmtp line with x-google-start-bitrate. + // This ensures all video codecs (including those like VP8 that typically lack fmtp) get the hint. let sdp = "v=0\n\ o=- 0 0 IN IP4 127.0.0.1\n\ s=-\n\ @@ -643,9 +641,16 @@ t=0 0\n\ m=video 9 UDP/TLS/RTP/SAVPF 98\n\ a=rtpmap:98 VP9/90000\n"; let out = PeerTransport::munge_x_google_start_bitrate(sdp, 3200); + let expected = "v=0\n\ +o=- 0 0 IN IP4 127.0.0.1\n\ +s=-\n\ +t=0 0\n\ +m=video 9 UDP/TLS/RTP/SAVPF 98\n\ +a=rtpmap:98 VP9/90000\n\ +a=fmtp:98 x-google-start-bitrate=3200\n"; assert_eq!( - out, sdp, - "should not modify SDP if there is no fmtp line for the VP9/AV1 payload type" + out, expected, + "should create fmtp line with x-google-start-bitrate for video codec without fmtp" ); } @@ -689,6 +694,68 @@ a=fmtp:104 x-google-start-bitrate=1111;baz=qux\n"; assert!(!out.contains("a=fmtp:104 x-google-start-bitrate=1111")); } + #[test] + fn all_video_codecs_get_fmtp_with_start_bitrate() { + // Mixed scenario: some codecs have fmtp, some don't + // All video codecs should end up with exactly one fmtp line containing x-google-start-bitrate + let sdp = "v=0\n\ +o=- 0 0 IN IP4 127.0.0.1\n\ +s=-\n\ +t=0 0\n\ +m=video 9 UDP/TLS/RTP/SAVPF 96 97 98 99 100\n\ +a=rtpmap:96 VP8/90000\n\ +a=rtpmap:97 VP9/90000\n\ +a=fmtp:97 profile-id=0\n\ +a=rtpmap:98 H264/90000\n\ +a=fmtp:98 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f\n\ +a=rtpmap:99 AV1/90000\n\ +a=rtpmap:100 H265/90000\n\ +a=fmtp:100 profile-id=1\n"; + let out = PeerTransport::munge_x_google_start_bitrate(sdp, 900); + + // VP8 (96): had no fmtp, should get new one + assert!( + out.contains("a=fmtp:96 x-google-start-bitrate=900\n"), + "VP8 should get new fmtp line" + ); + assert_eq!(out.matches("a=fmtp:96 ").count(), 1, "VP8 should have exactly one fmtp line"); + + // VP9 (97): had fmtp, should get bitrate appended + assert!( + out.contains("a=fmtp:97 profile-id=0;x-google-start-bitrate=900\n"), + "VP9 should have bitrate appended to existing fmtp" + ); + assert_eq!(out.matches("a=fmtp:97 ").count(), 1, "VP9 should have exactly one fmtp line"); + + // H264 (98): had fmtp, should get bitrate appended + assert!( + out.contains("a=fmtp:98 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f;x-google-start-bitrate=900\n"), + "H264 should have bitrate appended to existing fmtp" + ); + assert_eq!(out.matches("a=fmtp:98 ").count(), 1, "H264 should have exactly one fmtp line"); + + // AV1 (99): had no fmtp, should get new one + assert!( + out.contains("a=fmtp:99 x-google-start-bitrate=900\n"), + "AV1 should get new fmtp line" + ); + assert_eq!(out.matches("a=fmtp:99 ").count(), 1, "AV1 should have exactly one fmtp line"); + + // H265 (100): had fmtp, should get bitrate appended + assert!( + out.contains("a=fmtp:100 profile-id=1;x-google-start-bitrate=900\n"), + "H265 should have bitrate appended to existing fmtp" + ); + assert_eq!(out.matches("a=fmtp:100 ").count(), 1, "H265 should have exactly one fmtp line"); + + // Total: 5 video codecs, 5 fmtp lines with x-google-start-bitrate + assert_eq!( + out.matches("x-google-start-bitrate=900").count(), + 5, + "all 5 video codecs should have x-google-start-bitrate" + ); + } + #[test] fn does_not_duplicate_start_bitrate_when_already_present_no_semicolon_following() { // Existing x-google-start-bitrate at end of line (no trailing ';')