From 006dd3b8889be7ad73514c187e6971c2522e531d Mon Sep 17 00:00:00 2001 From: smoghe-bw Date: Mon, 13 Jul 2026 10:30:18 -0400 Subject: [PATCH] Match telephone-event clock rate to the primary audio codec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DTMF from a WebRTC endpoint was silently lost (audio unaffected) when the publish offer paired telephone-event/8000 with Opus/48000. RFC 4733 requires telephone-event to share the audio codec's RTP clock, so a peer that only registers telephone-event at the audio clock (e.g. pv-gateway at 48000 for Opus) fails to negotiate it, and endpoint DTMF is then dropped/mislabeled before it reaches the media server. The prior logic appended getCapabilities().find(telephone-event) — the first match, often the 8000 variant — whenever telephone-event was absent from the codec preferences (which is exactly the case when Opus is forced). Now strip all telephone-event codecs and re-add only the one whose clockRate matches the primary (first) media codec: Opus -> 48000, PCMU -> 8000, without hardcoding either rate. Prefer a match already in the caller's preferences, fall back to full capabilities, and omit telephone-event entirely rather than offer a mismatched one the peer will drop. Tests rewritten to assert clock-matching (they previously codified the buggy 8000 pairing). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/v1/bandwidthRtc.test.ts | 51 +++++++++++++++++++++++++++---------- src/v1/bandwidthRtc.ts | 23 ++++++++++++----- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/v1/bandwidthRtc.test.ts b/src/v1/bandwidthRtc.test.ts index 0e6f5d7..fb860fc 100644 --- a/src/v1/bandwidthRtc.test.ts +++ b/src/v1/bandwidthRtc.test.ts @@ -184,39 +184,60 @@ describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => { expect((brtc as any).localDtmfSenders.has("stream-1")).toBe(false); }); - test("appends telephone-event codec when missing from audio preferences", () => { + test("appends the clock-matched telephone-event when missing from audio preferences", () => { const brtc = new BandwidthRtc(); const transceiver = makeTransceiver(); withPublishingPeerConnection(brtc, transceiver); - const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 }; - (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [telephoneEventCodec] }) }; + const te8000 = { mimeType: "audio/telephone-event", clockRate: 8000 }; + const te48000 = { mimeType: "audio/telephone-event", clockRate: 48000 }; + (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [te8000, te48000] }) }; const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] }); - expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, telephoneEventCodec]); + // Must pair with telephone-event/48000 (Opus clock), not the 8000 variant. + expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, te48000]); }); - test("does not duplicate telephone-event when already in preferences", () => { + test("replaces a clock-mismatched telephone-event already in preferences", () => { const brtc = new BandwidthRtc(); const transceiver = makeTransceiver(); withPublishingPeerConnection(brtc, transceiver); const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; - const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 }; - (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec, telephoneEventCodec] }); + const te8000 = { mimeType: "audio/telephone-event", clockRate: 8000 }; + const te48000 = { mimeType: "audio/telephone-event", clockRate: 48000 }; + (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [te48000] }) }; - expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, telephoneEventCodec]); + (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec, te8000] }); + + // The mismatched 8000 event is dropped and replaced with the 48000 one from capabilities. + expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, te48000]); + expect(transceiver.setCodecPreferences).toHaveBeenCalledTimes(1); + }); + + test("keeps the clock-matched telephone-event already in preferences", () => { + const brtc = new BandwidthRtc(); + const transceiver = makeTransceiver(); + withPublishingPeerConnection(brtc, transceiver); + + const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; + const te48000 = { mimeType: "audio/telephone-event", clockRate: 48000 }; + (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec, te48000] }); + + expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, te48000]); expect(transceiver.setCodecPreferences).toHaveBeenCalledTimes(1); }); - test("falls back to original preferences when telephone-event not found in capabilities", () => { + test("falls back to original preferences when no clock-matched telephone-event is available", () => { const brtc = new BandwidthRtc(); const transceiver = makeTransceiver(); withPublishingPeerConnection(brtc, transceiver); - (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [] }) }; + // Only a mismatched (8000) telephone-event exists; better to omit it than offer a dropped codec. + const te8000 = { mimeType: "audio/telephone-event", clockRate: 8000 }; + (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [te8000] }) }; const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] }); @@ -224,18 +245,20 @@ describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => { expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec]); }); - test("forces telephone-event into codec preferences even without explicit codecPreferences", () => { + test("forces the clock-matched telephone-event even without explicit codecPreferences", () => { const brtc = new BandwidthRtc(); const transceiver = makeTransceiver(); withPublishingPeerConnection(brtc, transceiver); const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; - const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 }; - (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [opusCodec, telephoneEventCodec] }) }; + const te8000 = { mimeType: "audio/telephone-event", clockRate: 8000 }; + const te48000 = { mimeType: "audio/telephone-event", clockRate: 48000 }; + (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [opusCodec, te8000, te48000] }) }; (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio")); - expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, telephoneEventCodec]); + // Full capabilities collapse to the primary codec + its matching-clock telephone-event. + expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, te48000]); }); test("skips setCodecPreferences when RTCRtpSender is unavailable (e.g. non-browser environment)", () => { diff --git a/src/v1/bandwidthRtc.ts b/src/v1/bandwidthRtc.ts index b0cae0c..1c4352e 100644 --- a/src/v1/bandwidthRtc.ts +++ b/src/v1/bandwidthRtc.ts @@ -703,16 +703,25 @@ export class BandwidthRtc { // dropped from the SDP offer. Apply it unconditionally (not just when the caller // passes codecPreferences) so telephone-event is always present and RTCDTMFSender // can send RFC 4733 DTMF packets, regardless of the browser's default codec offer. + // + // RFC 4733: telephone-event shares the primary audio codec's RTP clock, so it MUST + // be offered at that codec's clock rate (e.g. 48000 alongside Opus). A mismatched + // telephone-event (e.g. 8000 next to Opus/48000) is silently dropped by the peer and + // DTMF never reaches the far end — so drop every telephone-event and re-add only the + // one whose clock rate matches, sourced from prefs or full capabilities. const audioCapabilities = typeof RTCRtpSender !== "undefined" ? RTCRtpSender.getCapabilities(TRACK_KIND_AUDIO) : undefined; const audioCodecs = codecPreferences?.audio ?? audioCapabilities?.codecs; if (audioCodecs) { - const hasTelephoneEvent = audioCodecs.some((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE); - if (!hasTelephoneEvent) { - const telephoneEventCodec = audioCapabilities?.codecs.find((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE); - transceiver.setCodecPreferences(telephoneEventCodec ? [...audioCodecs, telephoneEventCodec] : audioCodecs); - } else { - transceiver.setCodecPreferences(audioCodecs); - } + // Strip every telephone-event, then re-add only the one whose clock rate matches the + // primary (first) media codec, preferring one already in the caller's list and falling + // back to full capabilities. If none matches, leave the codecs untouched rather than + // offer a telephone-event the peer will drop. + const isTelephoneEvent = (c: RTCRtpCodec) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE; + const mediaCodecs = audioCodecs.filter((c) => !isTelephoneEvent(c)); + const primaryClockRate = mediaCodecs[0]?.clockRate; + const matchesPrimary = (c: RTCRtpCodec) => isTelephoneEvent(c) && c.clockRate === primaryClockRate; + const telephoneEventCodec = audioCodecs.find(matchesPrimary) ?? audioCapabilities?.codecs.find(matchesPrimary); + transceiver.setCodecPreferences(telephoneEventCodec ? [...mediaCodecs, telephoneEventCodec] : audioCodecs); } } else if (track.kind === TRACK_KIND_VIDEO && codecPreferences?.video) { transceiver.setCodecPreferences(codecPreferences.video);