Write the video start bitrate hint as one connection-level value, once

.changeset/video-start-bitrate-connection-level.md

@@ -0,0 +1,13 @@
+---
+'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 are currently sending.
+
+Only sending sections count. The list of registered track bitrates is append-only, and an unpublished section keeps its `a=msid`, so matching a section to a track by msid alone would still pair a stale entry with the section it used to occupy — letting an uncapped screen-share target seed a connection that now carries only a camera, or consuming the one-shot hint on a section that sends nothing, which would leave later publishes with no hint at all. The section's direction distinguishes them: `a=recvonly` and `a=inactive` cannot carry local media and are exactly where an unpublished or pre-populated section lands, while `a=sendonly`, `a=sendrecv` and an omitted direction all send.
+
+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.

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,159 @@ 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`;
+
+// The same bundle after the screen share is unpublished: `unpublishTrack` sets the
+// transceiver to `inactive`, but the section keeps its `a=msid`, so it still matches the
+// append-only trackBitrates entry. Only the camera is still sending.
+const UNPUBLISHED_SCREEN_SHARE = `v=0
+o=- 0 0 IN IP4 127.0.0.1
+s=-
+t=0 0
+a=group:BUNDLE 0 1
+m=video 9 UDP/TLS/RTP/SAVPF 96
+c=IN IP4 0.0.0.0
+a=mid:0
+a=inactive
+a=msid:PA_remote|camera other-track
+a=rtpmap:96 VP8/90000
+m=video 9 UDP/TLS/RTP/SAVPF 96
+c=IN IP4 0.0.0.0
+a=mid:1
+a=sendonly
+a=msid:PA_remote|camera camera-cid
+a=rtpmap:96 VP8/90000`;
+
+// The legacy `addTrack` fallback (no `addTransceiver` support) reuses a transceiver instead
+// of creating a sendonly one, so the published camera lands on a `sendrecv` section. Mid 1
+// omits the direction attribute entirely, which SDP also defaults to sendrecv. Both send.
+const LEGACY_ADD_TRACK_SECTIONS = `v=0
+o=- 0 0 IN IP4 127.0.0.1
+s=-
+t=0 0
+a=group:BUNDLE 0 1
+m=video 9 UDP/TLS/RTP/SAVPF 96
+c=IN IP4 0.0.0.0
+a=mid:0
+a=sendrecv
+a=msid:PA_remote|camera camera-cid
+a=rtpmap:96 VP8/90000
+m=video 9 UDP/TLS/RTP/SAVPF 96
+c=IN IP4 0.0.0.0
+a=mid:1
+a=msid:PA_remote|camera other-track
+a=rtpmap:96 VP8/90000`;
 
-    for (const section of media) {
-      applyVideoStartBitrate(section, 'camera-cid', 'VP8', 1_000);
-    }
+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);
+
+    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('ignores a section that stopped sending but kept its msid', () => {
+    const trackBitrates = [
+      { cid: 'camera-cid', codec: 'VP8', maxbr: 1_000 },
+      { cid: 'other-track', codec: 'VP8', maxbr: 8_000, isScreenShare: true },
+    ];
+
+    // While both send, the uncapped screen share wins the connection-level max.
+    expect(computeConnectionStartBitrate(parse(TWO_VIDEO_SECTIONS).media, trackBitrates)).toBe(
+      7_200,
+    );
+    // Once it is unpublished its entry is stale, so only the capped camera counts. Both
+    // directions a removed sender can land on are excluded: `inactive` from a sendonly
+    // transceiver, `recvonly` from the sendrecv one the addTrack fallback reuses.
+    expect(
+      computeConnectionStartBitrate(parse(UNPUBLISHED_SCREEN_SHARE).media, trackBitrates),
+    ).toBe(900);
+    expect(
+      computeConnectionStartBitrate(
+        parse(UNPUBLISHED_SCREEN_SHARE.replace('a=inactive', 'a=recvonly')).media,
+        trackBitrates,
+      ),
+    ).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();
+  });
+
+  it('counts sendrecv and direction-less sections, which still send local media', () => {
+    // The legacy addTrack fallback never produces `sendonly`, so a strict match on it would
+    // leave those clients with no hint at all.
+    const { media } = parse(LEGACY_ADD_TRACK_SECTIONS);
+
+    expect(
+      computeConnectionStartBitrate(media, [{ cid: 'camera-cid', codec: 'VP8', maxbr: 1_000 }]),
+    ).toBe(900);
+    expect(
+      computeConnectionStartBitrate(media, [
+        { cid: 'other-track', codec: 'VP8', maxbr: 2_000, isScreenShare: true },
+      ]),
+    ).toBe(1_800);
+  });
+
+  it('leaves the hint unset when only non-sending sections match', () => {
+    // Nothing is published, so the one-shot hint must not be consumed on a dead section.
+    const { media } = parse(UNPUBLISHED_SCREEN_SHARE);
+
+    expect(
+      computeConnectionStartBitrate(media, [
+        { cid: 'other-track', codec: 'VP8', maxbr: 8_000, isScreenShare: true },
+      ]),
+    ).toBeUndefined();
+  });
 });
 
 describe('placeholderMidsFromTransceivers', () => {