Conform bundled codec fmtp across receive sections

.changes/conform-bundled-codec-fmtp

@@ -0,0 +1 @@
+patch type="fixed" "Publisher offers now keep one fmtp config per payload type across the BUNDLE group, by conforming receive-only sections to the sending ones after munging — previously the video start bitrate written on a sending section left the preallocated receive sections of single peer connection mode declaring the same payload types without it"

Sources/LiveKit/Core/Transport.swift

@@ -324,9 +324,13 @@ final class Transport: NSObject, Loggable {
             let startBitrate: (String) -> String = {
                 hasAppliedStartBitrate ? $0 : Self.mungeVideoStartBitrate($0, kbpsBySenderId: kbpsBySenderId)
             }
+            // `conformBundledCodecFmtp` runs last in both modes: it repairs whatever the munges
+            // before it made inconsistent across the BUNDLE group, and it is a no-op on an offer
+            // with no receive-only section — which is every dual PC offer until a track is
+            // unpublished.
             offer = try await set(localDescription: offer, munging: singlePCMode
-                ? [Self.mungeInactiveToRecvOnlyForMedia, Self.mungeOpusStereoForAllAudio, startBitrate]
-                : [startBitrate])
+                ? [Self.mungeInactiveToRecvOnlyForMedia, Self.mungeOpusStereoForAllAudio, startBitrate, Self.conformBundledCodecFmtp]
+                : [startBitrate, Self.conformBundledCodecFmtp])
             // A rejected munge is dropped and the offer retried without it, so the one-shot
             // hint is consumed only once the description that carries it has been accepted.
             if !_hasAppliedVideoStartBitrate, Self.carriesVideoStartBitrate(offer.sdp) {
@@ -599,6 +603,91 @@ extension Transport {
         return modified ? document.write() : sdp
     }
 
+    /// Whether `section` cannot carry local media, and so is one of the sections
+    /// ``conformBundledCodecFmtp(_:)`` rewrites.
+    ///
+    /// `recvonly` and `inactive` are exactly the two directions a section lands on when it has
+    /// no outgoing track: the receive sections preallocated for single PC mode are offered
+    /// `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 {
+        section.direction == .recvonly || section.direction == .inactive
+    }
+
+    /// Munge the offer so every payload type carries identical fmtp parameters across the BUNDLE
+    /// group, by conforming the sections that cannot send to the ones that can.
+    ///
+    /// 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 PC mode creates exactly that divergence:
+    /// ``mungeVideoStartBitrate(_:kbpsBySenderId:)`` writes `x-google-start-bitrate` on the
+    /// sections that send video, while the receive sections preallocated for subscribed media
+    /// share those payload types without it.
+    ///
+    /// On the local-description path that report is a log, not an enforcement: 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, which this cannot
+    /// reach, since conforming an answer means selecting sections by transceiver rather than by
+    /// direction (an answer mirrors it) and munging a remote description.
+    ///
+    /// 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 so two of them cannot disagree either. It is 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 — ``mungeOpusStereoForAllAudio(_:)`` only
+    /// avoids the same fate by writing `stereo=1` to receive sections too.
+    ///
+    /// Must run last, after every munge that can introduce divergence.
+    ///
+    /// Mirrors `conformBundledCodecFmtp` in client-sdk-js, with two differences. That version
+    /// selects the sections to rewrite by transceiver — those with no outgoing track — where
+    /// this reads the direction the SDP already states, which libwebrtc derives from that same
+    /// state and which keeps the munge a pure `String -> String` like its siblings. And it only
+    /// rewrites fmtp lines that exist, where this inserts one when a receive section declares
+    /// the payload in an `a=rtpmap` but has no fmtp for it; "parameters vs. no parameters" is a
+    /// collision too, and it is the one VP8 hits, since libwebrtc offers it without an fmtp line.
+    nonisolated static func conformBundledCodecFmtp(_ sdp: String) -> String {
+        var document = SDP(parsing: sdp)
+
+        var canonicalByPayload: [String: String] = [:]
+        for section in document.mediaSections where !isReceiveOnlySection(section) {
+            // Every payload a sending section declares is canonical, including the ones it
+            // declares with no fmtp at all: "no parameters" collides with "some parameters"
+            // exactly as two different parameter sets do, so the absence has to be recorded or
+            // a receive section's fmtp would become canonical for a payload the sender leaves
+            // bare — and the rewrite below, which only touches receive sections, could never
+            // 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 ?? ""
+            }
+        }
+        for section in document.mediaSections where isReceiveOnlySection(section) {
+            for fmtp in section.fmtps where canonicalByPayload[fmtp.payload] == nil {
+                canonicalByPayload[fmtp.payload] = fmtp.config
+            }
+        }
+        guard !canonicalByPayload.isEmpty else { return sdp }
+
+        var modified = false
+        for index in document.mediaSections.indices where isReceiveOnlySection(document.mediaSections[index]) {
+            // Keyed off `a=rtpmap` rather than the existing fmtp lines: a payload the section
+            // declares but gives no parameters still collides with a sending section that gives
+            // some, and that section has no fmtp line to iterate.
+            for rtpmap in document.mediaSections[index].rtpmaps {
+                guard let canonical = canonicalByPayload[rtpmap.payload] else { continue }
+                let changed = canonical.isEmpty
+                    ? document.mediaSections[index].removeFmtp(forPayload: rtpmap.payload)
+                    : document.mediaSections[index].setFmtp(config: canonical, forPayload: rtpmap.payload)
+                if changed { modified = true }
+            }
+        }
+        return modified ? document.write() : sdp
+    }
+
     /// Whether any video section of `sdp` declares `x-google-start-bitrate`. Checked on the
     /// description libwebrtc accepted, since a rejected munge is dropped rather than applied.
     nonisolated static func carriesVideoStartBitrate(_ sdp: String) -> Bool {

Sources/LiveKit/Support/SDP/SDP.swift

@@ -152,6 +152,39 @@ struct SDPMediaSection {
         return true
     }
 
+    /// Replaces the whole fmtp config of `payload`, inserting an `a=fmtp:<payload> <config>`
+    /// line at the end of the section when the payload has none — which is the case libwebrtc
+    /// leaves for codecs it offers without parameters, VP8 among them. Returns `true` if the
+    /// section was modified; `false` when the config is already exactly `config`.
+    ///
+    /// Where ``setFmtpParameter(_:value:forPayload:)`` edits one parameter and leaves the rest,
+    /// this overwrites all of them — for conforming a section to a config decided elsewhere.
+    @discardableResult
+    mutating func setFmtp(config: String, forPayload payload: String) -> Bool {
+        for (index, line) in lines.enumerated() {
+            guard let fmtp = Self.fmtp(fromLine: line), fmtp.payload == payload else { continue }
+            guard fmtp.config != config else { return false }
+            lines[index] = "a=fmtp:\(payload) \(config)"
+            return true
+        }
+        lines.append("a=fmtp:\(payload) \(config)")
+        return true
+    }
+
+    /// Removes the fmtp line of `payload`, if any. Returns `true` if the section was modified.
+    ///
+    /// The counterpart to ``setFmtp(config:forPayload:)`` for conforming a section to a codec
+    /// that is declared with no parameters at all: "no fmtp" is a config like any other, and a
+    /// section cannot be conformed to it by writing.
+    @discardableResult
+    mutating func removeFmtp(forPayload payload: String) -> Bool {
+        guard let index = lines.indices.first(where: {
+            Self.fmtp(fromLine: lines[$0])?.payload == payload
+        }) else { return false }
+        lines.remove(at: index)
+        return true
+    }
+
     /// Whether an `a=rtcp-fb:<payload> <value>` line with exactly `value` exists for
     /// `payload`. Exact-value match: `nack` does not match `nack pli` (RFC 4585 §4.2
     /// — they are distinct feedback parameters).

Tests/LiveKitCoreTests/SDP/BundledCodecFmtpTests.swift

@@ -0,0 +1,180 @@
+/*
+ * Copyright 2026 LiveKit
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+@testable import LiveKit
+import Testing
+
+@Suite(.tags(.media))
+struct BundledCodecFmtpTests {
+    /// The single PC publisher shape: a sending Opus section and a sending video section
+    /// carrying the start-bitrate hint, each paired with a receive section preallocated for
+    /// subscribed media that shares its payload types — payload 100 (VP8) without an fmtp line
+    /// at all, which is how libwebrtc offers it.
+    private static let singlePCOffer = """
+    v=0
+    o=- 0 0 IN IP4 127.0.0.1
+    s=-
+    t=0 0
+    a=group:BUNDLE 0 1 2 3
+    m=audio 9 UDP/TLS/RTP/SAVPF 111
+    a=mid:0
+    a=msid:- mic
+    a=rtpmap:111 opus/48000/2
+    a=fmtp:111 minptime=10;useinbandfec=1;stereo=1
+    a=sendonly
+    m=audio 9 UDP/TLS/RTP/SAVPF 111
+    a=mid:1
+    a=rtpmap:111 opus/48000/2
+    a=fmtp:111 minptime=10;useinbandfec=1
+    a=recvonly
+    m=video 9 UDP/TLS/RTP/SAVPF 96 100
+    a=mid:2
+    a=msid:- cam
+    a=rtpmap:96 H264/90000
+    a=fmtp:96 packetization-mode=1;x-google-start-bitrate=900
+    a=rtpmap:100 VP8/90000
+    a=fmtp:100 x-google-start-bitrate=900
+    a=sendonly
+    m=video 9 UDP/TLS/RTP/SAVPF 96 100
+    a=mid:3
+    a=rtpmap:96 H264/90000
+    a=fmtp:96 packetization-mode=1
+    a=rtpmap:100 VP8/90000
+    a=recvonly
+    """.replacingOccurrences(of: "\n", with: "\r\n") + "\r\n"
+
+    /// Both ways a receive section can diverge are repaired: payload 96 has an fmtp line
+    /// missing the parameter, payload 100 has no fmtp line at all.
+    @Test func conformsReceiveSectionsToTheSendingConfig() {
+        let sections = SDP(parsing: Transport.conformBundledCodecFmtp(Self.singlePCOffer)).mediaSections
+
+        #expect(sections[1].fmtps.map(\.config) == ["minptime=10;useinbandfec=1;stereo=1"])
+        #expect(sections[3].fmtps.map(\.config) == ["packetization-mode=1;x-google-start-bitrate=900",
+                                                    "x-google-start-bitrate=900"])
+        // Inserted rather than rewritten, so it lands at the end of its own section.
+        #expect(sections[3].lines.last == "a=fmtp:100 x-google-start-bitrate=900")
+    }
+
+    @Test func leavesSendingSectionsUntouched() {
+        let conformed = SDP(parsing: Transport.conformBundledCodecFmtp(Self.singlePCOffer)).mediaSections
+        let original = SDP(parsing: Self.singlePCOffer).mediaSections
+
+        #expect(conformed[0].lines == original[0].lines)
+        #expect(conformed[2].lines == original[2].lines)
+    }
+
+    /// A sending section is authoritative wherever it appears, so an offer that lists the
+    /// receive sections first conforms the same way.
+    @Test func sendingSectionWinsRegardlessOfOrder() {
+        let reordered = SDP(parsing: Self.singlePCOffer)
+        var swapped = reordered
+        swapped.mediaSections = [reordered.mediaSections[1], reordered.mediaSections[0],
+                                 reordered.mediaSections[3], reordered.mediaSections[2]]
+
+        let sections = SDP(parsing: Transport.conformBundledCodecFmtp(swapped.write())).mediaSections
+
+        #expect(sections[0].fmtps.map(\.config) == ["minptime=10;useinbandfec=1;stereo=1"])
+        #expect(sections[2].fmtps.map(\.config) == ["packetization-mode=1;x-google-start-bitrate=900",
+                                                    "x-google-start-bitrate=900"])
+    }
+
+    /// With no sending section to copy from, the receive sections still converge on one
+    /// config — two of them disagreeing is a bundle collision just the same.
+    @Test func receiveSectionsConvergeWithoutASendingSection() {
+        let receiveOnly = Self.singlePCOffer
+            .replacingOccurrences(of: "a=fmtp:96 packetization-mode=1;x-google-start-bitrate=900\r\n", with: "")
+            .replacingOccurrences(of: "a=fmtp:100 x-google-start-bitrate=900\r\n", with: "")
+            .replacingOccurrences(of: "a=sendonly", with: "a=recvonly")
+
+        let sections = SDP(parsing: Transport.conformBundledCodecFmtp(receiveOnly)).mediaSections
+
+        #expect(Set(sections[0].fmtps.map(\.config)) == Set(sections[1].fmtps.map(\.config)))
+        #expect(sections[2].fmtps.map(\.config) == sections[3].fmtps.map(\.config))
+    }
+
+    /// `inactive` is the other direction a section with no outgoing track lands on — an
+    /// unpublished track leaves one behind — so it is conformed too.
+    @Test func treatsInactiveAsReceiveOnly() {
+        let inactive = Self.singlePCOffer.replacingOccurrences(of: "a=mid:3\r\n", with: "a=mid:3\r\na=inactive\r\n")
+        let sections = SDP(parsing: Transport.conformBundledCodecFmtp(inactive)).mediaSections
+
+        #expect(sections[3].fmtps.map(\.config) == ["packetization-mode=1;x-google-start-bitrate=900",
+                                                    "x-google-start-bitrate=900"])
+    }
+
+    /// A section with no direction attribute defaults to `sendrecv` (RFC 8866 §6.7), so it is
+    /// a sending section and must not be rewritten.
+    @Test func sectionWithoutDirectionCountsAsSending() {
+        let noDirection = Self.singlePCOffer.replacingOccurrences(
+            of: "a=rtpmap:100 VP8/90000\r\na=recvonly", with: "a=rtpmap:100 VP8/90000",
+        )
+        let sections = SDP(parsing: Transport.conformBundledCodecFmtp(noDirection)).mediaSections
+        let original = SDP(parsing: noDirection).mediaSections
+
+        #expect(sections[3].lines == original[3].lines)
+    }
+
+    @Test func isIdempotentAndPreservesUnrelatedSDP() {
+        let once = Transport.conformBundledCodecFmtp(Self.singlePCOffer)
+
+        #expect(Transport.conformBundledCodecFmtp(once) == once)
+        #expect(once.hasSuffix("\r\n"))
+        // Nothing to conform: no receive section, so byte-identical.
+        #expect(Transport.conformBundledCodecFmtp(once.replacingOccurrences(of: "a=recvonly", with: "a=sendrecv"))
+            == once.replacingOccurrences(of: "a=recvonly", with: "a=sendrecv"))
+    }
+
+    /// A payload a sending section declares with no fmtp at all is canonical in that state, so a
+    /// 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() {
+        let senderBare = Self.singlePCOffer
+            .replacingOccurrences(of: "a=fmtp:100 x-google-start-bitrate=900\r\n", with: "")
+        let sections = SDP(parsing: Transport.conformBundledCodecFmtp(senderBare)).mediaSections
+
+        // Sending section (mid 2) declares 100 with no fmtp; the receive section must match.
+        #expect(sections[2].fmtp(forPayload: "100") == nil)
+        #expect(sections[3].fmtp(forPayload: "100") == nil)
+        #expect(Self.fmtpConfigsByPayload(Transport.conformBundledCodecFmtp(senderBare))
+            .allSatisfy { $0.value.count == 1 })
+    }
+
+    /// The pipeline the publisher actually runs: the start-bitrate munge diverges the shared
+    /// payload types, the conform pass puts them back. Expressed as the invariant libwebrtc
+    /// enforces — one fmtp config per payload type across the BUNDLE group — rather than as
+    /// specific strings.
+    @Test func startBitrateFollowedByConformLeavesNoCollision() {
+        let unconformed = Transport.mungeVideoStartBitrate(Self.singlePCOffer, kbpsBySenderId: ["cam": 1000])
+        let conformed = Transport.conformBundledCodecFmtp(unconformed)
+
+        #expect(Self.fmtpConfigsByPayload(unconformed).contains { $0.value.count > 1 })
+        #expect(Self.fmtpConfigsByPayload(conformed).allSatisfy { $0.value.count == 1 })
+    }
+
+    /// Every fmtp config each payload type is given across the document, keyed by payload. A
+    /// payload declared by `a=rtpmap` but given no fmtp line contributes the empty config,
+    /// since "parameters" and "no parameters" collide just as two different parameter sets do.
+    private static func fmtpConfigsByPayload(_ sdp: String) -> [String: Set<String>] {
+        var result: [String: Set<String>] = [:]
+        for section in SDP(parsing: sdp).mediaSections {
+            for rtpmap in section.rtpmaps {
+                result[rtpmap.payload, default: []].insert(section.fmtp(forPayload: rtpmap.payload)?.config ?? "")
+            }
+        }
+        return result
+    }
+}