diff --git a/mediaengine.go b/mediaengine.go index a386d78a7fb..2c1f9b99a08 100644 --- a/mediaengine.go +++ b/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) } } diff --git a/mediaengine_test.go b/mediaengine_test.go index fa3cc2d8994..bab8b689b68 100644 --- a/mediaengine_test.go +++ b/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) + }) +} diff --git a/rtpcodec.go b/rtpcodec.go index 68beae8d395..a414d510501 100644 --- a/rtpcodec.go +++ b/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 +} diff --git a/rtptransceiver.go b/rtptransceiver.go index 4ba732e8a1e..d707bb00c29 100644 --- a/rtptransceiver.go +++ b/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) } }