Use intersection of codecs to generate rtcp-fb

Update MediaEngine codec creation to take into account remote
and local rtcp-fb. Before we would incorrectly always take
the remote rtcp-fb and ignore local.

Resolves #2943
Resolves #2944
Resolves #1968

Author: Sean-Der

mediaengine.go

@@ -406,7 +406,7 @@ func (m *MediaEngine) collectStats(collector *statsReportCollector) {
 }
 
 // Look up a codec and enable if it exists
-func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCodecType, exactMatches, partialMatches []RTPCodecParameters) (codecMatchType, error) {
+func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCodecType, exactMatches, partialMatches []RTPCodecParameters) (RTPCodecParameters, codecMatchType, error) {
 	codecs := m.videoCodecs
 	if typ == RTPCodecTypeAudio {
 		codecs = m.audioCodecs
@@ -416,7 +416,7 @@ func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCo
 	if apt, hasApt := remoteFmtp.Parameter("apt"); hasApt {
 		payloadType, err := strconv.ParseUint(apt, 10, 8)
 		if err != nil {
-			return codecMatchNone, err
+			return RTPCodecParameters{}, codecMatchNone, err
 		}
 
 		aptMatch := codecMatchNone
@@ -440,7 +440,7 @@ func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCo
 		}
 
 		if aptMatch == codecMatchNone {
-			return codecMatchNone, nil // not an error, we just ignore this codec we don't support
+			return RTPCodecParameters{}, codecMatchNone, nil // not an error, we just ignore this codec we don't support
 		}
 
 		// replace the apt value with the original codec's payload type
@@ -450,15 +450,15 @@ func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCo
 		}
 
 		// if apt's media codec is partial match, then apt codec must be partial match too
-		_, matchType := codecParametersFuzzySearch(toMatchCodec, codecs)
+		localCodec, matchType := codecParametersFuzzySearch(toMatchCodec, codecs)
 		if matchType == codecMatchExact && aptMatch == codecMatchPartial {
 			matchType = codecMatchPartial
 		}
-		return matchType, nil
+		return localCodec, matchType, nil
 	}
 
-	_, matchType := codecParametersFuzzySearch(remoteCodec, codecs)
-	return matchType, nil
+	localCodec, matchType := codecParametersFuzzySearch(remoteCodec, codecs)
+	return localCodec, matchType, nil
 }
 
 // Update header extensions from a remote media section
@@ -560,16 +560,18 @@ func (m *MediaEngine) updateFromRemoteDescription(desc sdp.SessionDescription) e
 		exactMatches := make([]RTPCodecParameters, 0, len(codecs))
 		partialMatches := make([]RTPCodecParameters, 0, len(codecs))
 
-		for _, codec := range codecs {
-			matchType, mErr := m.matchRemoteCodec(codec, typ, exactMatches, partialMatches)
+		for _, remoteCodec := range codecs {
+			localCodec, matchType, mErr := m.matchRemoteCodec(remoteCodec, typ, exactMatches, partialMatches)
 			if mErr != nil {
 				return mErr
 			}
 
+			remoteCodec.RTCPFeedback = rtcpFeedbackIntersection(localCodec.RTCPFeedback, remoteCodec.RTCPFeedback)
+
 			if matchType == codecMatchExact {
-				exactMatches = append(exactMatches, codec)
+				exactMatches = append(exactMatches, remoteCodec)
 			} else if matchType == codecMatchPartial {
-				partialMatches = append(partialMatches, codec)
+				partialMatches = append(partialMatches, remoteCodec)
 			}
 		}
 

mediaengine_test.go

@@ -722,3 +722,74 @@ a=fmtp:127 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001
 		})
 	}
 }
+
+// rtcp-fb should be an intersection of local and remote
+func TestRTCPFeedbackHandling(t *testing.T) {
+	const offerSdp = `
+v=0
+o=- 8448668841136641781 4 IN IP4 127.0.0.1
+s=-
+t=0 0
+a=group:BUNDLE 0
+a=extmap-allow-mixed
+a=msid-semantic: WMS 4beea6b0-cf95-449c-a1ec-78e16b247426
+m=video 9 UDP/TLS/RTP/SAVPF 96
+c=IN IP4 0.0.0.0
+a=rtcp:9 IN IP4 0.0.0.0
+a=ice-ufrag:1/MvHwjAyVf27aLu
+a=ice-pwd:3dBU7cFOBl120v33cynDvN1E
+a=ice-options:google-ice
+a=fingerprint:sha-256 75:74:5A:A6:A4:E5:52:F4:A7:67:4C:01:C7:EE:91:3F:21:3D:A2:E3:53:7B:6F:30:86:F2:30:AA:65:FB:04:24
+a=setup:actpass
+a=mid:0
+a=sendrecv
+a=rtpmap:96 VP8/90000
+a=rtcp-fb:96 goog-remb
+a=rtcp-fb:96 nack
+`
+
+	runTest := func(createTransceiver bool, t *testing.T) {
+		m := &MediaEngine{}
+		assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{MimeType: MimeTypeVP8, ClockRate: 90000, RTCPFeedback: []RTCPFeedback{
+				{Type: TypeRTCPFBTransportCC},
+				{Type: TypeRTCPFBNACK},
+			}},
+			PayloadType: 96,
+		}, RTPCodecTypeVideo))
+
+		peerConnection, err := NewAPI(WithMediaEngine(m)).NewPeerConnection(Configuration{})
+		assert.NoError(t, err)
+
+		if createTransceiver {
+			_, err = peerConnection.AddTransceiverFromKind(RTPCodecTypeVideo)
+			assert.NoError(t, err)
+		}
+
+		assert.NoError(t, peerConnection.SetRemoteDescription(SessionDescription{
+			Type: SDPTypeOffer,
+			SDP:  offerSdp,
+		},
+		))
+
+		answer, err := peerConnection.CreateAnswer(nil)
+		assert.NoError(t, err)
+
+		// Both clients support
+		assert.True(t, strings.Contains(answer.SDP, "a=rtcp-fb:96 nack"))
+
+		// Only one client supports
+		assert.False(t, strings.Contains(answer.SDP, "a=rtcp-fb:96 goog-remb"))
+		assert.False(t, strings.Contains(answer.SDP, "a=rtcp-fb:96 transport-cc"))
+
+		assert.NoError(t, peerConnection.Close())
+	}
+
+	t.Run("recvonly", func(t *testing.T) {
+		runTest(false, t)
+	})
+
+	t.Run("sendrecv", func(t *testing.T) {
+		runTest(true, t)
+	})
+}

rtpcodec.go

@@ -136,3 +136,16 @@ func findRTXPayloadType(needle PayloadType, haystack []RTPCodecParameters) Paylo
 
 	return PayloadType(0)
 }
+
+func rtcpFeedbackIntersection(a, b []RTCPFeedback) (out []RTCPFeedback) {
+	for _, aFeedback := range a {
+		for _, bFeeback := range b {
+			if aFeedback.Type == bFeeback.Type && aFeedback.Parameter == bFeeback.Parameter {
+				out = append(out, aFeedback)
+				break
+			}
+		}
+	}
+
+	return
+}

rtptransceiver.go

@@ -78,6 +78,7 @@ func (t *RTPTransceiver) getCodecs() []RTPCodecParameters {
 			if codec.PayloadType == 0 {
 				codec.PayloadType = c.PayloadType
 			}
+			codec.RTCPFeedback = rtcpFeedbackIntersection(codec.RTCPFeedback, c.RTCPFeedback)
 			filteredCodecs = append(filteredCodecs, codec)
 		}
 	}