Conform bundled codec fmtp across receive sections by xianshijing-lk · Pull Request #1163 · livekit/client-sdk-swift

xianshijing-lk · 2026-10-20T22:41:13Z

What

A payload type is bundle-wide, so libwebrtc expects it to map to identical codec parameters in every m-line, and reports a collision where it does not:

A BUNDLE group contains a codec collision between
  {payload_type: 100, codec: {mime_type: video/VP8, parameters: {x-google-start-bitrate: 315}}}
  and {payload_type: 100, codec: {mime_type: video/VP8}}

Single peer connection mode creates exactly that divergence: mungeVideoStartBitrate writes x-google-start-bitrate on the sections that send video, while the receive sections the same connection carries for subscribed media share those payload types without it.

This adds a conformBundledCodecFmtp munge, run last in the chain, that rewrites receive-only sections so every shared payload type carries one config. Sending sections always win, so a published track's encoder parameters are never altered. It is codec-agnostic — it conforms whatever diverged, not x-google-start-bitrate specifically.

Scope, honestly

On the local-description path this collision is logged, not enforced. I verified against the shipped libwebrtc that setLocalDescription succeeds, the munge is not dropped, and the start-bitrate hint reaches the peer connection either way. So this is a correctness/hygiene fix for self-inconsistent SDP, not a latency fix — please don't read it as one.

Measured effect on a staging benchmark run: BUNDLE collisions per run drop from 546 to ~295.

The remainder is the server's answer, which carries the same divergence on every single-PC negotiation. That is deliberately not addressed here: conforming an answer requires selecting sections by transceiver rather than by direction (an answer mirrors direction, so this predicate would rewrite exactly the wrong sections) and munging a remote description, which the SDK has no mechanism for today. client-sdk-js does both; this ports the offer half.

Not dependent on #1112

main already adds recvOnly transceivers to the publisher connection when MediaSectionsRequirement arrives, and already writes the start bitrate, so the divergence is reachable without any of #1112's preallocation. #1112 only makes it fire on every publisher offer instead of after the first subscription. No code dependency, no overlapping hunks — can land in either order.

Tests

BundledCodecFmtpTests — 9 unit tests: rewrite and insert paths, section ordering, inactive, no-direction-means-sendrecv, idempotence, convergence with no sending section, and the start-bitrate → conform invariant.
TransportMungeFallbackTests — 2 tests against a real peer connection: the unconformed munge is accepted but leaves the bundle inconsistent; the conformed one leaves every shared payload type with one config.

Also adds SDPMediaSection.setFmtp(config:forPayload:), which overwrites a whole fmtp config and inserts a line where the payload has none — the VP8 case, which libwebrtc offers without an fmtp line and which conformBundledCodecFmtp in client-sdk-js would miss.

🤖 Generated with Claude Code

A payload type is bundle-wide, so libwebrtc expects it to map to identical codec parameters in every m-line and reports "A BUNDLE group contains a codec collision between ..." where it does not. Single peer connection mode creates exactly that divergence: the video start bitrate is written on the sections that send video, while the receive sections the same connection carries for subscribed media share those payload types without it. Conform the receive sections to the sending ones after munging. Sending sections always win, so a published track's encoder parameters are never altered; where no sending section declares a payload type the receive sections converge on the first config seen. Codec-agnostic by design — it conforms whatever diverged rather than `x-google-start-bitrate` specifically, which is what keeps it correct as other munges are added. On the local-description path the collision is logged rather than enforced: the description is accepted, the munge is not dropped, and the hint reaches the peer connection either way. This runs because an SDP whose bundle is self-inconsistent is still wrong, and because what libwebrtc tolerates here it does not everywhere. The same divergence in the server's answer is reported on every single-PC negotiation and is not addressed here, since conforming an answer means selecting sections by transceiver rather than by direction (an answer mirrors it) and munging a remote description. Mirrors `conformBundledCodecFmtp` in client-sdk-js. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The canonical map was built by walking fmtp lines, so a payload a sending section declares in `a=rtpmap` with no `a=fmtp` contributed nothing. If a receive section carried parameters for that payload its config became canonical, and since the rewrite only visits receive sections the collision survived: the sender still advertised the payload bare. Walk the sending sections' rtpmaps instead and record the empty config where there is no fmtp, then conform a receive section to it by removing its fmtp line rather than writing one. "No parameters" collides with "some parameters" exactly as two different parameter sets do, so it has to be representable. Reachable whenever a receive section has fmtp for a payload the sender leaves bare — VP8 being the case that matters, since libwebrtc offers it without an fmtp line until something adds one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pblazej

Requesting changes for the removal test at BundledCodecFmtpTests.swift:144. The other comments are non-blocking, and the munge runs correctly inside @RTC.

pblazej · 2026-10-29T08:41:31Z

