-
Notifications
You must be signed in to change notification settings - Fork 3
feat(realtime): startBitrateKbps connect option — seed the publisher's initial bandwidth estimate #193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ilay-decart
wants to merge
2
commits into
main
Choose a base branch
from
api-1486-start-bitrate-seed-option
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat(realtime): startBitrateKbps connect option — seed the publisher's initial bandwidth estimate #193
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| import type { Logger } from "../utils/logger"; | ||
|
|
||
| /** | ||
| * Video codecs whose fmtp carries `x-google-start-bitrate`. Payload types are | ||
| * matched from `a=rtpmap` so retransmission/FEC payloads (rtx, red, ulpfec — | ||
| * also clocked at /90000) are never munged. | ||
| */ | ||
| const VIDEO_CODEC_RTPMAP = /^a=rtpmap:(\d+) (VP8|VP9|H264|H265|AV1)\/90000/i; | ||
|
|
||
| /** | ||
| * Append `x-google-start-bitrate=<kbps>` to every video codec's fmtp in the | ||
| * SDP (adding an fmtp line when the codec has none — VP8 typically doesn't). | ||
| * | ||
| * libwebrtc reads the parameter off the applied session descriptions and uses | ||
| * it as the initial send-side bandwidth estimate, replacing the stock | ||
| * ~300 kbps cold start. Idempotent: descriptions that already carry the | ||
| * parameter (e.g. on a renegotiation of an already-munged session) are left | ||
| * untouched. | ||
| */ | ||
| export function mungeStartBitrate(sdp: string, startKbps: number): string { | ||
| if (!(startKbps > 0)) return sdp; | ||
| const lines = sdp.split("\r\n"); | ||
|
|
||
| const videoPts = new Set<string>(); | ||
| const ptsWithFmtp = new Set<string>(); | ||
| for (const line of lines) { | ||
| const rtpmap = line.match(VIDEO_CODEC_RTPMAP); | ||
| if (rtpmap?.[1]) videoPts.add(rtpmap[1]); | ||
| const fmtp = line.match(/^a=fmtp:(\d+) /); | ||
| if (fmtp?.[1]) ptsWithFmtp.add(fmtp[1]); | ||
| } | ||
| if (videoPts.size === 0) return sdp; | ||
|
|
||
| const param = `x-google-start-bitrate=${startKbps}`; | ||
| const out: string[] = []; | ||
| for (const line of lines) { | ||
| const fmtp = line.match(/^a=fmtp:(\d+) (.+)$/); | ||
| if (fmtp?.[1] && fmtp[2] && videoPts.has(fmtp[1]) && !line.includes("x-google-start-bitrate")) { | ||
| out.push(`a=fmtp:${fmtp[1]} ${fmtp[2]};${param}`); | ||
| continue; | ||
| } | ||
| out.push(line); | ||
| const rtpmap = line.match(VIDEO_CODEC_RTPMAP); | ||
| if (rtpmap?.[1] && !ptsWithFmtp.has(rtpmap[1])) { | ||
| // Codec has no fmtp line anywhere in the SDP — give it one so the | ||
| // parameter still reaches libwebrtc (fmtp placement after rtpmap is | ||
| // valid and conventional). | ||
| out.push(`a=fmtp:${rtpmap[1]} ${param}`); | ||
| ptsWithFmtp.add(rtpmap[1]); | ||
| } | ||
| } | ||
| return out.join("\r\n"); | ||
| } | ||
|
|
||
| /** | ||
| * Patch the global `RTCPeerConnection` so every description applied while the | ||
| * patch is installed — local offers and remote answers alike, across LiveKit | ||
| * reconnects — carries the start-bitrate parameter. Returns an uninstaller. | ||
| * | ||
| * Patching the global is deliberate: livekit-client owns its peer connections | ||
| * and exposes no SDP hook, and this is the exact mechanism validated on the | ||
| * probe rig. The uninstaller restores the original constructor only if nobody | ||
| * else has re-patched the global since (never clobbers a foreign patch). | ||
| */ | ||
| export function installStartBitrateMunge(startKbps: number, logger?: Logger): () => void { | ||
| if (!(startKbps > 0)) return () => {}; | ||
| const g = globalThis as { RTCPeerConnection?: typeof RTCPeerConnection }; | ||
| const OriginalPC = g.RTCPeerConnection; | ||
| if (typeof OriginalPC !== "function") { | ||
| logger?.warn("startBitrateKbps ignored: no global RTCPeerConnection in this environment"); | ||
| return () => {}; | ||
| } | ||
|
|
||
| let logged = false; | ||
| const logOnce = (leg: string) => { | ||
| if (logged) return; | ||
| logged = true; | ||
| logger?.info(`publisher start-bitrate seeded to ${startKbps} kbps (x-google-start-bitrate, ${leg})`); | ||
| }; | ||
|
|
||
| const munged = <T extends { type?: RTCSdpType; sdp?: string }>(description: T, leg: string): T => { | ||
| if (typeof description.sdp !== "string") return description; | ||
| const sdp = mungeStartBitrate(description.sdp, startKbps); | ||
| if (sdp === description.sdp) return description; | ||
| logOnce(leg); | ||
| return { ...description, sdp }; | ||
| }; | ||
|
|
||
| class StartBitratePC extends OriginalPC { | ||
| // Seed ONLY the first offer/answer pair. libwebrtc re-initializes the | ||
| // send-side estimate to x-google-start-bitrate on EVERY applied | ||
| // description that carries it — munging renegotiations (e.g. LiveKit's | ||
| // track-publish round) resets an already-converged estimator back down | ||
| // to the seed (measured live: est 5301 kbps → 1106 kbps at the second | ||
| // negotiation, exactly the seed value). | ||
| #mungedLegs = 0; | ||
|
|
||
| #maybeMunge<T extends { type?: RTCSdpType; sdp?: string }>(description: T, leg: string): T { | ||
| if (this.#mungedLegs >= 2) return description; | ||
| const result = munged(description, leg); | ||
| if (result !== description) this.#mungedLegs++; | ||
| return result; | ||
| } | ||
|
|
||
| override setLocalDescription(description?: RTCLocalSessionDescriptionInit): Promise<void> { | ||
| // Argless form (implicit offer/answer) has no SDP to munge — the | ||
| // browser builds the description internally; the remote-answer leg | ||
| // still seeds those sessions. | ||
| if (description === undefined) return super.setLocalDescription(); | ||
| return super.setLocalDescription(this.#maybeMunge(description, "local offer")); | ||
| } | ||
| override setRemoteDescription(description: RTCSessionDescriptionInit): Promise<void> { | ||
| return super.setRemoteDescription(this.#maybeMunge(description, "remote answer")); | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| g.RTCPeerConnection = StartBitratePC as typeof RTCPeerConnection; | ||
| } catch (error) { | ||
| logger?.warn("startBitrateKbps ignored: RTCPeerConnection global is not patchable", { | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| return () => {}; | ||
| } | ||
|
|
||
| return () => { | ||
| if (g.RTCPeerConnection === StartBitratePC) { | ||
| g.RTCPeerConnection = OriginalPC; | ||
| } | ||
| }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| import { afterEach, describe, expect, it, vi } from "vitest"; | ||
| import { installStartBitrateMunge, mungeStartBitrate } from "../src/realtime/start-bitrate.js"; | ||
|
|
||
| const SDP = [ | ||
| "v=0", | ||
| "o=- 1 1 IN IP4 127.0.0.1", | ||
| "m=audio 9 UDP/TLS/RTP/SAVPF 111", | ||
| "a=rtpmap:111 opus/48000/2", | ||
| "a=fmtp:111 minptime=10;useinbandfec=1", | ||
| "m=video 9 UDP/TLS/RTP/SAVPF 96 97 98 99 100", | ||
| "a=rtpmap:96 VP9/90000", | ||
| "a=fmtp:96 profile-id=0", | ||
| "a=rtpmap:97 rtx/90000", | ||
| "a=fmtp:97 apt=96", | ||
| "a=rtpmap:98 VP8/90000", | ||
| "a=rtpmap:99 H264/90000", | ||
| "a=fmtp:99 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f", | ||
| "a=rtpmap:100 red/90000", | ||
| "", | ||
| ].join("\r\n"); | ||
|
|
||
| describe("mungeStartBitrate", () => { | ||
| it("appends the parameter to every video codec fmtp — VP9 and H264, both payload types", () => { | ||
| const out = mungeStartBitrate(SDP, 1100); | ||
| expect(out).toContain("a=fmtp:96 profile-id=0;x-google-start-bitrate=1100"); | ||
| expect(out).toContain( | ||
| "a=fmtp:99 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f;x-google-start-bitrate=1100", | ||
| ); | ||
| }); | ||
|
|
||
| it("adds an fmtp line for video codecs that have none (VP8)", () => { | ||
| const out = mungeStartBitrate(SDP, 1100); | ||
| const lines = out.split("\r\n"); | ||
| const vp8Index = lines.indexOf("a=rtpmap:98 VP8/90000"); | ||
| expect(lines[vp8Index + 1]).toBe("a=fmtp:98 x-google-start-bitrate=1100"); | ||
| }); | ||
|
|
||
| it("never touches audio, rtx, or red payloads", () => { | ||
| const out = mungeStartBitrate(SDP, 1100); | ||
| expect(out).toContain("a=fmtp:111 minptime=10;useinbandfec=1"); | ||
| expect(out).toContain("a=fmtp:97 apt=96"); | ||
| expect(out).not.toContain("a=fmtp:97 apt=96;x-google"); | ||
| expect(out).not.toContain("a=fmtp:100"); | ||
| }); | ||
|
|
||
| it("is idempotent: an already-munged description is returned unchanged", () => { | ||
| const once = mungeStartBitrate(SDP, 1100); | ||
| expect(mungeStartBitrate(once, 1100)).toBe(once); | ||
| }); | ||
|
|
||
| it("is the identity for non-positive values and video-free SDPs", () => { | ||
| expect(mungeStartBitrate(SDP, 0)).toBe(SDP); | ||
| const audioOnly = "m=audio 9 UDP/TLS/RTP/SAVPF 111\r\na=rtpmap:111 opus/48000/2"; | ||
| expect(mungeStartBitrate(audioOnly, 1100)).toBe(audioOnly); | ||
| }); | ||
| }); | ||
|
|
||
| type AnyDescription = { type?: string; sdp?: string } | undefined; | ||
|
|
||
| class FakePC { | ||
| static instances: FakePC[] = []; | ||
| lastLocal: AnyDescription | "ARGLESS"; | ||
| lastRemote: AnyDescription; | ||
| constructor() { | ||
| FakePC.instances.push(this); | ||
| } | ||
| setLocalDescription(...args: [AnyDescription?]): Promise<void> { | ||
| this.lastLocal = args.length === 0 ? "ARGLESS" : args[0]; | ||
| return Promise.resolve(); | ||
| } | ||
| setRemoteDescription(description: AnyDescription): Promise<void> { | ||
| this.lastRemote = description; | ||
| return Promise.resolve(); | ||
| } | ||
| } | ||
|
|
||
| const g = globalThis as { RTCPeerConnection?: unknown }; | ||
|
|
||
| describe("installStartBitrateMunge", () => { | ||
| afterEach(() => { | ||
| delete g.RTCPeerConnection; | ||
| FakePC.instances = []; | ||
| }); | ||
|
|
||
| it("munges local offers and remote answers on connections created while installed", async () => { | ||
| g.RTCPeerConnection = FakePC; | ||
| const uninstall = installStartBitrateMunge(1100); | ||
| const pc = new (g.RTCPeerConnection as new () => FakePC)(); | ||
|
|
||
| await pc.setLocalDescription({ type: "offer", sdp: SDP }); | ||
| expect((pc.lastLocal as { sdp: string }).sdp).toContain("x-google-start-bitrate=1100"); | ||
|
|
||
| await pc.setRemoteDescription({ type: "answer", sdp: SDP }); | ||
| expect((pc.lastRemote as { sdp: string }).sdp).toContain("x-google-start-bitrate=1100"); | ||
| uninstall(); | ||
| }); | ||
|
|
||
| it("seeds only the first offer/answer pair — renegotiations pass through unmunged", async () => { | ||
| g.RTCPeerConnection = FakePC; | ||
| const uninstall = installStartBitrateMunge(1100); | ||
| const pc = new (g.RTCPeerConnection as new () => FakePC)(); | ||
|
|
||
| await pc.setLocalDescription({ type: "offer", sdp: SDP }); | ||
| await pc.setRemoteDescription({ type: "answer", sdp: SDP }); | ||
| expect((pc.lastLocal as { sdp: string }).sdp).toContain("x-google-start-bitrate=1100"); | ||
| expect((pc.lastRemote as { sdp: string }).sdp).toContain("x-google-start-bitrate=1100"); | ||
|
|
||
| // Renegotiation (track publish): re-munging would reset a converged | ||
| // estimator back to the seed — these must pass through untouched. | ||
| await pc.setLocalDescription({ type: "offer", sdp: SDP }); | ||
| await pc.setRemoteDescription({ type: "answer", sdp: SDP }); | ||
| expect((pc.lastLocal as { sdp: string }).sdp).not.toContain("x-google-start-bitrate"); | ||
| expect((pc.lastRemote as { sdp: string }).sdp).not.toContain("x-google-start-bitrate"); | ||
|
|
||
| // A NEW peer connection (reconnect) gets its own first-pair seeding. | ||
| const pc2 = new (g.RTCPeerConnection as new () => FakePC)(); | ||
| await pc2.setLocalDescription({ type: "offer", sdp: SDP }); | ||
| expect((pc2.lastLocal as { sdp: string }).sdp).toContain("x-google-start-bitrate=1100"); | ||
| uninstall(); | ||
| }); | ||
|
|
||
| it("passes the argless setLocalDescription form through untouched", async () => { | ||
| g.RTCPeerConnection = FakePC; | ||
| const uninstall = installStartBitrateMunge(1100); | ||
| const pc = new (g.RTCPeerConnection as new () => FakePC)(); | ||
| await pc.setLocalDescription(); | ||
| expect(pc.lastLocal).toBe("ARGLESS"); | ||
| uninstall(); | ||
| }); | ||
|
|
||
| it("logs once on the first munged description", async () => { | ||
| g.RTCPeerConnection = FakePC; | ||
| const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; | ||
| const uninstall = installStartBitrateMunge(1100, logger); | ||
| const pc = new (g.RTCPeerConnection as new () => FakePC)(); | ||
| await pc.setLocalDescription({ type: "offer", sdp: SDP }); | ||
| await pc.setRemoteDescription({ type: "answer", sdp: SDP }); | ||
| expect(logger.info).toHaveBeenCalledTimes(1); | ||
| expect(logger.info.mock.calls[0]?.[0]).toContain("1100 kbps"); | ||
| uninstall(); | ||
| }); | ||
|
|
||
| it("uninstall restores the original constructor, but never clobbers a foreign patch", () => { | ||
| g.RTCPeerConnection = FakePC; | ||
| const uninstall = installStartBitrateMunge(1100); | ||
| expect(g.RTCPeerConnection).not.toBe(FakePC); | ||
|
|
||
| const foreign = class {}; | ||
| g.RTCPeerConnection = foreign; | ||
| uninstall(); | ||
| expect(g.RTCPeerConnection).toBe(foreign); | ||
|
|
||
| g.RTCPeerConnection = FakePC; | ||
| const uninstall2 = installStartBitrateMunge(1100); | ||
| uninstall2(); | ||
| expect(g.RTCPeerConnection).toBe(FakePC); | ||
| }); | ||
|
|
||
| it("is a no-op without a global RTCPeerConnection or for non-positive values", () => { | ||
| const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; | ||
| expect(installStartBitrateMunge(1100, logger)).toBeTypeOf("function"); | ||
| expect(logger.warn).toHaveBeenCalledOnce(); | ||
|
|
||
| g.RTCPeerConnection = FakePC; | ||
| installStartBitrateMunge(0)(); | ||
| expect(g.RTCPeerConnection).toBe(FakePC); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overlapping patches leak global PC
Medium Severity
Overlapping
installStartBitrateMungecalls nest subclasses on the globalRTCPeerConnection. The older uninstaller no-ops because the global is no longer its class, and the newer one restores that older subclass instead of the true original, so after every session ends the global stays patched and later page WebRTC still gets SDP munged.Additional Locations (1)
packages/sdk/src/realtime/media-channel.ts#L101-L107Reviewed by Cursor Bugbot for commit 2207a5f. Configure here.