fix: ensure extension id isn't reused across bundle

src/room/PCTransport.test.ts

@@ -8,7 +8,6 @@ import {
   extractStereoAndNackAudioFromOffer,
   fmtpConfigHasParam,
   placeholderMidsFromTransceivers,
-  videoSectionCanReceiveAV1,
 } from './PCTransport';
 import { ddExtensionURI } from './utils';
 
@@ -316,22 +315,6 @@ const sectionOf = (media: MediaDescription[], mid: string) =>
 const ddOf = (media: MediaDescription[], mid: string) =>
   sectionOf(media, mid).ext?.find((ext) => ext.uri === ddExtensionURI)?.value;
 
-describe('videoSectionCanReceiveAV1', () => {
-  const { media } = parse(SINGLE_PC_OFFER);
-
-  it('is true for a section we receive on that kept AV1', () => {
-    expect(videoSectionCanReceiveAV1(sectionOf(media, '1'))).toBe(true);
-  });
-
-  it('is false for a send-only section, where the SVC path owns the extension', () => {
-    expect(videoSectionCanReceiveAV1(sectionOf(media, '0'))).toBe(false);
-  });
-
-  it('is false when AV1 did not survive negotiation', () => {
-    expect(videoSectionCanReceiveAV1(sectionOf(media, '2'))).toBe(false);
-  });
-});
-
 describe('ensureVideoDDExtension', () => {
   it('assigns an id above every extension in the bundle', () => {
     const sdp = parse(SINGLE_PC_OFFER);
@@ -376,6 +359,29 @@ a=extmap:3 ${ddExtensionURI}`);
     expect(ensureVideoDDExtension(sectionOf(sdp.media, '1'), sdp, 0)).toBe(16);
   });
 
+  it('abandons the cached id once something else stands for it', () => {
+    // The id was free when it was picked, then the first section we send on brought the fuller
+    // extension set along and claimed it. Reusing it anyway is what makes the browser reject
+    // the offer with "a BUNDLE group contains a codec collision for header extension id".
+    const sdp = parse(SINGLE_PC_OFFER);
+    sectionOf(sdp.media, '0').ext!.push({ value: 12, uri: 'urn:3gpp:video-orientation' });
+
+    expect(ensureVideoDDExtension(sectionOf(sdp.media, '1'), sdp, 12)).toBe(13);
+    expect(ddOf(sdp.media, '1')).toBe(13);
+  });
+
+  it('leaves the offer alone when the mapped id is contested', () => {
+    // Nothing consistent for the whole bundle is available: the extension is mapped to 12 on one
+    // section and 12 means something else on another, and the browser's half of the map is not
+    // ours to renumber. Losing AV1 beats an offer that cannot be applied at all.
+    const sdp = parse(SINGLE_PC_OFFER);
+    sectionOf(sdp.media, '0').ext!.push({ value: 12, uri: 'urn:3gpp:video-orientation' });
+    sectionOf(sdp.media, '2').ext!.push({ value: 12, uri: ddExtensionURI });
+
+    expect(ensureVideoDDExtension(sectionOf(sdp.media, '1'), sdp, 0)).toBe(0);
+    expect(ddOf(sdp.media, '1')).toBeUndefined();
+  });
+
   it('adds the extension to a section that has none', () => {
     const sdp = parse(SINGLE_PC_OFFER);
     const section = sectionOf(sdp.media, '1');

src/room/PCTransport.ts

@@ -7,7 +7,7 @@ import log, { LoggerNames, getLogger } from '../logger';
 import { debounce } from './debounce';
 import { NegotiationError, UnexpectedConnectionState } from './errors';
 import type { LoggerOptions } from './types';
-import { ddExtensionURI, isChromiumBased, isSVCCodec, isSafari } from './utils';
+import { ddExtensionURI, isSVCCodec, isSafari } from './utils';
 
 /** @internal */
 interface TrackBitrateInfo {
@@ -458,12 +458,6 @@ export default class PCTransport extends (EventEmitter as new () => TypedEmitter
         if (media.type === 'audio') {
           ensureAudioNackAndStereo(media, ['all'], []);
         } else if (media.type === 'video') {
-          // Chrome 152 stopped decoding AV1 that arrives without DD
-          // (frames get assembled, none ever decode), which breaks
-          // subscribing to AV1 wherever we own the offer, i.e. on a single peer connection.
-          if (isChromiumBased() && videoSectionCanReceiveAV1(media)) {
-            this.ddExtID = ensureVideoDDExtension(media, sdpParsed, this.ddExtID);
-          }
           this.trackBitrates.some((trackbr): boolean => {
             if (!trackbr.cid) {
               return false;
@@ -741,7 +735,10 @@ export function ensureVideoDDExtension(
   sdp: SessionDescription,
   ddExtID: number,
 ): number {
-  const id = existingDDExtensionID(sdp) ?? (ddExtID === 0 ? unusedExtensionID(sdp) : ddExtID);
+  const id = ddExtensionIDFor(sdp, ddExtID);
+  if (id === undefined) {
+    return ddExtID;
+  }
 
   if (!media.ext?.some((ext) => ext.uri === ddExtensionURI)) {
     media.ext ??= [];
@@ -753,21 +750,47 @@ export function ensureVideoDDExtension(
   return id;
 }
 
-/** The id the dependency descriptor extension is already mapped to in `sdp`, if any. */
-function existingDDExtensionID(sdp: SessionDescription): number | undefined {
+/**
+ * The id to map the dependency descriptor to throughout `sdp`, or undefined when no id would be
+ * consistent for the whole bundle and the extension therefore has to be left out.
+ */
+function ddExtensionIDFor(sdp: SessionDescription, cachedID: number): number | undefined {
+  const mapped = mappedExtensionID(sdp, ddExtensionURI);
+  if (mapped !== undefined) {
+    // Adopting an id that also stands for another URI is what the browser rejects the bundle
+    // over, and its own half of the map is not ours to renumber, so give up on this offer.
+    return usedForOtherURI(sdp, mapped, ddExtensionURI) ? undefined : mapped;
+  }
+  // Reusing the id from the last offer keeps the mapping stable across renegotiations, but only
+  // while nothing else has taken it: the browser assigns ids to its own extensions without
+  // knowing about ours, so an id that was free when we picked it can since have been claimed —
+  // typically by the fuller extension set that arrives with the first section we send on.
+  if (cachedID !== 0 && !usedForOtherURI(sdp, cachedID, ddExtensionURI)) {
+    return cachedID;
+  }
+  return unusedExtensionID(sdp);
+}
+
+/** The id `uri` is mapped to in `sdp`, if any section maps it. */
+function mappedExtensionID(sdp: SessionDescription, uri: string): number | undefined {
   for (const media of sdp.media) {
-    const ext = media.ext?.find((candidate) => candidate.uri === ddExtensionURI);
+    const ext = media.ext?.find((candidate) => candidate.uri === uri);
     if (ext) {
       return ext.value;
     }
   }
   return undefined;
 }
 
+/** Whether `id` stands for anything in `sdp` other than `uri`. */
+function usedForOtherURI(sdp: SessionDescription, id: number, uri: string): boolean {
+  return sdp.media.some((media) => media.ext?.some((ext) => ext.value === id && ext.uri !== uri));
+}
+
 /**
  * An id no extension in `sdp` uses. Stays above every id in use rather than filling gaps, so it
- * cannot collide with an id the browser allocates to another extension later, and steps over 15,
- * which RFC 8285 reserves.
+ * is less likely to be an id the browser goes on to allocate to another extension, and steps
+ * over 15, which RFC 8285 reserves.
  */
 function unusedExtensionID(sdp: SessionDescription): number {
   let maxID = 0;
@@ -781,26 +804,6 @@ function unusedExtensionID(sdp: SessionDescription): number {
   return maxID + 1 === 15 ? 16 : maxID + 1;
 }
 
-/**
- * Whether AV1 could arrive on this section, i.e. it is one we receive on and AV1 survived
- * negotiation. Which codec the server actually sends is decided after negotiation, so any
- * receiving section offering AV1 has to be prepared for it.
- * @internal
- */
-export function videoSectionCanReceiveAV1(
-  media: {
-    type: string;
-    port: number;
-    protocol: string;
-    payloads?: string | undefined;
-  } & MediaDescription,
-): boolean {
-  if (media.direction !== 'recvonly' && media.direction !== 'sendrecv') {
-    return false;
-  }
-  return media.rtp.some((rtp) => rtp.codec.toLowerCase() === 'av1');
-}
-
 /**
  * Checks whether an fmtp config declares `param` as an exact, `;`-delimited
  * token. A plain substring check conflates distinct opus parameters — e.g.

src/room/RTCEngine.ts

@@ -101,6 +101,7 @@ import {
   isVideoCodec,
   isVideoTrack,
   isWeb,
+  negotiateDependencyDescriptor,
   sleep,
   supportsAddTrack,
   supportsTransceiver,
@@ -844,8 +845,15 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
     for (let i: number = 0; i < numAudios; i++) {
       this.pcManager?.addPublisherTransceiverOfKind('audio', transceiverInit);
     }
+    // media only arrives on these sections when there is no subscriber connection to arrive on
+    const receivesMedia = this.pcManager?.mode === 'publisher-only';
     for (let i: number = 0; i < numVideos; i++) {
-      this.pcManager?.addPublisherTransceiverOfKind('video', transceiverInit);
+      const transceiver = this.pcManager?.addPublisherTransceiverOfKind('video', transceiverInit);
+      if (receivesMedia && transceiver) {
+        this.log.debug('dependency descriptor negotiated for received video', {
+          negotiated: negotiateDependencyDescriptor(transceiver),
+        });
+      }
     }
   }
 

src/room/utils.test.ts

@@ -1,6 +1,13 @@
 import { ClientInfo_Capability } from '@livekit/protocol';
-import { describe, expect, it } from 'vitest';
-import { extractMaxAgeFromRequestHeaders, getClientInfo, splitUtf8, toWebsocketUrl } from './utils';
+import { describe, expect, it, vi } from 'vitest';
+import {
+  ddExtensionURI,
+  extractMaxAgeFromRequestHeaders,
+  getClientInfo,
+  negotiateDependencyDescriptor,
+  splitUtf8,
+  toWebsocketUrl,
+} from './utils';
 
 describe('toWebsocketUrl', () => {
   it('leaves wss urls alone', () => {
@@ -188,3 +195,65 @@ describe('extractMaxAgeFromRequestHeaders', () => {
     expect(extractMaxAgeFromRequestHeaders(headers)).toBe(3600);
   });
 });
+
+describe('negotiateDependencyDescriptor', () => {
+  /** A transceiver whose header extension control offers `extensions`, or none at all. */
+  const transceiverWith = (extensions?: RTCRtpHeaderExtensionCapability[], throws = false) => {
+    const set = vi.fn((updated: RTCRtpHeaderExtensionCapability[]) => {
+      if (throws) {
+        throw new Error('InvalidModificationError');
+      }
+      extensions = updated;
+    });
+    return {
+      transceiver: (extensions
+        ? {
+            getHeaderExtensionsToNegotiate: () => extensions!,
+            setHeaderExtensionsToNegotiate: set,
+          }
+        : {}) as unknown as RTCRtpTransceiver,
+      set,
+      current: () => extensions,
+    };
+  };
+
+  it('turns the extension on for a transceiver that has it stopped', () => {
+    const { transceiver, set, current } = transceiverWith([
+      { uri: 'urn:ietf:params:rtp-hdrext:sdes:mid', direction: 'sendrecv' },
+      { uri: ddExtensionURI, direction: 'stopped' },
+    ]);
+
+    expect(negotiateDependencyDescriptor(transceiver)).toBe(true);
+    expect(set).toHaveBeenCalledOnce();
+    // sendrecv keeps it a plain a=extmap line, with no direction suffix for the server to parse
+    expect(current()?.find((ext) => ext.uri === ddExtensionURI)?.direction).toBe('sendrecv');
+  });
+
+  it('leaves an extension the browser already negotiates alone', () => {
+    const { transceiver, set } = transceiverWith([{ uri: ddExtensionURI, direction: 'recvonly' }]);
+
+    expect(negotiateDependencyDescriptor(transceiver)).toBe(true);
+    expect(set).not.toHaveBeenCalled();
+  });
+
+  it('reports no negotiation where the browser does not know the extension', () => {
+    const { transceiver, set } = transceiverWith([
+      { uri: 'urn:ietf:params:rtp-hdrext:sdes:mid', direction: 'sendrecv' },
+    ]);
+
+    expect(negotiateDependencyDescriptor(transceiver)).toBe(false);
+    expect(set).not.toHaveBeenCalled();
+  });
+
+  it('reports no negotiation where the browser has no such control', () => {
+    const { transceiver } = transceiverWith();
+
+    expect(negotiateDependencyDescriptor(transceiver)).toBe(false);
+  });
+
+  it('swallows a rejected direction rather than failing the connection', () => {
+    const { transceiver } = transceiverWith([{ uri: ddExtensionURI, direction: 'stopped' }], true);
+
+    expect(negotiateDependencyDescriptor(transceiver)).toBe(false);
+  });
+});

src/room/utils.ts

@@ -146,6 +146,45 @@ export function isSVCCodec(codec?: string): boolean {
   return codec === 'av1' || codec === 'vp9';
 }
 
+/**
+ * Opts `transceiver` into negotiating the AV1 dependency descriptor, reporting whether it will be.
+ *
+ * Chrome only offers the extension on transceivers that can send, so one we create to receive on
+ * never negotiates it — and Chrome 152 stopped decoding AV1 that arrives without it: frames get
+ * assembled, none ever decode, and the receiver asks for a keyframe forever. Asking through the
+ * transceiver rather than munging the extension into the SDP leaves the browser owning the
+ * extension id, which is what keeps that id consistent across the bundle and across
+ * renegotiations.
+ *
+ * A no-op where the browser offers no such control, or does not know the extension at all.
+ * @internal
+ */
+export function negotiateDependencyDescriptor(transceiver: RTCRtpTransceiver): boolean {
+  const extensions = transceiver.getHeaderExtensionsToNegotiate?.();
+  if (!extensions || !transceiver.setHeaderExtensionsToNegotiate) {
+    return false;
+  }
+  const dd = extensions.find((ext) => ext.uri === ddExtensionURI);
+  if (!dd) {
+    return false;
+  }
+  if (dd.direction !== 'stopped') {
+    return true;
+  }
+  // sendrecv rather than recvonly, so that the extension is written as a plain `a=extmap` line —
+  // the form the server already emits where it is the one offering — rather than one carrying a
+  // `/recvonly` suffix that its parser may not expect
+  dd.direction = 'sendrecv';
+  try {
+    transceiver.setHeaderExtensionsToNegotiate(extensions);
+    return true;
+  } catch (e) {
+    // a rejected direction throws. Negotiating without the extension is what happened before this
+    // existed, so it is not worth failing the connection over
+    return false;
+  }
+}
+
 export function supportsSetSinkId(elm?: HTMLMediaElement): boolean {
   if (!document || isSafariBased()) {
     return false;

src/type-polyfills/header-extensions.d.ts

@@ -0,0 +1,13 @@
+/**
+ * Per-transceiver control over which RTP header extensions get negotiated. Declared as optional
+ * because it is not in lib.dom yet and not every browser implements it.
+ * https://w3c.github.io/webrtc-extensions/#rtcrtptransceiver-interface-extensions
+ */
+interface RTCRtpTransceiver {
+  getHeaderExtensionsToNegotiate?(): RTCRtpHeaderExtensionCapability[];
+  setHeaderExtensionsToNegotiate?(extensions: RTCRtpHeaderExtensionCapability[]): void;
+}
+
+interface RTCRtpHeaderExtensionCapability {
+  direction?: 'sendrecv' | 'sendonly' | 'recvonly' | 'stopped';
+}