Repository: pions/webrtc (go)

Issue:
CreateOffer and CreateAnswer shouldn't call setLocalDescription 
Currently these two methods incorrectly call `setLocalDescription` causing us to deviate from the spec.

This is going to cause user breakage, and we will have to figure out a way a nice way to notify users.

Pull request:
Stop Create(Offer/Answer) from setting localDesc
This deviates from the WebRTC spec, so we need to fix it. This is a
massively breaking change, so we need to figure out the best way to help
users with this.

I also renamed our RTCPeerConnection constructor. The hope is that
people will refer to the examples/backlog and see what changed.

Resolves #309

-----

I added all the devs as a heads up, if you think there is a better way we can handle this I am all ears!

Patch:
diff --git a/api.go b/api.go
index 93ead486..71e9de50 100644
--- a/api.go
+++ b/api.go
@@ -89,8 +89,8 @@ func RegisterDefaultCodecs() {
 
 // PeerConnection API
 
-// New using the default API.
+// NewRTCPeerConnection using the default API.
 // See API.NewRTCPeerConnection for details.
-func New(configuration RTCConfiguration) (*RTCPeerConnection, error) {
+func NewRTCPeerConnection(configuration RTCConfiguration) (*RTCPeerConnection, error) {
 	return defaultAPI.NewRTCPeerConnection(configuration)
 }
diff --git a/examples/data-channels-close/main.go b/examples/data-channels-close/main.go
index f1272483..837c5452 100644
--- a/examples/data-channels-close/main.go
+++ b/examples/data-channels-close/main.go
@@ -27,7 +27,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Set the handler for ICE connection state
@@ -90,10 +90,14 @@ func main() {
 	err = peerConnection.SetRemoteDescription(offer)
 	util.Check(err)
 
-	// Sets the LocalDescription, and starts our UDP listeners
+	// Create answer
 	answer, err := peerConnection.CreateAnswer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(answer)
+	util.Check(err)
+
 	// Output the answer in base64 so we can paste it in browser
 	fmt.Println(util.Encode(answer))
 
diff --git a/examples/data-channels-create/main.go b/examples/data-channels-create/main.go
index ac3d4334..984b8431 100644
--- a/examples/data-channels-create/main.go
+++ b/examples/data-channels-create/main.go
@@ -23,7 +23,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Create a datachannel with label 'data'
@@ -65,6 +65,10 @@ func main() {
 	offer, err := peerConnection.CreateOffer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(offer)
+	util.Check(err)
+
 	// Output the offer in base64 so we can paste it in browser
 	fmt.Println(util.Encode(offer))
 
diff --git a/examples/data-channels-detach-create/main.go b/examples/data-channels-detach-create/main.go
index 4255c47a..16c4f3ab 100644
--- a/examples/data-channels-detach-create/main.go
+++ b/examples/data-channels-detach-create/main.go
@@ -30,7 +30,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Create a datachannel with label 'data'
@@ -62,6 +62,10 @@ func main() {
 	offer, err := peerConnection.CreateOffer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(offer)
+	util.Check(err)
+
 	// Output the offer in base64 so we can paste it in browser
 	fmt.Println(util.Encode(offer))
 
diff --git a/examples/data-channels-detach/main.go b/examples/data-channels-detach/main.go
index 0527eb81..f21e4e6f 100644
--- a/examples/data-channels-detach/main.go
+++ b/examples/data-channels-detach/main.go
@@ -30,7 +30,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Set the handler for ICE connection state
@@ -67,10 +67,14 @@ func main() {
 	err = peerConnection.SetRemoteDescription(offer)
 	util.Check(err)
 
-	// Sets the LocalDescription, and starts our UDP listeners
+	// Create answer
 	answer, err := peerConnection.CreateAnswer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(answer)
+	util.Check(err)
+
 	// Output the answer in base64 so we can paste it in browser
 	fmt.Println(util.Encode(answer))
 
diff --git a/examples/data-channels/main.go b/examples/data-channels/main.go
index b4a39ecf..cbed1856 100644
--- a/examples/data-channels/main.go
+++ b/examples/data-channels/main.go
@@ -23,7 +23,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Set the handler for ICE connection state
@@ -70,10 +70,14 @@ func main() {
 	err = peerConnection.SetRemoteDescription(offer)
 	util.Check(err)
 
-	// Sets the LocalDescription, and starts our UDP listeners
+	// Create an answer
 	answer, err := peerConnection.CreateAnswer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(answer)
+	util.Check(err)
+
 	// Output the answer in base64 so we can paste it in browser
 	fmt.Println(util.Encode(answer))
 
diff --git a/examples/gstreamer-receive/main.go b/examples/gstreamer-receive/main.go
index a62085fc..40fa02cc 100644
--- a/examples/gstreamer-receive/main.go
+++ b/examples/gstreamer-receive/main.go
@@ -28,7 +28,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Set a handler for when a new remote track starts, this handler creates a gstreamer pipeline
@@ -70,10 +70,14 @@ func main() {
 	err = peerConnection.SetRemoteDescription(offer)
 	util.Check(err)
 
-	// Sets the LocalDescription, and starts our UDP listeners
+	// Create an answer
 	answer, err := peerConnection.CreateAnswer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(answer)
+	util.Check(err)
+
 	// Output the answer in base64 so we can paste it in browser
 	fmt.Println(util.Encode(answer))
 
diff --git a/examples/gstreamer-send-offer/main.go b/examples/gstreamer-send-offer/main.go
index 152c3a03..9c917b38 100644
--- a/examples/gstreamer-send-offer/main.go
+++ b/examples/gstreamer-send-offer/main.go
@@ -26,7 +26,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Set the handler for ICE connection state
@@ -51,6 +51,10 @@ func main() {
 	offer, err := peerConnection.CreateOffer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(offer)
+	util.Check(err)
+
 	// Output the offer in base64 so we can paste it in browser
 	fmt.Println(util.Encode(offer))
 
diff --git a/examples/gstreamer-send/main.go b/examples/gstreamer-send/main.go
index 37e67e7c..ebfc02b2 100644
--- a/examples/gstreamer-send/main.go
+++ b/examples/gstreamer-send/main.go
@@ -31,7 +31,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Set the handler for ICE connection state
@@ -60,10 +60,14 @@ func main() {
 	err = peerConnection.SetRemoteDescription(offer)
 	util.Check(err)
 
-	// Sets the LocalDescription, and starts our UDP listeners
+	// Create an answer
 	answer, err := peerConnection.CreateAnswer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(answer)
+	util.Check(err)
+
 	// Output the answer in base64 so we can paste it in browser
 	fmt.Println(util.Encode(answer))
 
diff --git a/examples/janus-gateway/streaming/main.go b/examples/janus-gateway/streaming/main.go
index 313f84cc..c91cb3c9 100644
--- a/examples/janus-gateway/streaming/main.go
+++ b/examples/janus-gateway/streaming/main.go
@@ -49,7 +49,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	peerConnection.OnICEConnectionStateChange(func(connectionState ice.ConnectionState) {
@@ -107,6 +107,9 @@ func main() {
 		answer, err := peerConnection.CreateAnswer(nil)
 		util.Check(err)
 
+		err = peerConnection.SetLocalDescription(answer)
+		util.Check(err)
+
 		// now we start
 		_, err = handle.Message(map[string]interface{}{
 			"request": "start",
diff --git a/examples/janus-gateway/video-room/main.go b/examples/janus-gateway/video-room/main.go
index fa0f806c..c95115f1 100644
--- a/examples/janus-gateway/video-room/main.go
+++ b/examples/janus-gateway/video-room/main.go
@@ -7,7 +7,7 @@ import (
 	janus "github.com/notedit/janus-go"
 	"github.com/pions/webrtc"
 	"github.com/pions/webrtc/examples/util"
-	"github.com/pions/webrtc/examples/util/gstreamer-src"
+	gst "github.com/pions/webrtc/examples/util/gstreamer-src"
 	"github.com/pions/webrtc/pkg/ice"
 )
 
@@ -49,7 +49,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	peerConnection.OnICEConnectionStateChange(func(connectionState ice.ConnectionState) {
@@ -71,6 +71,9 @@ func main() {
 	offer, err := peerConnection.CreateOffer(nil)
 	util.Check(err)
 
+	err = peerConnection.SetLocalDescription(offer)
+	util.Check(err)
+
 	gateway, err := janus.Connect("ws://localhost:8188/janus")
 	util.Check(err)
 
diff --git a/examples/pion-to-pion/answer/main.go b/examples/pion-to-pion/answer/main.go
index 790fde3e..795f70dd 100644
--- a/examples/pion-to-pion/answer/main.go
+++ b/examples/pion-to-pion/answer/main.go
@@ -29,7 +29,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Set the handler for ICE connection state
@@ -77,10 +77,14 @@ func main() {
 	err = peerConnection.SetRemoteDescription(offer)
 	util.Check(err)
 
-	// Sets the LocalDescription, and starts our UDP listeners
+	// Create answer
 	answer, err := peerConnection.CreateAnswer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(answer)
+	util.Check(err)
+
 	// Send the answer
 	answerChan <- answer
 
diff --git a/examples/pion-to-pion/offer/main.go b/examples/pion-to-pion/offer/main.go
index b76e71d5..0e9ede6c 100644
--- a/examples/pion-to-pion/offer/main.go
+++ b/examples/pion-to-pion/offer/main.go
@@ -30,7 +30,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Create a datachannel with label 'data'
@@ -72,6 +72,10 @@ func main() {
 	offer, err := peerConnection.CreateOffer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(offer)
+	util.Check(err)
+
 	// Exchange the offer for the answer
 	answer := mustSignalViaHTTP(offer, *addr)
 
diff --git a/examples/save-to-disk/main.go b/examples/save-to-disk/main.go
index 5c7b201f..4772f1fc 100644
--- a/examples/save-to-disk/main.go
+++ b/examples/save-to-disk/main.go
@@ -29,7 +29,7 @@ func main() {
 	}
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(config)
+	peerConnection, err := webrtc.NewRTCPeerConnection(config)
 	util.Check(err)
 
 	// Set a handler for when a new remote track starts, this handler saves buffers to disk as
@@ -73,10 +73,14 @@ func main() {
 	err = peerConnection.SetRemoteDescription(offer)
 	util.Check(err)
 
-	// Sets the LocalDescription, and starts our UDP listeners
+	// Create answer
 	answer, err := peerConnection.CreateAnswer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(answer)
+	util.Check(err)
+
 	// Output the answer in base64 so we can paste it in browser
 	fmt.Println(util.Encode(answer))
 
diff --git a/examples/sfu/main.go b/examples/sfu/main.go
index feeba121..6398cb45 100644
--- a/examples/sfu/main.go
+++ b/examples/sfu/main.go
@@ -32,7 +32,7 @@ func mustReadStdin(reader *bufio.Reader) string {
 	return rawSd
 }
 
-func mustReadHttp(sdp chan string) string {
+func mustReadHTTP(sdp chan string) string {
 	ret := <-sdp
 	return ret
 }
@@ -58,7 +58,7 @@ func main() {
 	}()
 
 	offer := webrtc.RTCSessionDescription{}
-	util.Decode(mustReadHttp(sdp), &offer)
+	util.Decode(mustReadHTTP(sdp), &offer)
 	fmt.Println("")
 
 	/* Everything below is the pion-WebRTC API, thanks for using it! */
@@ -67,7 +67,7 @@ func main() {
 	webrtc.RegisterCodec(webrtc.NewRTCRtpVP8Codec(webrtc.DefaultPayloadTypeVP8, 90000))
 
 	// Create a new RTCPeerConnection
-	peerConnection, err := webrtc.New(peerConnectionConfig)
+	peerConnection, err := webrtc.NewRTCPeerConnection(peerConnectionConfig)
 	util.Check(err)
 
 	inboundSSRC := make(chan uint32)
@@ -111,10 +111,14 @@ func main() {
 	// Set the remote SessionDescription
 	util.Check(peerConnection.SetRemoteDescription(offer))
 
-	// Sets the LocalDescription, and starts our UDP listeners
+	// Create answer
 	answer, err := peerConnection.CreateAnswer(nil)
 	util.Check(err)
 
+	// Sets the LocalDescription, and starts our UDP listeners
+	err = peerConnection.SetLocalDescription(answer)
+	util.Check(err)
+
 	// Get the LocalDescription and take it to base64 so we can paste in browser
 	fmt.Println(util.Encode(answer))
 
@@ -125,10 +129,10 @@ func main() {
 		fmt.Println("Curl an base64 SDP to start sendonly peer connection")
 
 		recvOnlyOffer := webrtc.RTCSessionDescription{}
-		util.Decode(mustReadHttp(sdp), &recvOnlyOffer)
+		util.Decode(mustReadHTTP(sdp), &recvOnlyOffer)
 
 		// Create a new RTCPeerConnection
-		peerConnection, err := webrtc.New(peerConnectionConfig)
+		peerConnection, err := webrtc.NewRTCPeerConnection(peerConnectionConfig)
 		util.Check(err)
 
 		// Create a single VP8 Track to send videa
@@ -146,10 +150,14 @@ func main() {
 		err = peerConnection.SetRemoteDescription(recvOnlyOffer)
 		util.Check(err)
 
-		// Sets the LocalDescription, and starts our UDP listeners
+		// Create answer
 		answer, err := peerConnection.CreateAnswer(nil)
 		util.Check(err)
 
+		// Sets the LocalDescription, and starts our UDP listeners
+		err = peerConnection.SetLocalDescription(answer)
+		util.Check(err)
+
 		// Get the LocalDescription and take it to base64 so we can paste in browser
 		fmt.Println(util.Encode(answer))
 	}
diff --git a/rtcpeerconnection.go b/rtcpeerconnection.go
index 48f218a5..328729a2 100644
--- a/rtcpeerconnection.go
+++ b/rtcpeerconnection.go
@@ -457,12 +457,6 @@ func (pc *RTCPeerConnection) CreateOffer(options *RTCOfferOptions) (RTCSessionDe
 		parsed: d,
 	}
 	pc.lastOffer = desc.Sdp
-
-	// FIXME: This doesn't follow the JS API spec, but removing it
-	// would mean our examples and existing code have to change
-	if err := pc.SetLocalDescription(desc); err != nil {
-		return RTCSessionDescription{}, err
-	}
 	return desc, nil
 }
 
@@ -567,12 +561,6 @@ func (pc *RTCPeerConnection) CreateAnswer(options *RTCAnswerOptions) (RTCSession
 		parsed: d,
 	}
 	pc.lastAnswer = desc.Sdp
-
-	// FIXME: This doesn't follow the JS API spec, but removing it
-	// would mean our examples and existing code have to change
-	if err := pc.SetLocalDescription(desc); err != nil {
-		return RTCSessionDescription{}, err
-	}
 	return desc, nil
 }
 


Test patch:
diff --git a/rtcpeerconnection_test.go b/rtcpeerconnection_test.go
index 635006f8..6df44e8f 100644
--- a/rtcpeerconnection_test.go
+++ b/rtcpeerconnection_test.go
@@ -37,6 +37,10 @@ func signalPair(pcOffer *RTCPeerConnection, pcAnswer *RTCPeerConnection) error {
 		return err
 	}
 
+	if err = pcOffer.SetLocalDescription(offer); err != nil {
+		return err
+	}
+
 	err = pcAnswer.SetRemoteDescription(offer)
 	if err != nil {
 		return err
@@ -47,6 +51,10 @@ func signalPair(pcOffer *RTCPeerConnection, pcAnswer *RTCPeerConnection) error {
 		return err
 	}
 
+	if err = pcAnswer.SetLocalDescription(answer); err != nil {
+		return err
+	}
+
 	err = pcOffer.SetRemoteDescription(answer)
 	if err != nil {
 		return err
@@ -355,6 +363,9 @@ func TestCreateOfferAnswer(t *testing.T) {
 	if err != nil {
 		t.Errorf("Create Offer: got error: %v", err)
 	}
+	if err = offerPeerConn.SetLocalDescription(offer); err != nil {
+		t.Errorf("SetLocalDescription: got error: %v", err)
+	}
 	answerPeerConn, err := api.NewRTCPeerConnection(RTCConfiguration{})
 	if err != nil {
 		t.Errorf("New RTCPeerConnection: got error: %v", err)
@@ -367,6 +378,9 @@ func TestCreateOfferAnswer(t *testing.T) {
 	if err != nil {
 		t.Errorf("Create Answer: got error: %v", err)
 	}
+	if err = answerPeerConn.SetLocalDescription(answer); err != nil {
+		t.Errorf("SetLocalDescription: got error: %v", err)
+	}
 	err = offerPeerConn.SetRemoteDescription(answer)
 	if err != nil {
 		t.Errorf("SetRemoteDescription (Originator): got error: %v", err)