fix!: correctly support wildcard payload types in feedback lines

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<string, string | undefined>;
 
@@ -72,7 +73,7 @@ export class FmtpLine extends Line {
    * @param payloadType - The payload type.
    * @param params - The fmtp parameters.
    */
-  constructor(payloadType: number, params: Map<string, string | undefined>) {
+  constructor(payloadType: PayloadTypeRef, params: Map<string, string | undefined>) {
     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));

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';

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('*');
+    });
+  });
+});

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}`;
+  }
+}

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');
+    });
+  });
+});

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,19 +25,19 @@ 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.
    *
    * @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);

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

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<number, CodecInfo> = 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) {

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;
   }