From 3cb6feb75b63233607d460117b91ea89add3e225 Mon Sep 17 00:00:00 2001 From: Bryce Tham Date: Mon, 29 Jul 2024 16:16:06 -0400 Subject: [PATCH 1/3] feat: update retain helpers --- src/munge.spec.ts | 114 +++++++++++++++++++++++++++++++++++++++++----- src/munge.ts | 83 +++++++++++++++++++++++++-------- 2 files changed, 166 insertions(+), 31 deletions(-) diff --git a/src/munge.spec.ts b/src/munge.spec.ts index e349eac..3b7851f 100644 --- a/src/munge.spec.ts +++ b/src/munge.spec.ts @@ -1,12 +1,15 @@ import * as fs from 'fs'; +import { CandidateLine } from 'lines'; import { AvMediaDescription, CodecInfo, Sdp } from './model'; import { + disableRemb, + disableRtcpFbValue, + disableTwcc, removeCodec, retainCandidates, + retainCandidatesByTransportType, retainCodecs, - disableRtcpFbValue, - disableRemb, - disableTwcc, + retainCodecsByCodecName, } from './munge'; import { parse } from './parser'; @@ -86,24 +89,67 @@ describe('munging', () => { expect(validateOfferCodecs(parsed)).toBe(true); }); }); + describe('retainCodecs', () => { it('should retain codecs correctly when passing in an SDP', () => { expect.hasAssertions(); const offer = fs.readFileSync('./src/sdp-corpus/offer_with_extra_codecs.sdp', 'utf-8'); const parsed = parse(offer); - retainCodecs(parsed, ['h264', 'opus']); + // eslint-disable-next-line jsdoc/require-jsdoc + const predicate = (codecInfo: CodecInfo) => + codecInfo.name === 'h264' || codecInfo.name === 'opus'; + + // should return true when some codecs have been filtered out + expect(retainCodecs(parsed, predicate)).toBeTruthy(); expect(validateOfferCodecs(parsed)).toBe(true); + // should return false when no codecs have been filtered out + expect(retainCodecs(parsed, predicate)).toBeFalsy(); }); it('should retain codecs correctly when passing in an AvMediaDescription', () => { expect.hasAssertions(); const offer = fs.readFileSync('./src/sdp-corpus/offer_with_extra_codecs.sdp', 'utf-8'); const parsed = parse(offer); + // eslint-disable-next-line jsdoc/require-jsdoc + const predicate = (codecInfo: CodecInfo) => + codecInfo.name === 'h264' || codecInfo.name === 'opus'; + + // should return true when some codecs have been filtered out parsed.avMedia.forEach((av) => { - retainCodecs(av, ['h264', 'opus']); + expect(retainCodecs(av, predicate)).toBeTruthy(); }); expect(validateOfferCodecs(parsed)).toBe(true); + // should return false when no codecs have been filtered out + parsed.avMedia.forEach((av) => { + expect(retainCodecs(av, predicate)).toBeFalsy(); + }); + }); + it('should retain codecs by name when passing in an SDP', () => { + expect.hasAssertions(); + const offer = fs.readFileSync('./src/sdp-corpus/offer_with_extra_codecs.sdp', 'utf-8'); + const parsed = parse(offer); + + // should return true when some codecs have been filtered out + expect(retainCodecsByCodecName(parsed, ['h264', 'opus'])).toBeTruthy(); + expect(validateOfferCodecs(parsed)).toBe(true); + // should return false when no codecs have been filtered out + expect(retainCodecsByCodecName(parsed, ['h264', 'opus'])).toBeFalsy(); + }); + it('should retain codecs by name when passing in an AvMediaDescription', () => { + expect.hasAssertions(); + const offer = fs.readFileSync('./src/sdp-corpus/offer_with_extra_codecs.sdp', 'utf-8'); + const parsed = parse(offer); + + // should return true when some codecs have been filtered out + parsed.avMedia.forEach((av) => { + expect(retainCodecsByCodecName(av, ['h264', 'opus'])).toBeTruthy(); + }); + expect(validateOfferCodecs(parsed)).toBe(true); + // should return false when no codecs have been filtered out + parsed.avMedia.forEach((av) => { + expect(retainCodecsByCodecName(av, ['h264', 'opus'])).toBeFalsy(); + }); }); }); @@ -113,35 +159,79 @@ describe('munging', () => { const offer = fs.readFileSync('./src/sdp-corpus/answer_with_extra_candidates.sdp', 'utf-8'); const parsed = parse(offer); + // eslint-disable-next-line jsdoc/require-jsdoc + const predicate = (candidate: CandidateLine) => + candidate.transport === 'UDP' || candidate.transport === 'TCP'; + + // should return true when some candidates have been filtered out + expect(retainCandidates(parsed, predicate)).toBeTruthy(); + parsed.media.forEach((mline) => { + expect(mline.iceInfo.candidates).toHaveLength(4); + expect( + mline.iceInfo.candidates.every((candidate) => + ['UDP', 'TCP'].includes(candidate.transport) + ) + ).toBeTruthy(); + }); + // should return false when no candidates have been filtered out + expect(retainCandidates(parsed, predicate)).toBeFalsy(); + }); + it('should retain candidates correctly when passing in a MediaDescription', () => { + expect.hasAssertions(); + const offer = fs.readFileSync('./src/sdp-corpus/answer_with_extra_candidates.sdp', 'utf-8'); + const parsed = parse(offer); + + // eslint-disable-next-line jsdoc/require-jsdoc + const predicate = (candidate: CandidateLine) => + candidate.transport === 'UDP' || candidate.transport === 'TCP'; + + parsed.media.forEach((media) => { + // should return true when some candidates have been filtered out + expect(retainCandidates(media, predicate)).toBeTruthy(); + expect(media.iceInfo.candidates).toHaveLength(4); + expect( + media.iceInfo.candidates.every((candidate) => + ['UDP', 'TCP'].includes(candidate.transport) + ) + ).toBeTruthy(); + // should return false when no candidates have been filtered out + expect(retainCandidates(media, predicate)).toBeFalsy(); + }); + }); + it('should retain candidates by transport type when passing in an SDP', () => { + expect.hasAssertions(); + const offer = fs.readFileSync('./src/sdp-corpus/answer_with_extra_candidates.sdp', 'utf-8'); + const parsed = parse(offer); + // should return true when some candidates have been filtered out - expect(retainCandidates(parsed, ['udp', 'tcp'])).toBeTruthy(); + expect(retainCandidatesByTransportType(parsed, ['UDP', 'TCP'])).toBeTruthy(); parsed.media.forEach((mline) => { expect(mline.iceInfo.candidates).toHaveLength(4); expect( mline.iceInfo.candidates.every((candidate) => - ['udp', 'tcp'].includes(candidate.transport.toLowerCase()) + ['UDP', 'TCP'].includes(candidate.transport) ) ).toBeTruthy(); }); // should return false when no candidates have been filtered out - expect(retainCandidates(parsed, ['udp', 'tcp'])).toBeFalsy(); + expect(retainCandidatesByTransportType(parsed, ['UDP', 'TCP'])).toBeFalsy(); }); - it('should retain candidates correctly when passing in an AvMediaDescription', () => { + it('should retain candidates by transport type when passing in a MediaDescription', () => { expect.hasAssertions(); const offer = fs.readFileSync('./src/sdp-corpus/answer_with_extra_candidates.sdp', 'utf-8'); const parsed = parse(offer); parsed.media.forEach((media) => { // should return true when some candidates have been filtered out - expect(retainCandidates(media, ['udp', 'tcp'])).toBeTruthy(); + expect(retainCandidatesByTransportType(media, ['UDP', 'TCP'])).toBeTruthy(); expect(media.iceInfo.candidates).toHaveLength(4); expect( media.iceInfo.candidates.every((candidate) => - ['udp', 'tcp'].includes(candidate.transport.toLowerCase()) + ['UDP', 'TCP'].includes(candidate.transport) ) ).toBeTruthy(); // should return false when no candidates have been filtered out - expect(retainCandidates(media, ['udp', 'tcp'])).toBeFalsy(); + expect(retainCandidatesByTransportType(media, ['UDP', 'TCP'])).toBeFalsy(); }); }); }); diff --git a/src/munge.ts b/src/munge.ts index a035cbe..109197d 100644 --- a/src/munge.ts +++ b/src/munge.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { CandidateLine } from './lines'; import { AvMediaDescription, CodecInfo, MediaDescription, Sdp } from './model'; /** @@ -69,42 +70,67 @@ export function removeCodec(sdpOrAv: Sdp | AvMediaDescription, codecName: string /** * Retain specific codecs, filtering out unwanted ones from the given SDP or audio/video media - * description. + * description. The provided predicate should take in a single {@link codecInfo}, and only codecs + * for which the predicate returns true will be retained. * - * Note: Done this way because of a feature not implemented in all browsers, currently missing in - * Firefox. Once that is added we can use `RTPSender.getCapabilities` and filter those to call - * with `RTCRtpTransceiver.setCodecPreferences` instead of doing this manually. + * Note: Done this way because of a feature that was only recently implemented in all browsers, + * previously missing in Firefox. You can also use `RTPSender.getCapabilities` and filter those to + * call with `RTCRtpTransceiver.setCodecPreferences` instead of doing this manually. * * @param sdpOrAv - The {@link Sdp} or {@link AvMediaDescription} from which to filter codecs. - * @param allowedCodecNames - The names of the codecs that should remain in the SDP. + * @param predicate - A function used to determine which codecs should be retained. + * @returns A boolean that indicates if some codecs have been filtered out. */ export function retainCodecs( sdpOrAv: Sdp | AvMediaDescription, - allowedCodecNames: Array -): void { + predicate: (codecInfo: CodecInfo) => boolean +): boolean { const avMediaDescriptions = sdpOrAv instanceof Sdp ? sdpOrAv.avMedia : [sdpOrAv]; + let filtered = false; + + avMediaDescriptions.forEach((av) => { + av.codecs.forEach((codecInfo) => { + if (!predicate(codecInfo)) { + av.removePt(codecInfo.pt); + filtered = true; + } + }); + }); + + return filtered; +} + +/** + * Retain specific codecs, filtering out unwanted ones from the given SDP or audio/video media + * description by codec name. + * + * @param sdpOrAv - The {@link Sdp} or {@link AvMediaDescription} from which to filter codecs. + * @param allowedCodecNames - The names of the codecs that should remain in the SDP. + * @returns A boolean that indicates if some codecs have been filtered out. + */ +export function retainCodecsByCodecName( + sdpOrAv: Sdp | AvMediaDescription, + allowedCodecNames: Array +): boolean { const allowedLowerCase = allowedCodecNames.map((s) => s.toLowerCase()); - avMediaDescriptions - .map((av) => { - return [...av.codecs.values()].map((c) => c.name as string); - }) - .flat() - .filter((codecName) => !allowedLowerCase.includes(codecName.toLowerCase())) - .forEach((unwantedCodec) => removeCodec(sdpOrAv, unwantedCodec)); + return retainCodecs(sdpOrAv, (codecInfo) => + allowedLowerCase.includes(codecInfo.name?.toLowerCase() as string) + ); } /** - * Retain specific candidates, filtering out unwanted ones from the given SDP or media description - * by transport type. + * Retain specific candidates, filtering out unwanted ones from the given SDP or media description. + * The provided predicate should take in a single {@link CandidateLine}, and only candidates for + * which the predicate returns true will be retained. * * @param sdpOrMedia - The {@link Sdp} or {@link MediaDescription} from which to filter candidates. - * @param allowedTransportTypes - The names of the transport types of the candidates that should remain in the SDP. + * @param predicate - A function used to determine which candidates should be retained. * @returns A boolean that indicates if some candidates have been filtered out. */ export function retainCandidates( sdpOrMedia: Sdp | MediaDescription, - allowedTransportTypes: Array + predicate: (candidate: CandidateLine) => boolean ) { const mediaDescriptions = sdpOrMedia instanceof Sdp ? sdpOrMedia.media : [sdpOrMedia]; let filtered = false; @@ -112,7 +138,7 @@ export function retainCandidates( mediaDescriptions.forEach((media) => { // eslint-disable-next-line no-param-reassign media.iceInfo.candidates = media.iceInfo.candidates.filter((candidate) => { - if (allowedTransportTypes.includes(candidate.transport.toLowerCase())) { + if (predicate(candidate)) { return true; } filtered = true; @@ -122,3 +148,22 @@ export function retainCandidates( return filtered; } + +/** + * Retain specific candidates, filtering out unwanted ones from the given SDP or media description + * by transport type. + * + * @param sdpOrMedia - The {@link Sdp} or {@link MediaDescription} from which to filter candidates. + * @param allowedTransportTypes - The names of the transport types of the candidates that should remain in the SDP. + * @returns A boolean that indicates if some candidates have been filtered out. + */ +export function retainCandidatesByTransportType( + sdpOrMedia: Sdp | MediaDescription, + allowedTransportTypes: Array +) { + const allowedLowerCase = allowedTransportTypes.map((s) => s.toLowerCase()); + + return retainCandidates(sdpOrMedia, (candidate) => + allowedLowerCase.includes(candidate.transport.toLowerCase()) + ); +} From 68c8527d357e64916b866a5555a19f0740be94ed Mon Sep 17 00:00:00 2001 From: Bryce Tham Date: Tue, 30 Jul 2024 13:59:57 -0400 Subject: [PATCH 2/3] fix: safely handle missing codec name --- src/munge.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/munge.ts b/src/munge.ts index 109197d..6655508 100644 --- a/src/munge.ts +++ b/src/munge.ts @@ -114,8 +114,9 @@ export function retainCodecsByCodecName( ): boolean { const allowedLowerCase = allowedCodecNames.map((s) => s.toLowerCase()); - return retainCodecs(sdpOrAv, (codecInfo) => - allowedLowerCase.includes(codecInfo.name?.toLowerCase() as string) + return retainCodecs( + sdpOrAv, + (codecInfo) => !!codecInfo.name && allowedLowerCase.includes(codecInfo.name.toLowerCase()) ); } From afe9e69c9dc81d35f8bf50f3602f9d144b0a5338 Mon Sep 17 00:00:00 2001 From: Bryce Tham Date: Tue, 30 Jul 2024 14:09:28 -0400 Subject: [PATCH 3/3] fix: pass only AvMediaDescription when retaining codecs --- src/munge.spec.ts | 26 -------------------------- src/munge.ts | 35 ++++++++++++++++------------------- 2 files changed, 16 insertions(+), 45 deletions(-) diff --git a/src/munge.spec.ts b/src/munge.spec.ts index 3b7851f..5ee6cb6 100644 --- a/src/munge.spec.ts +++ b/src/munge.spec.ts @@ -91,21 +91,6 @@ describe('munging', () => { }); describe('retainCodecs', () => { - it('should retain codecs correctly when passing in an SDP', () => { - expect.hasAssertions(); - const offer = fs.readFileSync('./src/sdp-corpus/offer_with_extra_codecs.sdp', 'utf-8'); - const parsed = parse(offer); - - // eslint-disable-next-line jsdoc/require-jsdoc - const predicate = (codecInfo: CodecInfo) => - codecInfo.name === 'h264' || codecInfo.name === 'opus'; - - // should return true when some codecs have been filtered out - expect(retainCodecs(parsed, predicate)).toBeTruthy(); - expect(validateOfferCodecs(parsed)).toBe(true); - // should return false when no codecs have been filtered out - expect(retainCodecs(parsed, predicate)).toBeFalsy(); - }); it('should retain codecs correctly when passing in an AvMediaDescription', () => { expect.hasAssertions(); const offer = fs.readFileSync('./src/sdp-corpus/offer_with_extra_codecs.sdp', 'utf-8'); @@ -125,17 +110,6 @@ describe('munging', () => { expect(retainCodecs(av, predicate)).toBeFalsy(); }); }); - it('should retain codecs by name when passing in an SDP', () => { - expect.hasAssertions(); - const offer = fs.readFileSync('./src/sdp-corpus/offer_with_extra_codecs.sdp', 'utf-8'); - const parsed = parse(offer); - - // should return true when some codecs have been filtered out - expect(retainCodecsByCodecName(parsed, ['h264', 'opus'])).toBeTruthy(); - expect(validateOfferCodecs(parsed)).toBe(true); - // should return false when no codecs have been filtered out - expect(retainCodecsByCodecName(parsed, ['h264', 'opus'])).toBeFalsy(); - }); it('should retain codecs by name when passing in an AvMediaDescription', () => { expect.hasAssertions(); const offer = fs.readFileSync('./src/sdp-corpus/offer_with_extra_codecs.sdp', 'utf-8'); diff --git a/src/munge.ts b/src/munge.ts index 6655508..71c2f12 100644 --- a/src/munge.ts +++ b/src/munge.ts @@ -69,53 +69,50 @@ export function removeCodec(sdpOrAv: Sdp | AvMediaDescription, codecName: string } /** - * Retain specific codecs, filtering out unwanted ones from the given SDP or audio/video media - * description. The provided predicate should take in a single {@link codecInfo}, and only codecs - * for which the predicate returns true will be retained. + * Retain specific codecs, filtering out unwanted ones from the given audio/video media description. + * The provided predicate should take in a single {@link codecInfo}, and only codecs for which the + * predicate returns true will be retained. * * Note: Done this way because of a feature that was only recently implemented in all browsers, * previously missing in Firefox. You can also use `RTPSender.getCapabilities` and filter those to * call with `RTCRtpTransceiver.setCodecPreferences` instead of doing this manually. * - * @param sdpOrAv - The {@link Sdp} or {@link AvMediaDescription} from which to filter codecs. + * @param av - The {@link AvMediaDescription} from which to filter codecs. * @param predicate - A function used to determine which codecs should be retained. * @returns A boolean that indicates if some codecs have been filtered out. */ export function retainCodecs( - sdpOrAv: Sdp | AvMediaDescription, + av: AvMediaDescription, predicate: (codecInfo: CodecInfo) => boolean ): boolean { - const avMediaDescriptions = sdpOrAv instanceof Sdp ? sdpOrAv.avMedia : [sdpOrAv]; let filtered = false; - avMediaDescriptions.forEach((av) => { - av.codecs.forEach((codecInfo) => { - if (!predicate(codecInfo)) { - av.removePt(codecInfo.pt); - filtered = true; - } - }); + av.codecs.forEach((codecInfo) => { + if (!predicate(codecInfo)) { + av.removePt(codecInfo.pt); + filtered = true; + } }); return filtered; } /** - * Retain specific codecs, filtering out unwanted ones from the given SDP or audio/video media - * description by codec name. + * Retain specific codecs, filtering out unwanted ones from the given audio/video media description + * by codec name. * - * @param sdpOrAv - The {@link Sdp} or {@link AvMediaDescription} from which to filter codecs. - * @param allowedCodecNames - The names of the codecs that should remain in the SDP. + * @param av - The {@link AvMediaDescription} from which to filter codecs. + * @param allowedCodecNames - The names of the codecs that should remain in the media description. * @returns A boolean that indicates if some codecs have been filtered out. */ export function retainCodecsByCodecName( - sdpOrAv: Sdp | AvMediaDescription, + av: AvMediaDescription, allowedCodecNames: Array ): boolean { const allowedLowerCase = allowedCodecNames.map((s) => s.toLowerCase()); return retainCodecs( - sdpOrAv, + av, (codecInfo) => !!codecInfo.name && allowedLowerCase.includes(codecInfo.name.toLowerCase()) ); }