Repository: pion/webrtc
Pull request #630 (2019-04-19 03:59:14)

Allow remote Sendrecv to be used with Recvonly
Before our RTPTransceiver logic would only allow Sendrecv to
match with another Sendrecv. Instead allow a SendRecv to be used
with a local Recvonly



Patch:
diff --git a/README.md b/README.md
index 9f8f5caf..d1fe3e76 100644
--- a/README.md
+++ b/README.md
@@ -106,6 +106,7 @@ Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contribu
 * [Žiga Željko](https://github.com/zigazeljko)
 * [Simonacca Fotokite](https://github.com/simonacca-fotokite)
 * [Marouane](https://github.com/nindolabs) *Fix Offer bundle generation*
+* [Christopher Fry](https://github.com/christopherfry)
 
 ### License
 MIT License - see [LICENSE](LICENSE) for full text
diff --git a/peerconnection.go b/peerconnection.go
index 0052048a..9c80ac7b 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -26,6 +26,10 @@ import (
 	"github.com/pion/webrtc/v2/pkg/rtcerr"
 )
 
+const (
+	newTransceiverMid = "NEW"
+)
+
 // PeerConnection represents a WebRTC connection that establishes a
 // peer-to-peer communications with another PeerConnection instance in a
 // browser, or to another endpoint implementing the required protocols.
@@ -596,6 +600,40 @@ func (pc *PeerConnection) getMidValue(media *sdp.MediaDescription) string {
 	return ""
 }
 
+// Given a direction+type pluck a transceiver from the passed list
+// if no entry satisfies the requested type+direction return a inactive Transceiver
+func satisfyTypeAndDirection(remoteKind RTPCodecType, remoteDirection RTPTransceiverDirection, localTransceivers []*RTPTransceiver) (*RTPTransceiver, []*RTPTransceiver) {
+	// Get direction order from most preferred to least
+	getPreferredDirections := func() []RTPTransceiverDirection {
+		switch remoteDirection {
+		case RTPTransceiverDirectionSendrecv:
+			return []RTPTransceiverDirection{RTPTransceiverDirectionRecvonly, RTPTransceiverDirectionSendrecv}
+		case RTPTransceiverDirectionSendonly:
+			return []RTPTransceiverDirection{RTPTransceiverDirectionRecvonly, RTPTransceiverDirectionSendrecv}
+		case RTPTransceiverDirectionRecvonly:
+			return []RTPTransceiverDirection{RTPTransceiverDirectionSendonly, RTPTransceiverDirectionSendrecv}
+		}
+		return []RTPTransceiverDirection{}
+	}
+
+	for _, possibleDirection := range getPreferredDirections() {
+		for i := range localTransceivers {
+			t := localTransceivers[i]
+			if t.kind != remoteKind || possibleDirection != t.Direction {
+				continue
+			}
+
+			return t, append(localTransceivers[:i], localTransceivers[i+1:]...)
+		}
+	}
+
+	return &RTPTransceiver{
+		Mid:       newTransceiverMid,
+		kind:      remoteKind,
+		Direction: RTPTransceiverDirectionInactive,
+	}, localTransceivers
+}
+
 func (pc *PeerConnection) addAnswerMediaTransceivers(d *sdp.SessionDescription) (*sdp.SessionDescription, error) {
 	iceParams, err := pc.iceGatherer.GetLocalParameters()
 	if err != nil {
@@ -612,34 +650,10 @@ func (pc *PeerConnection) addAnswerMediaTransceivers(d *sdp.SessionDescription)
 		bundleValue += " " + midValue
 	}
 
-	newTransceiverMid := "NEW"
+	var t *RTPTransceiver
 	localTransceivers := append([]*RTPTransceiver{}, pc.GetTransceivers()...)
-	satisfyPeerMedia := func(kind RTPCodecType, direction RTPTransceiverDirection) *RTPTransceiver {
-		for i := range localTransceivers {
-			t := localTransceivers[i]
-
-			switch {
-			case t.kind != kind:
-				continue
-			case direction == RTPTransceiverDirectionSendrecv && t.Direction != RTPTransceiverDirectionSendrecv:
-				continue
-			case direction != RTPTransceiverDirectionSendrecv && direction == t.Direction:
-				continue
-			case direction == RTPTransceiverDirectionInactive:
-				continue
-			}
-			localTransceivers = append(localTransceivers[:i], localTransceivers[i+1:]...)
-			return t
-		}
-
-		return &RTPTransceiver{
-			Mid:       newTransceiverMid,
-			kind:      kind,
-			Direction: RTPTransceiverDirectionInactive,
-		}
-	}
-
 	detectedPlanB := pc.descriptionIsPlanB(pc.RemoteDescription())
+
 	for _, media := range pc.RemoteDescription().parsed.MediaDescriptions {
 		midValue := pc.getMidValue(media)
 		if midValue == "" {
@@ -658,7 +672,7 @@ func (pc *PeerConnection) addAnswerMediaTransceivers(d *sdp.SessionDescription)
 			continue
 		}
 
-		t := satisfyPeerMedia(kind, direction)
+		t, localTransceivers = satisfyTypeAndDirection(kind, direction, localTransceivers)
 		mediaTransceivers := []*RTPTransceiver{t}
 		switch pc.configuration.SDPSemantics {
 		case SDPSemanticsUnifiedPlanWithFallback:
@@ -676,7 +690,7 @@ func (pc *PeerConnection) addAnswerMediaTransceivers(d *sdp.SessionDescription)
 			// media entry with all matching local transceivers
 			for {
 				// keep going until we can't get any more
-				t := satisfyPeerMedia(kind, direction)
+				t, localTransceivers = satisfyTypeAndDirection(kind, direction, localTransceivers)
 				if t.Mid == newTransceiverMid {
 					break
 				}


Test patch:
diff --git a/peerconnection_close_test.go b/peerconnection_close_test.go
index 1f15b2bf..7a8baa3e 100644
--- a/peerconnection_close_test.go
+++ b/peerconnection_close_test.go
@@ -61,7 +61,7 @@ func TestPeerConnection_Close(t *testing.T) {
 // Assert that a PeerConnection that is shutdown before ICE starts doesn't leak
 func TestPeerConnection_Close_PreICE(t *testing.T) {
 	// Limit runtime in case of deadlocks
-	lim := test.TimeOut(time.Second * 10)
+	lim := test.TimeOut(time.Second * 30)
 	defer lim.Stop()
 
 	report := test.CheckRoutines(t)
diff --git a/peerconnection_go_test.go b/peerconnection_go_test.go
index 38c84ddf..7902e448 100644
--- a/peerconnection_go_test.go
+++ b/peerconnection_go_test.go
@@ -7,6 +7,7 @@ import (
 	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/x509"
+	"fmt"
 	"math/big"
 	"reflect"
 	"testing"
@@ -375,3 +376,91 @@ func TestPeerConnection_AnswerWithoutOffer(t *testing.T) {
 		t.Errorf("CreateAnswer without RemoteDescription: got error: %v", err)
 	}
 }
+
+func TestPeerConnection_satisfyTypeAndDirection(t *testing.T) {
+	createTransceiver := func(kind RTPCodecType, direction RTPTransceiverDirection) *RTPTransceiver {
+		return &RTPTransceiver{Mid: newTransceiverMid, kind: kind, Direction: direction}
+	}
+
+	for _, test := range []struct {
+		name string
+
+		kinds      []RTPCodecType
+		directions []RTPTransceiverDirection
+
+		localTransceivers []*RTPTransceiver
+		want              []*RTPTransceiver
+	}{
+		{
+			"Audio and Video Transceivers can not satifsy each other",
+			[]RTPCodecType{RTPCodecTypeVideo},
+			[]RTPTransceiverDirection{RTPTransceiverDirectionSendrecv},
+			[]*RTPTransceiver{createTransceiver(RTPCodecTypeAudio, RTPTransceiverDirectionSendrecv)},
+			[]*RTPTransceiver{createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionInactive)},
+		},
+		{
+			"No local Transceivers, every remote should get an inactive",
+			[]RTPCodecType{RTPCodecTypeVideo, RTPCodecTypeAudio, RTPCodecTypeVideo, RTPCodecTypeVideo},
+			[]RTPTransceiverDirection{RTPTransceiverDirectionSendrecv, RTPTransceiverDirectionRecvonly, RTPTransceiverDirectionSendonly, RTPTransceiverDirectionInactive},
+
+			[]*RTPTransceiver{},
+
+			[]*RTPTransceiver{
+				createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionInactive),
+				createTransceiver(RTPCodecTypeAudio, RTPTransceiverDirectionInactive),
+				createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionInactive),
+				createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionInactive),
+			},
+		},
+		{
+			"Local Recv can satisfy remote SendRecv",
+			[]RTPCodecType{RTPCodecTypeVideo},
+			[]RTPTransceiverDirection{RTPTransceiverDirectionSendrecv},
+
+			[]*RTPTransceiver{createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionRecvonly)},
+
+			[]*RTPTransceiver{createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionRecvonly)},
+		},
+		{
+			"Don't satisify a Sendonly with a SendRecv, later SendRecv will be marked as Inactive",
+			[]RTPCodecType{RTPCodecTypeVideo, RTPCodecTypeVideo},
+			[]RTPTransceiverDirection{RTPTransceiverDirectionSendonly, RTPTransceiverDirectionSendrecv},
+
+			[]*RTPTransceiver{
+				createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionSendrecv),
+				createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionRecvonly),
+			},
+
+			[]*RTPTransceiver{
+				createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionRecvonly),
+				createTransceiver(RTPCodecTypeVideo, RTPTransceiverDirectionSendrecv),
+			},
+		},
+	} {
+		if len(test.kinds) != len(test.directions) {
+			t.Fatal("Kinds and Directions must be the same length")
+		}
+
+		got := []*RTPTransceiver{}
+		for i := range test.kinds {
+			res, filteredLocalTransceivers := satisfyTypeAndDirection(test.kinds[i], test.directions[i], test.localTransceivers)
+
+			got = append(got, res)
+			test.localTransceivers = filteredLocalTransceivers
+		}
+
+		if !reflect.DeepEqual(got, test.want) {
+			gotStr := ""
+			for _, t := range got {
+				gotStr += fmt.Sprintf("%+v\n", t)
+			}
+
+			wantStr := ""
+			for _, t := range test.want {
+				wantStr += fmt.Sprintf("%+v\n", t)
+			}
+			t.Errorf("satisfyTypeAndDirection %q: \ngot\n%s \nwant\n%s", test.name, gotStr, wantStr)
+		}
+	}
+
+}