diff --git a/README.md b/README.md index c97b7f1..c8163db 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,7 @@ Please see the following resources for more information on MediaStreamConstraint - streamId (optional): the stream to send the tone on; defaults to all published streams - duration (optional): tone duration in milliseconds, between 40 and 6000 (default: 100) - interToneGap (optional): gap between tones in milliseconds, minimum 30 (default: 70) +- Returns: `true` if the tones were queued on at least one stream, `false` otherwise (e.g. no audio stream published yet, or the telephone-event codec has not been negotiated) ```javascript bandwidthRtc.sendDtmf("3"); diff --git a/src/bandwidthRtc.ts b/src/bandwidthRtc.ts index 2b1cdcd..18a6605 100644 --- a/src/bandwidthRtc.ts +++ b/src/bandwidthRtc.ts @@ -211,7 +211,15 @@ class BandwidthRtc { return devices; } - sendDtmf(tone: string, streamId?: string, duration: number = 100, interToneGap: number = 70) { + /** + * Send DTMF tones on published audio streams via the browser's native RTCDTMFSender (RFC 4733). + * @param tone The DTMF tones to send - a string composed of the characters [0-9,*,#,A-D,\,]* + * @param streamId The optional stream id to send on; defaults to all published streams. + * @param duration Tone duration in milliseconds (default: 100). Must be between 40 and 6000. + * @param interToneGap Gap between tones in milliseconds (default: 70). Minimum 30. + * @returns true if the tones were queued on at least one stream, false otherwise + */ + sendDtmf(tone: string, streamId?: string, duration: number = 100, interToneGap: number = 70): boolean { if (!this.delegate) { throw new BandwidthRtcError("You must call 'connect' before 'sendDtmf'"); } diff --git a/src/v1/bandwidthRtc.test.ts b/src/v1/bandwidthRtc.test.ts index 0e6f5d7..3fa9891 100644 --- a/src/v1/bandwidthRtc.test.ts +++ b/src/v1/bandwidthRtc.test.ts @@ -103,17 +103,24 @@ describe("bandwidthRtcV1 sendDtmf", () => { expect(sender.insertDTMF).toHaveBeenCalledWith("1", 200, 80); }); - test("does not throw when no senders are registered", () => { + test("returns true when tones are queued on at least one sender", () => { const brtc = new BandwidthRtc(); - expect(() => brtc.sendDtmf("5")).not.toThrow(); + (brtc as any).localDtmfSenders.set("stream-1", makeDtmfSender()); + + expect(brtc.sendDtmf("5")).toBe(true); + }); + + test("returns false without throwing when no senders are registered", () => { + const brtc = new BandwidthRtc(); + expect(brtc.sendDtmf("5")).toBe(false); }); - test("does nothing for an unknown streamId", () => { + test("returns false for an unknown streamId", () => { const brtc = new BandwidthRtc(); const sender = makeDtmfSender(); (brtc as any).localDtmfSenders.set("stream-1", sender); - brtc.sendDtmf("5", "nonexistent"); + expect(brtc.sendDtmf("5", "nonexistent")).toBe(false); expect(sender.insertDTMF).not.toHaveBeenCalled(); }); @@ -125,12 +132,19 @@ describe("bandwidthRtcV1 sendDtmf", () => { (brtc as any).localDtmfSenders.set("stream-1", notReady); (brtc as any).localDtmfSenders.set("stream-2", ready); - expect(() => brtc.sendDtmf("5")).not.toThrow(); + expect(brtc.sendDtmf("5")).toBe(true); expect(notReady.insertDTMF).not.toHaveBeenCalled(); expect(ready.insertDTMF).toHaveBeenCalledWith("5", 100, 70); }); + test("returns false when the only sender is not ready", () => { + const brtc = new BandwidthRtc(); + (brtc as any).localDtmfSenders.set("stream-1", makeDtmfSender(false)); + + expect(brtc.sendDtmf("5")).toBe(false); + }); + test("catches an insertDTMF error on one sender and still calls the others", () => { const brtc = new BandwidthRtc(); const throwing = makeDtmfSender(); @@ -141,7 +155,7 @@ describe("bandwidthRtcV1 sendDtmf", () => { (brtc as any).localDtmfSenders.set("stream-1", throwing); (brtc as any).localDtmfSenders.set("stream-2", healthy); - expect(() => brtc.sendDtmf("5")).not.toThrow(); + expect(brtc.sendDtmf("5")).toBe(true); expect(healthy.insertDTMF).toHaveBeenCalledWith("5", 100, 70); }); @@ -150,6 +164,7 @@ describe("bandwidthRtcV1 sendDtmf", () => { describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => { afterEach(() => { delete (global as any).RTCRtpSender; + delete (global as any).RTCRtpReceiver; }); function makeTransceiver(dtmf: RTCDTMFSender | null = { insertDTMF: jest.fn() } as any) { @@ -184,12 +199,27 @@ 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 telephone-event codec from receiver capabilities 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).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [telephoneEventCodec] }) }; + + const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; + (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] }); + + expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, telephoneEventCodec]); + }); + + test("falls back to sender capabilities for telephone-event when receiver capabilities lack it", () => { + const brtc = new BandwidthRtc(); + const transceiver = makeTransceiver(); + withPublishingPeerConnection(brtc, transceiver); + + const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 }; + (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [] }) }; (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [telephoneEventCodec] }) }; const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; @@ -217,6 +247,7 @@ describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => { withPublishingPeerConnection(brtc, transceiver); (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [] }) }; + (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [] }) }; const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] }); @@ -224,7 +255,7 @@ describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => { expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec]); }); - test("forces telephone-event into codec preferences even without explicit codecPreferences", () => { + test("does not call setCodecPreferences when no codecPreferences are provided", () => { const brtc = new BandwidthRtc(); const transceiver = makeTransceiver(); withPublishingPeerConnection(brtc, transceiver); @@ -232,20 +263,74 @@ describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => { 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] }) }; + (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [opusCodec, telephoneEventCodec] }) }; (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio")); - expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, telephoneEventCodec]); + expect(transceiver.setCodecPreferences).not.toHaveBeenCalled(); }); - test("skips setCodecPreferences when RTCRtpSender is unavailable (e.g. non-browser environment)", () => { + test("applies audio preferences as-is when capability APIs are unavailable (e.g. non-browser environment)", () => { const brtc = new BandwidthRtc(); const transceiver = makeTransceiver(); withPublishingPeerConnection(brtc, transceiver); - expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"))).not.toThrow(); + const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; + expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] })).not.toThrow(); - expect(transceiver.setCodecPreferences).not.toHaveBeenCalled(); + expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec]); + }); + + test("retries with the caller's preferences when the telephone-event-augmented list is rejected", () => { + const brtc = new BandwidthRtc(); + const transceiver = makeTransceiver(); + withPublishingPeerConnection(brtc, transceiver); + + const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 }; + (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [telephoneEventCodec] }) }; + + transceiver.setCodecPreferences.mockImplementationOnce(() => { + throw new DOMException("RTCRtpCodec sdpFmtLine badly formated", "InvalidModificationError"); + }); + + const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; + expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] })).not.toThrow(); + + expect(transceiver.setCodecPreferences).toHaveBeenNthCalledWith(1, [opusCodec, telephoneEventCodec]); + expect(transceiver.setCodecPreferences).toHaveBeenNthCalledWith(2, [opusCodec]); + }); + + test("falls back to browser default codecs without throwing when setCodecPreferences always rejects", () => { + const brtc = new BandwidthRtc(); + const transceiver = makeTransceiver(); + withPublishingPeerConnection(brtc, transceiver); + + const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 }; + (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [telephoneEventCodec] }) }; + + transceiver.setCodecPreferences.mockImplementation(() => { + throw new DOMException("codecs do not match capabilities", "InvalidModificationError"); + }); + + const opusCodec = { mimeType: "audio/opus", clockRate: 48000 }; + expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] })).not.toThrow(); + + expect(transceiver.setCodecPreferences).toHaveBeenCalledTimes(2); + }); + + test("does not throw when video codec preferences are rejected", () => { + const brtc = new BandwidthRtc(); + const transceiver = makeTransceiver(); + withPublishingPeerConnection(brtc, transceiver); + + transceiver.setCodecPreferences.mockImplementation(() => { + throw new DOMException("codecs do not match capabilities", "InvalidModificationError"); + }); + + const vp8Codec = { mimeType: "video/VP8", clockRate: 90000 }; + expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "video"), { video: [vp8Codec] })).not.toThrow(); + + expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([vp8Codec]); }); }); diff --git a/src/v1/bandwidthRtc.ts b/src/v1/bandwidthRtc.ts index b0cae0c..6f3528e 100644 --- a/src/v1/bandwidthRtc.ts +++ b/src/v1/bandwidthRtc.ts @@ -290,30 +290,42 @@ export class BandwidthRtc { * @param streamId The optional stream id to send on; defaults to all published streams. * @param duration Tone duration in milliseconds (default: 100). Must be between 40 and 6000. * @param interToneGap Gap between tones in milliseconds (default: 70). Minimum 30. + * @returns true if the tones were queued on at least one stream, false otherwise */ - sendDtmf(tone: string, streamId?: string, duration: number = 100, interToneGap: number = 70) { - const insert = (dtmfSender: RTCDTMFSender, id: string) => { + sendDtmf(tone: string, streamId?: string, duration: number = 100, interToneGap: number = 70): boolean { + const insert = (dtmfSender: RTCDTMFSender, id: string): boolean => { if (!dtmfSender.canInsertDTMF) { logger.warn(`sendDtmf: DTMF sender for stream ${id} is not ready (canInsertDTMF is false); skipping`); - return; + return false; } try { dtmfSender.insertDTMF(tone, duration, interToneGap); + return true; } catch (err) { logger.warn(`sendDtmf: insertDTMF failed for stream ${id}`, err); + return false; } }; if (streamId) { const dtmfSender = this.localDtmfSenders.get(streamId); if (dtmfSender) { - insert(dtmfSender, streamId); - } else { - logger.warn(`sendDtmf: no DTMF sender registered for stream ${streamId}`); + return insert(dtmfSender, streamId); } - } else { - this.localDtmfSenders.forEach(insert); + logger.warn(`sendDtmf: no DTMF sender registered for stream ${streamId}`); + return false; + } + + if (this.localDtmfSenders.size === 0) { + logger.warn("sendDtmf: no DTMF senders registered; has an audio stream been published?"); + return false; } + + let sent = false; + this.localDtmfSenders.forEach((dtmfSender, id) => { + sent = insert(dtmfSender, id) || sent; + }); + return sent; } /** @@ -698,28 +710,79 @@ export class BandwidthRtc { this.localDtmfSenders.set(mediaStream.id, dtmfSender); } - if (track.kind === TRACK_KIND_AUDIO) { - // setCodecPreferences is a strict allowlist: any codec omitted from the list is - // 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. - 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); - } - } + // Only restrict codecs when the caller explicitly asks for it. Every browser + // that supports RTCDTMFSender already includes telephone-event in its default + // audio offer, and setCodecPreferences behaves very differently across WebKit + // versions (throwing on some, silently dropping codecs on others), so touching + // it in the default path only adds risk. + if (track.kind === TRACK_KIND_AUDIO && codecPreferences?.audio) { + this.applyAudioCodecPreferences(transceiver, codecPreferences.audio); } else if (track.kind === TRACK_KIND_VIDEO && codecPreferences?.video) { - transceiver.setCodecPreferences(codecPreferences.video); + this.trySetCodecPreferences(transceiver, codecPreferences.video, TRACK_KIND_VIDEO); } }); } + /** + * Apply caller-provided audio codec preferences, keeping telephone-event in the + * list so RTCDTMFSender can negotiate RFC 4733 DTMF. setCodecPreferences is a + * strict allowlist: any codec omitted from the list is dropped from the SDP offer. + */ + private applyAudioCodecPreferences(transceiver: RTCRtpTransceiver, audioCodecs: RTCRtpCodec[]) { + const hasTelephoneEvent = audioCodecs.some((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE); + if (!hasTelephoneEvent) { + const telephoneEventCodec = this.findTelephoneEventCodec(); + if (telephoneEventCodec) { + if (this.trySetCodecPreferences(transceiver, [...audioCodecs, telephoneEventCodec], TRACK_KIND_AUDIO)) { + return; + } + } else { + logger.warn( + "telephone-event codec not found in this browser's capabilities; applying audio codec preferences as-is, DTMF may not be able to negotiate", + ); + } + } + this.trySetCodecPreferences(transceiver, audioCodecs, TRACK_KIND_AUDIO); + } + + /** + * Per the WebRTC spec, codecs passed to setCodecPreferences must come from the + * receiver's capabilities; older engines matched against the sender's, so check both. + */ + private findTelephoneEventCodec(): RTCRtpCodec | undefined { + const capabilityCodecs = [ + typeof RTCRtpReceiver !== "undefined" && typeof RTCRtpReceiver.getCapabilities === "function" + ? RTCRtpReceiver.getCapabilities(TRACK_KIND_AUDIO)?.codecs + : undefined, + typeof RTCRtpSender !== "undefined" && typeof RTCRtpSender.getCapabilities === "function" + ? RTCRtpSender.getCapabilities(TRACK_KIND_AUDIO)?.codecs + : undefined, + ]; + for (const codecs of capabilityCodecs) { + const telephoneEventCodec = codecs?.find((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE); + if (telephoneEventCodec) { + return telephoneEventCodec; + } + } + return undefined; + } + + /** + * setCodecPreferences must never break publishing: WebKit's matching rules vary + * by version (case sensitivity, receiver-only capability matching, strict + * sdpFmtpLine parsing) and a failure here just means the browser's default codec + * offer is used instead. + */ + private trySetCodecPreferences(transceiver: RTCRtpTransceiver, codecs: RTCRtpCodec[], kind: string): boolean { + try { + transceiver.setCodecPreferences(codecs); + return true; + } catch (err) { + logger.warn(`setCodecPreferences failed for ${kind} track; falling back to browser default codecs`, err); + return false; + } + } + private cleanupPublishedStreams(...streams: PublishedStream[]) { logger.debug(`cleanupPublishedStreams: ${streams}`); if (streams.length === 0) {