diff --git a/src/lines/fmtp-line.ts b/src/lines/fmtp-line.ts index c483ca9..f20d796 100644 --- a/src/lines/fmtp-line.ts +++ b/src/lines/fmtp-line.ts @@ -16,6 +16,7 @@ import { NUM, REST } from '../regex-helpers'; import { Line } from './line'; +import { PayloadTypeRef } from './payload-type-ref'; /** * Parse the fmtpParams because SDP has no such util function for that. @@ -60,7 +61,7 @@ export function parseFmtpParams(fmtpParams: string) { * a=fmtp:97 apt=96 */ export class FmtpLine extends Line { - payloadType: number; + payloadType: PayloadTypeRef; params: Map; @@ -72,7 +73,7 @@ export class FmtpLine extends Line { * @param payloadType - The payload type. * @param params - The fmtp parameters. */ - constructor(payloadType: number, params: Map) { + constructor(payloadType: PayloadTypeRef, params: Map) { super(); this.payloadType = payloadType; this.params = params; @@ -89,7 +90,7 @@ export class FmtpLine extends Line { return undefined; } const tokens = line.match(FmtpLine.regex) as RegExpMatchArray; - const payloadType = parseInt(tokens[1], 10); + const payloadType = new PayloadTypeRef(parseInt(tokens[1], 10)); const params = tokens[2]; return new FmtpLine(payloadType, parseFmtpParams(params)); diff --git a/src/lines/index.ts b/src/lines/index.ts index de69301..0baa338 100644 --- a/src/lines/index.ts +++ b/src/lines/index.ts @@ -31,6 +31,7 @@ export * from './max-message-size-line'; export * from './media-line'; export * from './mid-line'; export * from './origin-line'; +export * from './payload-type-ref'; export * from './rid-line'; export * from './rtcp-mux-line'; export * from './rtcpfb-line'; diff --git a/src/lines/payload-type-ref.spec.ts b/src/lines/payload-type-ref.spec.ts new file mode 100644 index 0000000..7144258 --- /dev/null +++ b/src/lines/payload-type-ref.spec.ts @@ -0,0 +1,63 @@ +import { PayloadTypeRef } from './payload-type-ref'; + +describe('payloadTypeRef', () => { + describe('numeric', () => { + it('should store a numeric value', () => { + expect.hasAssertions(); + const pt = new PayloadTypeRef(96); + expect(pt.value).toBe(96); + }); + + it('should not be a wildcard', () => { + expect.hasAssertions(); + const pt = new PayloadTypeRef(96); + expect(pt.isWildcard()).toBe(false); + }); + + it('should match the same numeric value', () => { + expect.hasAssertions(); + const pt = new PayloadTypeRef(96); + expect(pt.matches(96)).toBe(true); + }); + + it('should not match a different numeric value', () => { + expect.hasAssertions(); + const pt = new PayloadTypeRef(96); + expect(pt.matches(97)).toBe(false); + }); + + it('should serialize to string as the number', () => { + expect.hasAssertions(); + const pt = new PayloadTypeRef(96); + expect(pt.toString()).toBe('96'); + }); + }); + + describe('wildcard', () => { + it('should store the wildcard value', () => { + expect.hasAssertions(); + const pt = new PayloadTypeRef('*'); + expect(pt.value).toBe('*'); + }); + + it('should be a wildcard', () => { + expect.hasAssertions(); + const pt = new PayloadTypeRef('*'); + expect(pt.isWildcard()).toBe(true); + }); + + it('should match any numeric value', () => { + expect.hasAssertions(); + const pt = new PayloadTypeRef('*'); + expect(pt.matches(96)).toBe(true); + expect(pt.matches(0)).toBe(true); + expect(pt.matches(127)).toBe(true); + }); + + it('should serialize to string as *', () => { + expect.hasAssertions(); + const pt = new PayloadTypeRef('*'); + expect(pt.toString()).toBe('*'); + }); + }); +}); diff --git a/src/lines/payload-type-ref.ts b/src/lines/payload-type-ref.ts new file mode 100644 index 0000000..6fb0219 --- /dev/null +++ b/src/lines/payload-type-ref.ts @@ -0,0 +1,45 @@ +/** + * Represents a payload type reference as it appears in SDP attribute lines. + * Can be either a numeric payload type or a wildcard (*) meaning "all payload types". + */ +export class PayloadTypeRef { + readonly value: number | '*'; + + /** + * Create a PayloadTypeRef. + * + * @param value - A numeric payload type or '*' for wildcard. + */ + constructor(value: number | '*') { + this.value = value; + } + + /** + * Check if this reference matches the given numeric payload type. + * A wildcard matches any payload type. + * + * @param pt - The numeric payload type to match against. + * @returns True if this reference matches the given payload type. + */ + matches(pt: number): boolean { + return this.value === '*' || this.value === pt; + } + + /** + * Check if this is a wildcard reference. + * + * @returns True if this is a wildcard reference. + */ + isWildcard(): boolean { + return this.value === '*'; + } + + /** + * Serialize to string. + * + * @returns The string representation. + */ + toString(): string { + return `${this.value}`; + } +} diff --git a/src/lines/rtcpfb-line.spec.ts b/src/lines/rtcpfb-line.spec.ts new file mode 100644 index 0000000..6055819 --- /dev/null +++ b/src/lines/rtcpfb-line.spec.ts @@ -0,0 +1,51 @@ +import { RtcpFbLine } from './rtcpfb-line'; + +describe('rtcpFbLine', () => { + describe('fromSdpLine', () => { + it('should parse a numeric payload type', () => { + expect.hasAssertions(); + const line = RtcpFbLine.fromSdpLine('rtcp-fb:96 goog-remb'); + expect(line).toBeDefined(); + expect(line?.payloadType.value).toBe(96); + expect(line?.payloadType.isWildcard()).toBe(false); + expect(line?.feedback).toBe('goog-remb'); + }); + + it('should parse a wildcard payload type', () => { + expect.hasAssertions(); + const line = RtcpFbLine.fromSdpLine('rtcp-fb:* nack'); + expect(line).toBeDefined(); + expect(line?.payloadType.value).toBe('*'); + expect(line?.payloadType.isWildcard()).toBe(true); + expect(line?.feedback).toBe('nack'); + }); + + it('should parse a wildcard payload type with compound feedback', () => { + expect.hasAssertions(); + const line = RtcpFbLine.fromSdpLine('rtcp-fb:* nack pli'); + expect(line).toBeDefined(); + expect(line?.payloadType.value).toBe('*'); + expect(line?.feedback).toBe('nack pli'); + }); + + it('should return undefined for invalid line', () => { + expect.hasAssertions(); + const line = RtcpFbLine.fromSdpLine('rtcp-fb:abc nack'); + expect(line).toBeUndefined(); + }); + }); + + describe('toSdpLine', () => { + it('should serialize a numeric payload type', () => { + expect.hasAssertions(); + const line = RtcpFbLine.fromSdpLine('rtcp-fb:96 goog-remb'); + expect(line?.toSdpLine()).toBe('a=rtcp-fb:96 goog-remb'); + }); + + it('should serialize a wildcard payload type', () => { + expect.hasAssertions(); + const line = RtcpFbLine.fromSdpLine('rtcp-fb:* nack pli'); + expect(line?.toSdpLine()).toBe('a=rtcp-fb:* nack pli'); + }); + }); +}); diff --git a/src/lines/rtcpfb-line.ts b/src/lines/rtcpfb-line.ts index 44a72e6..e2fa6c8 100644 --- a/src/lines/rtcpfb-line.ts +++ b/src/lines/rtcpfb-line.ts @@ -16,6 +16,7 @@ import { NUM, REST } from '../regex-helpers'; import { Line } from './line'; +import { PayloadTypeRef } from './payload-type-ref'; /** * Implementation of an rtcp-fb attribute as defined by https://datatracker.ietf.org/doc/html/rfc4585#section-4.2. @@ -24,11 +25,11 @@ import { Line } from './line'; * a=rtcp-fb:96 goog-remb */ export class RtcpFbLine extends Line { - payloadType: number; + payloadType: PayloadTypeRef; feedback: string; - private static regex = new RegExp(`^rtcp-fb:(${NUM}) (${REST})`); + private static regex = new RegExp(`^rtcp-fb:(${NUM}|\\*) (${REST})`); /** * Create an RtcpFbLine from the given values. @@ -36,7 +37,7 @@ export class RtcpFbLine extends Line { * @param payloadType - The payload type. * @param feedback - The feedback name. */ - constructor(payloadType: number, feedback: string) { + constructor(payloadType: PayloadTypeRef, feedback: string) { super(); this.payloadType = payloadType; this.feedback = feedback; @@ -53,7 +54,9 @@ export class RtcpFbLine extends Line { return undefined; } const tokens = line.match(RtcpFbLine.regex) as RegExpMatchArray; - const payloadType = parseInt(tokens[1], 10); + const ptToken = tokens[1]; + const payloadType = + ptToken === '*' ? new PayloadTypeRef('*') : new PayloadTypeRef(parseInt(ptToken, 10)); const feedback = tokens[2]; return new RtcpFbLine(payloadType, feedback); diff --git a/src/lines/rtpmap-line.ts b/src/lines/rtpmap-line.ts index af8fa2a..aae2b21 100644 --- a/src/lines/rtpmap-line.ts +++ b/src/lines/rtpmap-line.ts @@ -16,6 +16,7 @@ import { NUM } from '../regex-helpers'; import { Line } from './line'; +import { PayloadTypeRef } from './payload-type-ref'; /** * Definition of an rtpmap attribute line as defined in https://datatracker.ietf.org/doc/html/rfc4566#section-6. @@ -24,7 +25,7 @@ import { Line } from './line'; * a=rtpmap:96 VP8/90000 */ export class RtpMapLine extends Line { - payloadType: number; + payloadType: PayloadTypeRef; encodingName: string; @@ -52,7 +53,7 @@ export class RtpMapLine extends Line { * @param encodingParams - Optional additional encoding parameters. */ constructor( - payloadType: number, + payloadType: PayloadTypeRef, encodingName: string, clockRate: number, encodingParams?: string @@ -75,7 +76,7 @@ export class RtpMapLine extends Line { return undefined; } const tokens = line.match(RtpMapLine.regex) as RegExpMatchArray; - const payloadType = parseInt(tokens[1], 10); + const payloadType = new PayloadTypeRef(parseInt(tokens[1], 10)); const encodingName = tokens[2]; const clockRate = parseInt(tokens[3], 10); // encodingParams, if present, will be in index 4 diff --git a/src/model/av-media-description.ts b/src/model/av-media-description.ts index 65596fa..6453973 100644 --- a/src/model/av-media-description.ts +++ b/src/model/av-media-description.ts @@ -35,6 +35,7 @@ import { SsrcLine, } from '../lines'; import { CodecInfo } from './codec-info'; +import { CodecStore } from './codec-store'; import { MediaDescription } from './media-description'; /** @@ -49,7 +50,7 @@ export class AvMediaDescription extends MediaDescription { simulcast?: SimulcastLine; - codecs: Map = new Map(); + codecs: CodecStore = new CodecStore(); direction?: MediaDirection; @@ -115,7 +116,7 @@ export class AvMediaDescription extends MediaDescription { if (this.direction) { lines.push(new DirectionLine(this.direction as MediaDirection)); } - this.codecs.forEach((codec) => lines.push(...codec.toLines())); + lines.push(...this.codecs.toLines()); lines.push(...this.ssrcs); lines.push(...this.ssrcGroups); @@ -162,11 +163,7 @@ export class AvMediaDescription extends MediaDescription { } // Lines pertaining to a specific codec if (line instanceof RtpMapLine || line instanceof FmtpLine || line instanceof RtcpFbLine) { - const codec = this.codecs.get(line.payloadType); - if (!codec) { - throw new Error(`Error: got line for unknown codec: ${line.toSdpLine()}`); - } - codec.addLine(line); + this.codecs.addLine(line); return true; } if (line instanceof SsrcLine) { diff --git a/src/model/codec-info.ts b/src/model/codec-info.ts index ee7d267..da9f7ae 100644 --- a/src/model/codec-info.ts +++ b/src/model/codec-info.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { FmtpLine, Line, RtcpFbLine, RtpMapLine } from '../lines'; +import { FmtpLine, Line, PayloadTypeRef, RtcpFbLine, RtpMapLine } from '../lines'; import { SdpBlock } from './sdp-block'; /** @@ -82,16 +82,21 @@ export class CodecInfo implements SdpBlock { // First the RtpMap if (this.name && this.clockRate) { lines.push( - new RtpMapLine(this.pt, this.name as string, this.clockRate as number, this.encodingParams) + new RtpMapLine( + new PayloadTypeRef(this.pt), + this.name as string, + this.clockRate as number, + this.encodingParams + ) ); } // Now all RtcpFb this.feedback.forEach((fb) => { - lines.push(new RtcpFbLine(this.pt, fb)); + lines.push(new RtcpFbLine(new PayloadTypeRef(this.pt), fb)); }); // Now all Fmtp if (this.fmtParams.size > 0) { - lines.push(new FmtpLine(this.pt, this.fmtParams)); + lines.push(new FmtpLine(new PayloadTypeRef(this.pt), this.fmtParams)); } return lines; } diff --git a/src/model/codec-store.spec.ts b/src/model/codec-store.spec.ts new file mode 100644 index 0000000..80611aa --- /dev/null +++ b/src/model/codec-store.spec.ts @@ -0,0 +1,184 @@ +import { FmtpLine, PayloadTypeRef, RtcpFbLine, RtpMapLine } from '../lines'; +import { CodecInfo } from './codec-info'; +import { CodecStore } from './codec-store'; + +describe('codecStore', () => { + let store: CodecStore; + + beforeEach(() => { + store = new CodecStore(); + store.set(96, new CodecInfo(96)); + store.set(97, new CodecInfo(97)); + }); + + describe('codec storage operations', () => { + it('should get a codec by pt', () => { + expect.hasAssertions(); + expect(store.get(96)).toBeDefined(); + expect(store.get(96)?.pt).toBe(96); + }); + + it('should check if a codec exists', () => { + expect.hasAssertions(); + expect(store.has(96)).toBe(true); + expect(store.has(99)).toBe(false); + }); + + it('should delete a codec', () => { + expect.hasAssertions(); + store.delete(96); + expect(store.has(96)).toBe(false); + }); + + it('should iterate with forEach', () => { + expect.hasAssertions(); + const pts: number[] = []; + store.forEach((codec) => pts.push(codec.pt)); + expect(pts).toContain(96); + expect(pts).toContain(97); + }); + + it('should return values', () => { + expect.hasAssertions(); + const pts = [...store.values()].map((c) => c.pt); + expect(pts).toContain(96); + expect(pts).toContain(97); + }); + + it('should return entries', () => { + expect.hasAssertions(); + const entries = [...store.entries()]; + expect(entries).toHaveLength(2); + expect(entries.find(([pt]) => pt === 96)).toBeDefined(); + }); + }); + + describe('payloadTypeExists', () => { + it('should return true for a known numeric payload type', () => { + expect.hasAssertions(); + expect(store.payloadTypeExists(new PayloadTypeRef(96))).toBe(true); + }); + + it('should return false for an unknown numeric payload type', () => { + expect.hasAssertions(); + expect(store.payloadTypeExists(new PayloadTypeRef(99))).toBe(false); + }); + + it('should return true for a wildcard payload type', () => { + expect.hasAssertions(); + expect(store.payloadTypeExists(new PayloadTypeRef('*'))).toBe(true); + }); + }); + + describe('addLine', () => { + it('should route a numeric RtcpFbLine to the correct codec', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef(96), 'nack')); + expect(store.get(96)?.feedback).toContain('nack'); + expect(store.get(97)?.feedback).not.toContain('nack'); + }); + + it('should route a wildcard RtcpFbLine to wildcard feedback', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef('*'), 'goog-remb')); + expect(store.getFeedback(96)).toContain('goog-remb'); + expect(store.getFeedback(97)).toContain('goog-remb'); + }); + + it('should store wildcard feedback separately from per-codec feedback', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef('*'), 'nack')); + expect(store.get(96)?.feedback).not.toContain('nack'); + expect(store.get(97)?.feedback).not.toContain('nack'); + }); + + it('should route an RtpMapLine to the correct codec', () => { + expect.hasAssertions(); + store.addLine(new RtpMapLine(new PayloadTypeRef(96), 'VP8', 90000)); + expect(store.get(96)?.name).toBe('VP8'); + }); + + it('should route an FmtpLine to the correct codec', () => { + expect.hasAssertions(); + store.addLine(new FmtpLine(new PayloadTypeRef(96), new Map([['apt', '97']]))); + expect(store.get(96)?.fmtParams.get('apt')).toBe('97'); + }); + + it('should throw for an unknown numeric payload type', () => { + expect.hasAssertions(); + expect(() => store.addLine(new RtcpFbLine(new PayloadTypeRef(99), 'nack'))).toThrow( + /unknown codec/ + ); + }); + + it('should throw for a wildcard non-RtcpFbLine', () => { + expect.hasAssertions(); + expect(() => store.addLine(new RtpMapLine(new PayloadTypeRef('*'), 'VP8', 90000))).toThrow( + /wildcard/i + ); + }); + + it('should apply wildcard feedback to codecs added after the wildcard line', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef('*'), 'goog-remb')); + store.set(100, new CodecInfo(100)); + expect(store.getFeedback(100)).toContain('goog-remb'); + }); + }); + + describe('getFeedback', () => { + it('should return per-codec feedback', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef(96), 'nack')); + expect(store.getFeedback(96)).toContain('nack'); + expect(store.getFeedback(97)).not.toContain('nack'); + }); + + it('should include wildcard feedback for any codec', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef('*'), 'goog-remb')); + expect(store.getFeedback(96)).toContain('goog-remb'); + expect(store.getFeedback(97)).toContain('goog-remb'); + }); + + it('should merge per-codec and wildcard feedback', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef(96), 'nack')); + store.addLine(new RtcpFbLine(new PayloadTypeRef('*'), 'goog-remb')); + const fb = store.getFeedback(96); + expect(fb).toContain('nack'); + expect(fb).toContain('goog-remb'); + }); + }); + + describe('removeFeedback', () => { + it('should remove feedback from all codecs and wildcard', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef(96), 'nack')); + store.addLine(new RtcpFbLine(new PayloadTypeRef('*'), 'nack')); + store.removeFeedback('nack'); + expect(store.getFeedback(96)).not.toContain('nack'); + expect(store.getFeedback(97)).not.toContain('nack'); + }); + }); + + describe('toLines', () => { + it('should emit per-codec lines from codec info', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef(96), 'nack')); + const lines = store.toLines(); + const fbLines = lines.filter((l) => l instanceof RtcpFbLine); + expect(fbLines).toHaveLength(1); + expect(fbLines[0].toSdpLine()).toBe('a=rtcp-fb:96 nack'); + }); + + it('should emit wildcard feedback as rtcp-fb:* lines', () => { + expect.hasAssertions(); + store.addLine(new RtcpFbLine(new PayloadTypeRef('*'), 'goog-remb')); + const lines = store.toLines(); + const fbLines = lines.filter((l) => l instanceof RtcpFbLine); + expect(fbLines).toHaveLength(1); + expect(fbLines[0].toSdpLine()).toBe('a=rtcp-fb:* goog-remb'); + }); + }); +}); diff --git a/src/model/codec-store.ts b/src/model/codec-store.ts new file mode 100644 index 0000000..c70c679 --- /dev/null +++ b/src/model/codec-store.ts @@ -0,0 +1,150 @@ +import { FmtpLine, Line, PayloadTypeRef, RtcpFbLine, RtpMapLine } from '../lines'; +import { CodecInfo } from './codec-info'; + +/** + * A store for codec information that handles wildcard rtcp-fb feedback. + * Wildcard feedback (rtcp-fb:*) applies to all codecs and is stored separately + * from per-codec feedback, preserving the wildcard semantics through round-trips. + */ +export class CodecStore { + private codecs: Map = new Map(); + + private wildcardFeedback: string[] = []; + + /** + * Get the CodecInfo for the given payload type. + * + * @param pt - The payload type. + * @returns The CodecInfo, or undefined if not found. + */ + get(pt: number): CodecInfo | undefined { + return this.codecs.get(pt); + } + + /** + * Set a CodecInfo for the given payload type. + * + * @param pt - The payload type. + * @param codec - The CodecInfo to store. + */ + set(pt: number, codec: CodecInfo) { + this.codecs.set(pt, codec); + } + + /** + * Check if a codec exists for the given payload type. + * + * @param pt - The payload type. + * @returns True if a codec exists for this payload type. + */ + has(pt: number): boolean { + return this.codecs.has(pt); + } + + /** + * Delete the codec for the given payload type. + * + * @param pt - The payload type. + */ + delete(pt: number) { + this.codecs.delete(pt); + } + + /** + * Iterate over all codecs. + * + * @param cb - The callback to invoke for each codec. + */ + forEach(cb: (codec: CodecInfo, pt: number, map: Map) => void) { + this.codecs.forEach(cb); + } + + /** + * Get an iterator over all CodecInfo values. + * + * @returns An iterator of CodecInfo values. + */ + values(): IterableIterator { + return this.codecs.values(); + } + + /** + * Get an iterator over all [pt, CodecInfo] entries. + * + * @returns An iterator of entries. + */ + entries(): IterableIterator<[number, CodecInfo]> { + return this.codecs.entries(); + } + + /** + * Check if the given payload type reference is valid. A wildcard is always valid; + * a numeric payload type is valid if it was defined on the m-line. + * + * @param pt - The payload type reference to check. + * @returns True if the payload type is valid. + */ + payloadTypeExists(pt: PayloadTypeRef): boolean { + return pt.isWildcard() || this.codecs.has(pt.value as number); + } + + /** + * Add a codec-related line. Validates the payload type exists, then routes + * wildcard lines to wildcard feedback storage and numeric lines to the + * appropriate CodecInfo. + * + * @param line - The line to add. + */ + addLine(line: RtpMapLine | FmtpLine | RtcpFbLine) { + if (!this.payloadTypeExists(line.payloadType)) { + throw new Error(`Error: got line for unknown codec: ${line.toSdpLine()}`); + } + if (line.payloadType.isWildcard()) { + if (!(line instanceof RtcpFbLine)) { + throw new Error(`Error: wildcard payload type is only valid for rtcp-fb lines`); + } + this.wildcardFeedback.push(line.feedback); + } else { + const codec = this.codecs.get(line.payloadType.value as number) as CodecInfo; + codec.addLine(line); + } + } + + /** + * Get all feedback for a given numeric payload type, including any wildcard feedback. + * + * @param pt - The numeric payload type. + * @returns An array of all feedback strings that apply to this payload type. + */ + getFeedback(pt: number): string[] { + const codec = this.codecs.get(pt); + return [...(codec?.feedback ?? []), ...this.wildcardFeedback]; + } + + /** + * Remove a feedback value from all codecs and from wildcard feedback. + * + * @param feedback - The feedback value to remove. + */ + removeFeedback(feedback: string) { + this.codecs.forEach((codec) => { + // eslint-disable-next-line no-param-reassign + codec.feedback = codec.feedback.filter((fb) => fb !== feedback); + }); + this.wildcardFeedback = this.wildcardFeedback.filter((fb) => fb !== feedback); + } + + /** + * Serialize all codec info and wildcard feedback to SDP lines. + * + * @returns An array of Lines. + */ + toLines(): Line[] { + const lines: Line[] = []; + this.codecs.forEach((codec) => lines.push(...codec.toLines())); + this.wildcardFeedback.forEach((fb) => { + lines.push(new RtcpFbLine(new PayloadTypeRef('*'), fb)); + }); + return lines; + } +} diff --git a/src/model/index.ts b/src/model/index.ts index 8494b57..15cf066 100644 --- a/src/model/index.ts +++ b/src/model/index.ts @@ -17,6 +17,7 @@ export * from './application-media-description'; export * from './av-media-description'; export * from './codec-info'; +export * from './codec-store'; export * from './ice-info'; export * from './media-description'; export * from './sdp-block'; diff --git a/src/munge.spec.ts b/src/munge.spec.ts index 5ee6cb6..c1a9f88 100644 --- a/src/munge.spec.ts +++ b/src/munge.spec.ts @@ -234,6 +234,17 @@ describe('munging', () => { expect(checkOfferContainsRtcpFeedback(parsed, 'transport-cc')).toBe(true); expect(checkOfferContainsRtcpFeedback(parsed, 'goog-remb')).toBe(true); }); + it('should remove wildcard rtcp feedback from serialized output', () => { + expect.hasAssertions(); + const sdp = fs.readFileSync('./src/sdp-corpus/wildcard_rtcpfb.sdp', 'utf-8'); + const parsed = parse(sdp); + + disableRtcpFbValue(parsed, 'goog-remb'); + const output = parsed.toString(); + expect(output).not.toContain('a=rtcp-fb:* goog-remb'); + expect(output).toContain('a=rtcp-fb:* nack\r\n'); + expect(output).toContain('a=rtcp-fb:96 ccm fir'); + }); }); describe('disableTwcc', () => { diff --git a/src/munge.ts b/src/munge.ts index 71c2f12..9f3b924 100644 --- a/src/munge.ts +++ b/src/munge.ts @@ -26,10 +26,7 @@ import { AvMediaDescription, CodecInfo, MediaDescription, Sdp } from './model'; export function disableRtcpFbValue(sdpOrAv: Sdp | AvMediaDescription, rtcpFbValue: string) { const mediaDescriptions = sdpOrAv instanceof Sdp ? sdpOrAv.avMedia : [sdpOrAv]; mediaDescriptions.forEach((media: AvMediaDescription) => { - media.codecs.forEach((codec: CodecInfo) => { - // eslint-disable-next-line no-param-reassign - codec.feedback = codec.feedback.filter((fb) => fb !== rtcpFbValue); - }); + media.codecs.removeFeedback(rtcpFbValue); }); } diff --git a/src/parser.spec.ts b/src/parser.spec.ts index 9453fbc..e91d78f 100644 --- a/src/parser.spec.ts +++ b/src/parser.spec.ts @@ -157,6 +157,16 @@ describe('parsing', () => { ); }); + describe('wildcard rtcp-fb', () => { + it('should round-trip correctly', () => { + expect.hasAssertions(); + const file = fs.readFileSync('./src/sdp-corpus/wildcard_rtcpfb.sdp', 'utf-8'); + const result = parse(file); + const str = result.toString(); + compareSdps(str, file); + }); + }); + it('should handle media sections without any rtpmap lines', () => { expect.hasAssertions(); const sdpWithoutRtpmapLines = fs.readFileSync('./src/sdp-corpus/without_rtpmap.sdp', 'utf-8'); diff --git a/src/sdp-corpus/wildcard_rtcpfb.sdp b/src/sdp-corpus/wildcard_rtcpfb.sdp new file mode 100644 index 0000000..2e3e028 --- /dev/null +++ b/src/sdp-corpus/wildcard_rtcpfb.sdp @@ -0,0 +1,13 @@ +v=0 +o=- 0 0 IN IP4 0.0.0.0 +s=- +t=0 0 +m=video 9 RTP/AVP 96 97 +c=IN IP4 0.0.0.0 +a=rtpmap:96 VP8/90000 +a=rtpmap:97 rtx/90000 +a=fmtp:97 apt=96 +a=rtcp-fb:* goog-remb +a=rtcp-fb:* nack +a=rtcp-fb:* nack pli +a=rtcp-fb:96 ccm fir \ No newline at end of file diff --git a/src/utils.spec.ts b/src/utils.spec.ts index 9d7a1c0..47e493d 100644 --- a/src/utils.spec.ts +++ b/src/utils.spec.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { MediaLine, RtpMapLine } from './lines'; +import { MediaLine, PayloadTypeRef, RtpMapLine } from './lines'; import { AvMediaDescription } from './model'; import { hasCodec } from './utils'; @@ -24,7 +24,7 @@ describe('hasCodec', () => { const mLine = new AvMediaDescription( new MediaLine('video', 9, 'UDP/TLS/RTP/SAVPF', ['96', '97', '98', '99', '100', '101', '127']) ); - mLine.addLine(new RtpMapLine(96, 'h264', 9000)); + mLine.addLine(new RtpMapLine(new PayloadTypeRef(96), 'h264', 9000)); expect(hasCodec('h264', mLine)).toBe(true); expect(hasCodec('H264', mLine)).toBe(true);