From 3d5615d6fa54c9c9e3ca5a7178cb3c0c970e37e7 Mon Sep 17 00:00:00 2001 From: shijing xian Date: Mon, 14 Sep 2026 09:52:23 -0700 Subject: [PATCH] Write the video start bitrate hint as one connection-level value, once x-google-start-bitrate is connection-scoped in libwebrtc: ApplyChangedParams reads it per m-section but pushes it into the shared Call via SetSdpBitrateParameters, where RtpBitrateConfigurator holds one config for the whole peer connection. Differing per-section values were last-writer-wins on m-section order, so a camera plus a screen share could seed the estimator from either one depending on SDP layout. Every video section now carries the same value: the largest hint among the sections that map to a published track. Write it only on the first offer that carries local video. libwebrtc 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 a restart of a converged bandwidth estimator. The latch is set only once the offer carrying the hint is accepted locally, so a rejected munge retries on the next offer. Add a 300 kbps target floor, matching the Rust SDK: below that, seeding above the real capacity costs more than the ramp it saves. applyVideoStartBitrate no longer matches the section to a track; that moves to findTrackCodecPayload so the dependent DD extension munging still runs on every offer. Co-Authored-By: Claude Opus 5 (1M context) --- .../video-start-bitrate-connection-level.md | 11 ++ src/room/PCTransport.test.ts | 68 +++++++- src/room/PCTransport.ts | 164 ++++++++++++++---- 3 files changed, 200 insertions(+), 43 deletions(-) create mode 100644 .changeset/video-start-bitrate-connection-level.md diff --git a/.changeset/video-start-bitrate-connection-level.md b/.changeset/video-start-bitrate-connection-level.md new file mode 100644 index 0000000000..af5807e0c2 --- /dev/null +++ b/.changeset/video-start-bitrate-connection-level.md @@ -0,0 +1,11 @@ +--- +'livekit-client': patch +--- + +Write the `x-google-start-bitrate` hint as a single connection-level value, once per publisher connection. + +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. Differing per-section values were therefore last-writer-wins on m-section order, so publishing a camera and a screen share together could seed the estimator from either one depending on SDP layout. Every video section now carries the same value: the largest hint among the sections that map to a published track. + +The hint is also written only on the first offer that carries local video, instead of on every offer. libwebrtc retains `start_bitrate_bps` and re-applies it on network route changes (`RtpTransportControllerSend::OnNetworkRouteChanged`), 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. + +Targets below 300 kbps now get no hint, matching the Rust SDK: below that, seeding above the real capacity costs more than the ramp it saves. diff --git a/src/room/PCTransport.test.ts b/src/room/PCTransport.test.ts index af12f5dbdc..334bdb03e9 100644 --- a/src/room/PCTransport.test.ts +++ b/src/room/PCTransport.test.ts @@ -2,10 +2,13 @@ import { type MediaDescription, parse } from 'sdp-transform'; import { describe, expect, it } from 'vitest'; import { applyVideoStartBitrate, + computeConnectionStartBitrate, + computeTrackStartBitrate, conformBundledCodecFmtp, ensureAudioNackAndStereo, ensureVideoDDExtension, extractStereoAndNackAudioFromOffer, + findTrackCodecPayload, fmtpConfigHasParam, placeholderMidsFromTransceivers, } from './PCTransport'; @@ -61,9 +64,7 @@ a=recvonly a=rtpmap:49 H265/90000 a=fmtp:49 level-id=180;profile-id=1;tier-flag=0;tx-mode=SRST`; -describe('video start bitrate', () => { - it('applies the bitrate only to the section whose msid track ID matches the cid', () => { - const { media } = parse(`v=0 +const TWO_VIDEO_SECTIONS = `v=0 o=- 0 0 IN IP4 127.0.0.1 s=- t=0 0 @@ -79,15 +80,68 @@ c=IN IP4 0.0.0.0 a=mid:1 a=sendonly a=msid:PA_remote|camera camera-cid -a=rtpmap:96 VP8/90000`); +a=rtpmap:96 VP8/90000`; + +describe('video start bitrate', () => { + it('matches only the section whose msid track ID matches the cid', () => { + const { media } = parse(TWO_VIDEO_SECTIONS); + + expect(findTrackCodecPayload(media[0], 'camera-cid', 'VP8')).toBeUndefined(); + expect(findTrackCodecPayload(media[1], 'camera-cid', 'VP8')).toBe(96); + // Section belongs to the track but does not offer the codec. + expect(findTrackCodecPayload(media[1], 'camera-cid', 'AV1')).toBe(0); + }); + + it('applies the bitrate only to the section it is given', () => { + const { media } = parse(TWO_VIDEO_SECTIONS); - for (const section of media) { - applyVideoStartBitrate(section, 'camera-cid', 'VP8', 1_000); - } + applyVideoStartBitrate(media[1], 96, 900); expect(fmtpOf(media, '0', 96)).toBeUndefined(); expect(paramSet(fmtpOf(media, '1', 96)!)).toContain('x-google-start-bitrate=900'); }); + + it('caps camera at 1 Mbps but leaves screen share uncapped', () => { + const camera = { cid: 'c', codec: 'VP8', maxbr: 3_000 }; + const screenShare = { ...camera, isScreenShare: true }; + + expect(computeTrackStartBitrate(camera)).toBe(1_000); + expect(computeTrackStartBitrate(screenShare)).toBe(2_700); + }); + + it('gives no hint below the 300 kbps target floor', () => { + expect(computeTrackStartBitrate({ cid: 'c', codec: 'VP8', maxbr: 299 })).toBeUndefined(); + expect(computeTrackStartBitrate({ cid: 'c', codec: 'VP8', maxbr: 300 })).toBe(270); + }); + + it('uses one connection-level value: the largest hint across video sections', () => { + const { media } = parse(TWO_VIDEO_SECTIONS); + + const startBitrate = computeConnectionStartBitrate(media, [ + { cid: 'camera-cid', codec: 'VP8', maxbr: 1_000 }, + { cid: 'other-track', codec: 'VP8', maxbr: 3_000, isScreenShare: true }, + ]); + + expect(startBitrate).toBe(2_700); + }); + + it('ignores registered tracks with no section in the current SDP', () => { + const { media } = parse(TWO_VIDEO_SECTIONS); + + const startBitrate = computeConnectionStartBitrate(media, [ + { cid: 'camera-cid', codec: 'VP8', maxbr: 1_000 }, + // Stale entry: trackBitrates is append-only and outlives an unpublish. + { cid: 'unpublished-cid', codec: 'VP8', maxbr: 8_000, isScreenShare: true }, + ]); + + expect(startBitrate).toBe(900); + }); + + it('gives no connection value when no section maps to a published track', () => { + const { media } = parse(TWO_VIDEO_SECTIONS); + + expect(computeConnectionStartBitrate(media, [])).toBeUndefined(); + }); }); describe('placeholderMidsFromTransceivers', () => { diff --git a/src/room/PCTransport.ts b/src/room/PCTransport.ts index 6983a35486..54afd416bf 100644 --- a/src/room/PCTransport.ts +++ b/src/room/PCTransport.ts @@ -34,51 +34,118 @@ const startBitrateMultiplier = 0.9; /** Maximum x-google-start-bitrate in kbps. 1 Mbps prevents BWE from starting too aggressively. */ const maxStartBitrateKbps = 1000; +/** + * Minimum target bitrate in kbps for the start bitrate hint. Below this, seeding above the + * real capacity costs more than the ramp it saves, so libwebrtc's default is left in place. + */ +const minTargetBitrateKbps = 300; + const debounceInterval = 20; /** - * Applies the configured start bitrate when this media section belongs to `cid`. - * This SDP munging is used for a bitrate setting that cannot be applied through - * `RTCRtpEncodingParameters`. + * Codec payload for `codec` in this media section, when the section carries `cid`. * - * Returns `undefined` when the section does not belong to the track, `0` when - * it does but does not offer the requested codec, and the codec payload when the - * requested codec is present (whether the bitrate was added or already set). + * Returns `undefined` when the section does not belong to the track, `0` when it does but + * does not offer the requested codec, and the codec payload otherwise. * * @internal */ -export function applyVideoStartBitrate( +export function findTrackCodecPayload( media: MediaDescription, cid: string, codec: string, - maxbr: number, - isScreenShare = false, ): number | undefined { if (!media.msid?.includes(cid)) { return undefined; } + return media.rtp.find((rtp) => rtp.codec.toUpperCase() === codec.toUpperCase())?.payload ?? 0; +} - const codecPayload = - media.rtp.find((rtp) => rtp.codec.toUpperCase() === codec.toUpperCase())?.payload ?? 0; - if (codecPayload === 0) { - return 0; +/** + * Start bitrate hinted for a single track, or `undefined` when its target is too low to + * be worth seeding. + * + * 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 1 Mbps 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. + * + * TODO: adjust dynamically from network conditions (e.g. a previous BWE estimate) rather + * than a fixed cap. + * + * @internal + */ +export function computeTrackStartBitrate(trackbr: TrackBitrateInfo): number | undefined { + if (trackbr.maxbr < minTargetBitrateKbps) { + return undefined; } + const calculated = Math.round(trackbr.maxbr * startBitrateMultiplier); + return trackbr.isScreenShare ? calculated : Math.min(calculated, maxStartBitrateKbps); +} - // Use 90% of target bitrate, capped at 1 Mbps for camera to prevent BWE - // from starting too aggressively. Screen share is not capped since text/UI - // clarity requires high bitrate from the start. - // TODO: dynamically adjust start bitrate based on network conditions (e.g., previous BWE estimate) - const calculatedStartBitrate = Math.round(maxbr * startBitrateMultiplier); - const startBitrate = isScreenShare - ? calculatedStartBitrate - : Math.min(calculatedStartBitrate, maxStartBitrateKbps); +/** + * The single start bitrate for this peer connection: the largest hint among the video + * m-sections of `media` that map to a published track. + * + * libwebrtc reads `x-google-start-bitrate` per m-section but applies it to the shared + * `Call` (`WebRtcVideoSendChannel::ApplyChangedParams` -> `SetSdpBitrateParameters`), where + * `RtpBitrateConfigurator` holds one config for the whole connection. Differing per-section + * values are therefore last-writer-wins, decided by m-section order, so every video section + * gets the same number instead. + * + * Only sections present in the current SDP are considered: `trackBitrates` is append-only + * and can hold entries for tracks that are no longer published. + * + * @internal + */ +export function computeConnectionStartBitrate( + media: MediaDescription[], + trackBitrates: TrackBitrateInfo[], +): number | undefined { + let connectionStartBitrate: number | undefined; + for (const m of media) { + if (m.type !== 'video') { + continue; + } + for (const trackbr of trackBitrates) { + if (!trackbr.cid) { + continue; + } + const codecPayload = findTrackCodecPayload(m, trackbr.cid, trackbr.codec); + if (codecPayload === undefined) { + continue; + } + const startBitrate = codecPayload > 0 ? computeTrackStartBitrate(trackbr) : undefined; + if ( + startBitrate !== undefined && + (connectionStartBitrate === undefined || startBitrate > connectionStartBitrate) + ) { + connectionStartBitrate = startBitrate; + } + break; + } + } + return connectionStartBitrate; +} +/** + * Declares `x-google-start-bitrate` on `codecPayload`'s fmtp. This SDP munging is used for + * a bitrate setting that cannot be applied through `RTCRtpEncodingParameters`. + * + * Returns whether the section now carries the hint. + * + * @internal + */ +export function applyVideoStartBitrate( + media: MediaDescription, + codecPayload: number, + startBitrate: number, +): boolean { const fmtp = media.fmtp.find((entry) => entry.payload === codecPayload); if (fmtp) { - // If another track's fmtp already has a start bitrate, it cannot be - // overridden here because the payload type is shared across the bundle. - // This forces every track sharing that payload to use the initial track's - // start bitrate. + // A payload type is shared across the bundle, so a value written for one section is + // already the connection-level one; leave it rather than rewrite it. if (!fmtp.config.includes('x-google-start-bitrate')) { fmtp.config += `;x-google-start-bitrate=${startBitrate}`; } @@ -90,7 +157,7 @@ export function applyVideoStartBitrate( }); } - return codecPayload; + return true; } export const PCEvents = { @@ -141,6 +208,15 @@ export default class PCTransport extends (EventEmitter as new () => TypedEmitter trackBitrates: TrackBitrateInfo[] = []; + /** + * Whether an offer carrying the connection-level `x-google-start-bitrate` has been + * accepted locally. The hint is written once per peer connection: libwebrtc retains + * `start_bitrate_bps` in `RtpBitrateConfigurator` and re-applies it on network route + * changes, so a later rewrite is at best a no-op and at worst restarts a converged + * bandwidth estimator. A new peer connection (full reconnect) seeds a new estimator. + */ + private hasAppliedVideoStartBitrate = false; + remoteStereoMids: string[] = []; remoteNackMids: string[] = []; @@ -453,6 +529,14 @@ export default class PCTransport extends (EventEmitter as new () => TypedEmitter this.log.debug('original offer', { sdp: offer.sdp }); const sdpParsed = parse(offer.sdp ?? ''); + // One value for every video m-section, written only on the first offer that carries + // local video: the hint is connection-level in libwebrtc, so differing per-section + // values would be last-writer-wins on m-section order. Offers before any video is + // published (data channel or audio only) find no target and leave the latch unset. + const connectionStartBitrate = this.hasAppliedVideoStartBitrate + ? undefined + : computeConnectionStartBitrate(sdpParsed.media, this.trackBitrates); + let appliedVideoStartBitrate = false; sdpParsed.media.forEach((media) => { ensureIPAddrMatchVersion(media); if (media.type === 'audio') { @@ -463,19 +547,21 @@ export default class PCTransport extends (EventEmitter as new () => TypedEmitter return false; } - const codecPayload = applyVideoStartBitrate( - media, - trackbr.cid, - trackbr.codec, - trackbr.maxbr, - trackbr.isScreenShare, - ); + const codecPayload = findTrackCodecPayload(media, trackbr.cid, trackbr.codec); if (codecPayload === undefined) { return false; } - if (codecPayload > 0 && isSVCCodec(trackbr.codec) && !isSafari()) { - this.ddExtID = ensureVideoDDExtension(media, sdpParsed, this.ddExtID); + if (codecPayload > 0) { + if (connectionStartBitrate !== undefined) { + appliedVideoStartBitrate = + applyVideoStartBitrate(media, codecPayload, connectionStartBitrate) || + appliedVideoStartBitrate; + } + + if (isSVCCodec(trackbr.codec) && !isSafari()) { + this.ddExtID = ensureVideoDDExtension(media, sdpParsed, this.ddExtID); + } } return true; @@ -500,7 +586,13 @@ export default class PCTransport extends (EventEmitter as new () => TypedEmitter }); return; } - await this.setMungedSDP(offer, write(sdpParsed)); + const mungedSdp = write(sdpParsed); + await this.setMungedSDP(offer, mungedSdp); + // setMungedSDP falls back to the unmunged SDP on rejection. Only consume the + // one-shot hint once the SDP carrying it has been accepted locally. + if (appliedVideoStartBitrate && offer.sdp === mungedSdp) { + this.hasAppliedVideoStartBitrate = true; + } this.onOffer(offer, this.latestOfferId); } finally { unlock();