+    /// receive section carrying parameters for it is stripped rather than left to collide. The
+    /// rewrite only visits receive sections, so recording the absence is the only way this
+    /// resolves.
+    @Test func sendingSectionWithoutFmtpStripsTheReceiveSection() {


issue (blocking): This test still passes with the removeFmtp branch deleted, because the receive section never has an fmtp for payload 100.

Give the receive section a=fmtp:100 x-google-start-bitrate=900, keep the sender bare, and run both section orders.

pblazej · 2026-10-29T08:41:31Z

+            // resolve it. A sending section overwrites unconditionally, so it wins regardless
+            // of the order the sections appear in.
+            for rtpmap in section.rtpmaps {
+                canonicalByPayload[rtpmap.payload] = section.fmtp(forPayload: rtpmap.payload)?.config ?? ""


issue (non-blocking): When two sending sections disagree, the last one wins and the collision stays. libwebrtc produces this on the offer after a second video publish.

Let the first sender win (canonicalByPayload[p] = canonicalByPayload[p] ?? config), and add a test for two disagreeing senders.

pblazej · 2026-10-29T08:41:31Z

            offer = try await set(localDescription: offer, munging: singlePCMode
-                ? [Self.mungeInactiveToRecvOnlyForMedia, Self.mungeOpusStereoForAllAudio, startBitrate]
-                : [startBitrate])
+                ? [Self.mungeInactiveToRecvOnlyForMedia, Self.mungeOpusStereoForAllAudio, startBitrate, Self.conformBundledCodecFmtp]


suggestion (non-blocking): No test fails if this conformBundledCodecFmtp call is removed.

A test through createAndSendOffer that reads transport.localDescription pins the wiring.

pblazej · 2026-10-29T08:41:31Z

+    /// `recvonly`, and a section whose track was unpublished reverts to one or the other.
+    /// Everything else sends — `sendonly`, `sendrecv`, and a section with no direction
+    /// attribute, which SDP defaults to `sendrecv`.
+    nonisolated static func isReceiveOnlySection(_ section: SDPMediaSection) -> Bool {


polish: isSendingVideoSection can be mediaType == "video" && !isReceiveOnlySection(section), so the direction rule lives in one place.

pblazej · 2026-10-29T08:41:31Z

+        section.direction == .recvonly || section.direction == .inactive
+    }
+
+    /// Munge the offer so every payload type carries identical fmtp parameters across the BUNDLE


nitpick: The 34-line docstring repeats the PR description, and it says "across the BUNDLE group" while the code works across the whole document.

pblazej · 2026-10-29T08:41:31Z

+                canonicalByPayload[rtpmap.payload] = section.fmtp(forPayload: rtpmap.payload)?.config ?? ""
+            }
+        }
+        for section in document.mediaSections where isReceiveOnlySection(section) {


question: Which munge makes two receive sections disagree with each other?

If none does, this loop, its test, and the guard on line 673 can go.

pblazej · 2026-10-29T08:41:32Z

+    /// Every payload type's distinct fmtp configs across the document, keyed by payload. A
+    /// payload declared by `a=rtpmap` with no fmtp line contributes the empty config, since
+    /// libwebrtc counts "parameters" against "no parameters" as a collision too.
+    private func fmtpConfigsByPayload(_ sdp: String) -> [String: Set<String>] {


nitpick: This copies fmtpConfigsByPayload from BundledCodecFmtpTests.swift:171. Make that one non-private and reuse it.

pblazej · 2026-10-29T08:41:32Z

+    ///
+    /// Note what it does *not* do: `setLocalDescription` still succeeds, the munge is not
+    /// dropped, and the hint survives. The collision is logged, not enforced, on this path.
+    @Test func startBitrateAloneLeavesTheBundleInconsistent() async throws {


praise: A real peer connection backing the "logged, not enforced" claim is exactly the right evidence.

xianshijing-lk requested review from hiroshihorie and pblazej as code owners October 20, 2026 22:41

This comment was marked as resolved.

Sign in to view

pblazej requested changes Oct 29, 2026

View reviewed changes

Navigation Menu

Repository navigation

Conform bundled codec fmtp across receive sections - #1163

Conform bundled codec fmtp across receive sections#1163
xianshijing-lk wants to merge 2 commits into
mainfrom
sxian/conform-bundled-codec-fmtp

xianshijing-lk commented Oct 20, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

pblazej left a comment

Uh oh!

pblazej Oct 29, 2026

Uh oh!

pblazej Oct 29, 2026

Uh oh!

pblazej Oct 29, 2026

Uh oh!

pblazej Oct 29, 2026

Uh oh!

pblazej Oct 29, 2026

Uh oh!

pblazej Oct 29, 2026

Uh oh!

pblazej Oct 29, 2026

Uh oh!

pblazej Oct 29, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

xianshijing-lk commented Oct 20, 2026

What

Scope, honestly

Not dependent on #1112

Tests

Uh oh!

This comment was marked as resolved.

Uh oh!

pblazej left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants