Repository: pions/webrtc (go)

Issue:
pc.AddIceCandidate() should take RTCIceCandidateInit
See RTCIceCandidateInit: [w3c definition](https://w3c.github.io/webrtc-pc/#dom-rtcicecandidateinit)

## Issue
Current AddIceCandidate() takes a string, `candidate` component of RTCIceCandidateInit. In the future enhancement, we'd need sdpMLineIndex, sdpMid and usernameFragment fields to correctly pair the new candidates.

## Proposed approach
Handling of these field would require a huge modification to the current implementation of pion.  I'd assume we would like to incrementally enhance the feature. To do so, we should introduce RTCIceCandidateInit object for AddIceCandidate() method first.

To ensure backward compatibility, I'd suggest to chage the argument type from string to interface{}, then inside the function, we would use type switch to handle both string and (new) RTCCandidateInit object. (so that we could introduce this change in 1.x)

In 2.0, we can drop string arg, only accept RTCCandidateInit object.


Pull request:
API: Use ICECandidateInit in AddICECandidate
Resolves #358

Patch:
diff --git a/icecandidateinit.go b/icecandidateinit.go
new file mode 100644
index 00000000..64cfa89d
--- /dev/null
+++ b/icecandidateinit.go
@@ -0,0 +1,9 @@
+package webrtc
+
+// ICECandidateInit is used to serialize ice candidates
+type ICECandidateInit struct {
+	Candidate        string  `json:"candidate"`
+	SDPMid           *string `json:"sdpMid,omitempty"`
+	SDPMLineIndex    *uint16 `json:"sdpMLineIndex,omitempty"`
+	UsernameFragment string  `json:"usernameFragment"`
+}
diff --git a/peerconnection.go b/peerconnection.go
index 8ea327cb..eb2f77da 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -1030,26 +1030,24 @@ func (pc *PeerConnection) RemoteDescription() *SessionDescription {
 
 // AddICECandidate accepts an ICE candidate string and adds it
 // to the existing set of candidates
-func (pc *PeerConnection) AddICECandidate(s string) error {
-	// TODO: AddICECandidate should take ICECandidateInit
+func (pc *PeerConnection) AddICECandidate(candidate ICECandidateInit) error {
 	if pc.RemoteDescription() == nil {
 		return &rtcerr.InvalidStateError{Err: ErrNoRemoteDescription}
 	}
 
-	s = strings.TrimPrefix(s, "candidate:")
-
-	attribute := sdp.NewAttribute("candidate", s)
+	candidateValue := strings.TrimPrefix(candidate.Candidate, "candidate:")
+	attribute := sdp.NewAttribute("candidate", candidateValue)
 	sdpCandidate, err := attribute.ToICECandidate()
 	if err != nil {
 		return err
 	}
 
-	candidate, err := newICECandidateFromSDP(sdpCandidate)
+	iceCandidate, err := newICECandidateFromSDP(sdpCandidate)
 	if err != nil {
 		return err
 	}
 
-	return pc.iceTransport.AddRemoteCandidate(candidate)
+	return pc.iceTransport.AddRemoteCandidate(iceCandidate)
 }
 
 // ICEConnectionState returns the ICE connection state of the


Test patch:
diff --git a/icecandidateinit_test.go b/icecandidateinit_test.go
new file mode 100644
index 00000000..c9c98b60
--- /dev/null
+++ b/icecandidateinit_test.go
@@ -0,0 +1,53 @@
+package webrtc
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestICECandidateInit_Serialization(t *testing.T) {
+	tt := []struct {
+		candidate  ICECandidateInit
+		serialized string
+	}{
+		{ICECandidateInit{
+			Candidate:        "candidate:abc123",
+			SDPMid:           refString("0"),
+			SDPMLineIndex:    refUint16(0),
+			UsernameFragment: "def",
+		}, `{"candidate":"candidate:abc123","sdpMid":"0","sdpMLineIndex":0,"usernameFragment":"def"}`},
+		{ICECandidateInit{
+			Candidate:        "candidate:abc123",
+			UsernameFragment: "def",
+		}, `{"candidate":"candidate:abc123","usernameFragment":"def"}`},
+	}
+
+	for i, tc := range tt {
+		b, err := json.Marshal(tc.candidate)
+		if err != nil {
+			t.Errorf("Failed to marshal %d: %v", i, err)
+		}
+		actualSerialized := string(b)
+		if actualSerialized != tc.serialized {
+			t.Errorf("%d expected %s got %s", i, tc.serialized, actualSerialized)
+		}
+
+		var actual ICECandidateInit
+		err = json.Unmarshal(b, &actual)
+		if err != nil {
+			t.Errorf("Failed to unmarshal %d: %v", i, err)
+		}
+
+		assert.Equal(t, tc.candidate, actual, "should match")
+	}
+}
+
+func refString(s string) *string {
+	return &s
+}
+
+func refUint16(i uint16) *uint16 {
+	return &i
+}