Improved h264 fmtp line parsing

Implements h264 fmtp parsing based on RFC 6184 Section 8.2.2

fmtp.go

@@ -1,37 +0,0 @@
-package webrtc
-
-import (
-	"strings"
-)
-
-type fmtp map[string]string
-
-// parseFmtp parses fmtp string.
-func parseFmtp(line string) fmtp {
-	f := fmtp{}
-	for _, p := range strings.Split(line, ";") {
-		pp := strings.SplitN(strings.TrimSpace(p), "=", 2)
-		key := strings.ToLower(pp[0])
-		var value string
-		if len(pp) > 1 {
-			value = pp[1]
-		}
-		f[key] = value
-	}
-	return f
-}
-
-// fmtpConsist checks that two FMTP parameters are not inconsistent.
-func fmtpConsist(a, b fmtp) bool {
-	for k, v := range a {
-		if vb, ok := b[k]; ok && !strings.EqualFold(vb, v) {
-			return false
-		}
-	}
-	for k, v := range b {
-		if va, ok := a[k]; ok && !strings.EqualFold(va, v) {
-			return false
-		}
-	}
-	return true
-}

internal/fmtp/fmtp.go

@@ -0,0 +1,92 @@
+// Package fmtp implements per codec parsing of fmtp lines
+package fmtp
+
+import (
+	"strings"
+)
+
+// FMTP interface for implementing custom
+// FMTP parsers based on MimeType
+type FMTP interface {
+	// MimeType returns the MimeType associated with
+	// the fmtp
+	MimeType() string
+	// Match compares two fmtp descriptions for
+	// compatibility based on the MimeType
+	Match(f FMTP) bool
+	// Parameter returns a value for the associated key
+	// if contained in the parsed fmtp string
+	Parameter(key string) (string, bool)
+}
+
+// Parse parses an fmtp string based on the MimeType
+func Parse(mimetype, line string) FMTP {
+	var f FMTP
+
+	parameters := make(map[string]string)
+
+	for _, p := range strings.Split(line, ";") {
+		pp := strings.SplitN(strings.TrimSpace(p), "=", 2)
+		key := strings.ToLower(pp[0])
+		var value string
+		if len(pp) > 1 {
+			value = pp[1]
+		}
+		parameters[key] = value
+	}
+
+	switch {
+	case strings.EqualFold(mimetype, "video/h264"):
+		f = &h264FMTP{
+			parameters: parameters,
+		}
+	default:
+		f = &genericFMTP{
+			mimeType:   mimetype,
+			parameters: parameters,
+		}
+	}
+
+	return f
+}
+
+type genericFMTP struct {
+	mimeType   string
+	parameters map[string]string
+}
+
+func (g *genericFMTP) MimeType() string {
+	return g.mimeType
+}
+
+// Match returns true if g and b are compatible fmtp descriptions
+// The generic implementation is used for MimeTypes that are not defined
+func (g *genericFMTP) Match(b FMTP) bool {
+	c, ok := b.(*genericFMTP)
+	if !ok {
+		return false
+	}
+
+	if g.mimeType != c.MimeType() {
+		return false
+	}
+
+	for k, v := range g.parameters {
+		if vb, ok := c.parameters[k]; ok && !strings.EqualFold(vb, v) {
+			return false
+		}
+	}
+
+	for k, v := range c.parameters {
+		if va, ok := g.parameters[k]; ok && !strings.EqualFold(va, v) {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (g *genericFMTP) Parameter(key string) (string, bool) {
+	v, ok := g.parameters[key]
+	return v, ok
+}

internal/fmtp/fmtp_test.go

@@ -1,54 +1,70 @@
-package webrtc
+package fmtp
 
 import (
 	"reflect"
 	"testing"
 )
 
-func TestParseFmtp(t *testing.T) {
+func TestGenericParseFmtp(t *testing.T) {
 	testCases := map[string]struct {
 		input    string
-		expected fmtp
+		expected FMTP
 	}{
 		"OneParam": {
 			input: "key-name=value",
-			expected: fmtp{
-				"key-name": "value",
+			expected: &genericFMTP{
+				mimeType: "generic",
+				parameters: map[string]string{
+					"key-name": "value",
+				},
 			},
 		},
 		"OneParamWithWhiteSpeces": {
 			input: "\tkey-name=value ",
-			expected: fmtp{
-				"key-name": "value",
+			expected: &genericFMTP{
+				mimeType: "generic",
+				parameters: map[string]string{
+					"key-name": "value",
+				},
 			},
 		},
 		"TwoParams": {
 			input: "key-name=value;key2=value2",
-			expected: fmtp{
-				"key-name": "value",
-				"key2":     "value2",
+			expected: &genericFMTP{
+				mimeType: "generic",
+				parameters: map[string]string{
+					"key-name": "value",
+					"key2":     "value2",
+				},
 			},
 		},
 		"TwoParamsWithWhiteSpeces": {
 			input: "key-name=value;  \n\tkey2=value2 ",
-			expected: fmtp{
-				"key-name": "value",
-				"key2":     "value2",
+			expected: &genericFMTP{
+				mimeType: "generic",
+				parameters: map[string]string{
+					"key-name": "value",
+					"key2":     "value2",
+				},
 			},
 		},
 	}
 	for name, testCase := range testCases {
 		testCase := testCase
 		t.Run(name, func(t *testing.T) {
-			f := parseFmtp(testCase.input)
+			f := Parse("generic", testCase.input)
 			if !reflect.DeepEqual(testCase.expected, f) {
 				t.Errorf("Expected Fmtp params: %v, got: %v", testCase.expected, f)
 			}
+
+			if f.MimeType() != "generic" {
+				t.Errorf("Expected MimeType of generic, got: %s", f.MimeType())
+			}
 		})
 	}
 }
 
-func TestFmtpConsist(t *testing.T) {
+func TestGenericFmtpCompare(t *testing.T) {
 	consistString := map[bool]string{true: "consist", false: "inconsist"}
 
 	testCases := map[string]struct {
@@ -89,7 +105,18 @@ func TestFmtpConsist(t *testing.T) {
 	for name, testCase := range testCases {
 		testCase := testCase
 		check := func(t *testing.T, a, b string) {
-			c := fmtpConsist(parseFmtp(a), parseFmtp(b))
+			aa := Parse("", a)
+			bb := Parse("", b)
+			c := aa.Match(bb)
+			if c != testCase.consist {
+				t.Errorf(
+					"'%s' and '%s' are expected to be %s, but treated as %s",
+					a, b, consistString[testCase.consist], consistString[c],
+				)
+			}
+
+			// test reverse case here
+			c = bb.Match(aa)
 			if c != testCase.consist {
 				t.Errorf(
 					"'%s' and '%s' are expected to be %s, but treated as %s",
@@ -100,8 +127,5 @@ func TestFmtpConsist(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			check(t, testCase.a, testCase.b)
 		})
-		t.Run(name+"_Reversed", func(t *testing.T) {
-			check(t, testCase.b, testCase.a)
-		})
 	}
 }

internal/fmtp/h264.go

@@ -0,0 +1,80 @@
+package fmtp
+
+import (
+	"encoding/hex"
+)
+
+func profileLevelIDMatches(a, b string) bool {
+	aa, err := hex.DecodeString(a)
+	if err != nil || len(aa) < 2 {
+		return false
+	}
+	bb, err := hex.DecodeString(b)
+	if err != nil || len(bb) < 2 {
+		return false
+	}
+	return aa[0] == bb[0] && aa[1] == bb[1]
+}
+
+type h264FMTP struct {
+	parameters map[string]string
+}
+
+func (h *h264FMTP) MimeType() string {
+	return "video/h264"
+}
+
+// Match returns true if h and b are compatible fmtp descriptions
+// Based on RFC6184 Section 8.2.2:
+//   The parameters identifying a media format configuration for H.264
+//   are profile-level-id and packetization-mode.  These media format
+//   configuration parameters (except for the level part of profile-
+//   level-id) MUST be used symmetrically; that is, the answerer MUST
+//   either maintain all configuration parameters or remove the media
+//   format (payload type) completely if one or more of the parameter
+//   values are not supported.
+//     Informative note: The requirement for symmetric use does not
+//     apply for the level part of profile-level-id and does not apply
+//     for the other stream properties and capability parameters.
+func (h *h264FMTP) Match(b FMTP) bool {
+	c, ok := b.(*h264FMTP)
+	if !ok {
+		return false
+	}
+
+	// test packetization-mode
+	hpmode, hok := h.parameters["packetization-mode"]
+	if !hok {
+		return false
+	}
+	cpmode, cok := c.parameters["packetization-mode"]
+	if !cok {
+		return false
+	}
+
+	if hpmode != cpmode {
+		return false
+	}
+
+	// test profile-level-id
+	hplid, hok := h.parameters["profile-level-id"]
+	if !hok {
+		return false
+	}
+
+	cplid, cok := c.parameters["profile-level-id"]
+	if !cok {
+		return false
+	}
+
+	if !profileLevelIDMatches(hplid, cplid) {
+		return false
+	}
+
+	return true
+}
+
+func (h *h264FMTP) Parameter(key string) (string, bool) {
+	v, ok := h.parameters[key]
+	return v, ok
+}

internal/fmtp/h264_test.go

@@ -0,0 +1,142 @@
+package fmtp
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestH264FMTPParse(t *testing.T) {
+	testCases := map[string]struct {
+		input    string
+		expected FMTP
+	}{
+		"OneParam": {
+			input: "key-name=value",
+			expected: &h264FMTP{
+				parameters: map[string]string{
+					"key-name": "value",
+				},
+			},
+		},
+		"OneParamWithWhiteSpeces": {
+			input: "\tkey-name=value ",
+			expected: &h264FMTP{
+				parameters: map[string]string{
+					"key-name": "value",
+				},
+			},
+		},
+		"TwoParams": {
+			input: "key-name=value;key2=value2",
+			expected: &h264FMTP{
+				parameters: map[string]string{
+					"key-name": "value",
+					"key2":     "value2",
+				},
+			},
+		},
+		"TwoParamsWithWhiteSpeces": {
+			input: "key-name=value;  \n\tkey2=value2 ",
+			expected: &h264FMTP{
+				parameters: map[string]string{
+					"key-name": "value",
+					"key2":     "value2",
+				},
+			},
+		},
+	}
+	for name, testCase := range testCases {
+		testCase := testCase
+		t.Run(name, func(t *testing.T) {
+			f := Parse("video/h264", testCase.input)
+			if !reflect.DeepEqual(testCase.expected, f) {
+				t.Errorf("Expected Fmtp params: %v, got: %v", testCase.expected, f)
+			}
+
+			if f.MimeType() != "video/h264" {
+				t.Errorf("Expected MimeType of video/h264, got: %s", f.MimeType())
+			}
+		})
+	}
+}
+
+func TestH264FMTPCompare(t *testing.T) {
+	consistString := map[bool]string{true: "consist", false: "inconsist"}
+
+	testCases := map[string]struct {
+		a, b    string
+		consist bool
+	}{
+		"Equal": {
+			a:       "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f",
+			b:       "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f",
+			consist: true,
+		},
+		"EqualWithWhitespaceVariants": {
+			a:       "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f",
+			b:       "  level-asymmetry-allowed=1;  \npacketization-mode=1;\t\nprofile-level-id=42e01f",
+			consist: true,
+		},
+		"EqualWithCase": {
+			a:       "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f",
+			b:       "level-asymmetry-allowed=1;packetization-mode=1;PROFILE-LEVEL-ID=42e01f",
+			consist: true,
+		},
+		"OneHasExtraParam": {
+			a:       "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f",
+			b:       "packetization-mode=1;profile-level-id=42e01f",
+			consist: true,
+		},
+		"DifferentProfileLevelIDVersions": {
+			a:       "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f",
+			b:       "packetization-mode=1;profile-level-id=42e029",
+			consist: true,
+		},
+		"Inconsistent": {
+			a:       "packetization-mode=1;profile-level-id=42e029",
+			b:       "packetization-mode=0;profile-level-id=42e029",
+			consist: false,
+		},
+		"Inconsistent_MissingPacketizationMode": {
+			a:       "packetization-mode=1;profile-level-id=42e029",
+			b:       "profile-level-id=42e029",
+			consist: false,
+		},
+		"Inconsistent_MissingProfileLevelID": {
+			a:       "packetization-mode=1;profile-level-id=42e029",
+			b:       "packetization-mode=1",
+			consist: false,
+		},
+		"Inconsistent_InvalidProfileLevelID": {
+			a:       "packetization-mode=1;profile-level-id=42e029",
+			b:       "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=41e029",
+			consist: false,
+		},
+	}
+	for name, testCase := range testCases {
+		testCase := testCase
+		check := func(t *testing.T, a, b string) {
+			aa := Parse("video/h264", a)
+			bb := Parse("video/h264", b)
+			c := aa.Match(bb)
+			if c != testCase.consist {
+				t.Errorf(
+					"'%s' and '%s' are expected to be %s, but treated as %s",
+					a, b, consistString[testCase.consist], consistString[c],
+				)
+			}
+
+			// test reverse case here
+			c = bb.Match(aa)
+			if c != testCase.consist {
+				t.Errorf(
+					"'%s' and '%s' are expected to be %s, but treated as %s",
+					a, b, consistString[testCase.consist], consistString[c],
+				)
+			}
+		}
+		t.Run(name, func(t *testing.T) {
+			check(t, testCase.a, testCase.b)
+		})
+	}
+}

mediaengine.go

@@ -12,6 +12,7 @@ import (
 	"github.com/pion/rtp/v2"
 	"github.com/pion/rtp/v2/codecs"
 	"github.com/pion/sdp/v3"
+	"github.com/pion/webrtc/v3/internal/fmtp"
 )
 
 const (
@@ -372,8 +373,8 @@ func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCo
 		codecs = m.audioCodecs
 	}
 
-	remoteFmtp := parseFmtp(remoteCodec.RTPCodecCapability.SDPFmtpLine)
-	if apt, hasApt := remoteFmtp["apt"]; hasApt {
+	remoteFmtp := fmtp.Parse(remoteCodec.RTPCodecCapability.MimeType, remoteCodec.RTPCodecCapability.SDPFmtpLine)
+	if apt, hasApt := remoteFmtp.Parameter("apt"); hasApt {
 		payloadType, err := strconv.Atoi(apt)
 		if err != nil {
 			return codecMatchNone, err

rtpcodec.go

@@ -2,6 +2,8 @@ package webrtc
 
 import (
 	"strings"
+
+	"github.com/pion/webrtc/v3/internal/fmtp"
 )
 
 // RTPCodecType determines the type of a codec
@@ -97,12 +99,12 @@ const (
 // Used for lookup up a codec in an existing list to find a match
 // Returns codecMatchExact, codecMatchPartial, or codecMatchNone
 func codecParametersFuzzySearch(needle RTPCodecParameters, haystack []RTPCodecParameters) (RTPCodecParameters, codecMatchType) {
-	needleFmtp := parseFmtp(needle.RTPCodecCapability.SDPFmtpLine)
+	needleFmtp := fmtp.Parse(needle.RTPCodecCapability.MimeType, needle.RTPCodecCapability.SDPFmtpLine)
 
 	// First attempt to match on MimeType + SDPFmtpLine
 	for _, c := range haystack {
-		if strings.EqualFold(c.RTPCodecCapability.MimeType, needle.RTPCodecCapability.MimeType) &&
-			fmtpConsist(needleFmtp, parseFmtp(c.RTPCodecCapability.SDPFmtpLine)) {
+		cfmtp := fmtp.Parse(c.RTPCodecCapability.MimeType, c.RTPCodecCapability.SDPFmtpLine)
+		if needleFmtp.Match(cfmtp) {
 			return c, codecMatchExact
 		}
 	}