Repository: pions/webrtc
Pull request #381 (2019-01-28 17:40:58)

Add OnSelectedCandidatePairChange callback
Non-standard, but useful for logging the chosen candidate.

mjmac: It may be a little spammy, but since it only happens during the initial setup phase it doesn't bother me. This results in logging like the following (pions logging mixed in with app logging):

> Jan 28 12:36:22 c82a14476e150000 CameraServer[727]: ice INFO: 2019/01/28 12:36:22 Selected candidate pair: prio 24552 (local, prio 3776) host 192.168.1.5:55987 <-> relay 159.203.64.229:53015 related 107.77.224.37:45236 (remote, prio 500)
Jan 28 12:36:22 c82a14476e150000 CameraServer[727]: ice INFO: 2019/01/28 12:36:22 Setting new connection state: Connected
Jan 28 12:36:22 c82a14476e150000 CameraServer[727]: pc INFO: 2019/01/28 12:36:22 ICE candidate pair selected: (local) host 192.168.1.5:55987 <-> (remote) relay 159.203.64.229:53015 related 107.77.224.37:45236
Jan 28 12:36:22 c82a14476e150000 CameraServer[727]: pc INFO: 2019/01/28 12:36:22 ICE connection state changed: Connected
Jan 28 12:36:22 c82a14476e150000 CameraServer[727]: proxy.go:474: PeerConnection 69F7DC05-5D14-44EE-A2CA-64C2EEF57E7B: Chose ICE candidate pair: host 192.168.1.5:55987 <-> relay 159.203.64.229:53015 related 107.77.224.37:45236
Jan 28 12:36:22 c82a14476e150000 CameraServer[727]: proxy.go:458: PeerConnection 69F7DC05-5D14-44EE-A2CA-64C2EEF57E7B: New ICE connection state: Connected
coveralls: ## Pull Request Test Coverage Report for [Build 1921](https://coveralls.io/builds/21316241)

* **51** of **74**   **(68.92%)**  changed or added relevant lines in **5** files are covered.
* **2** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.03%**) to **78.62%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [pkg/ice/candidatepair.go](https://coveralls.io/builds/21316241/source?filename=pkg%2Fice%2Fcandidatepair.go#L31) | 6 | 8 | 75.0%
| [rtcicetransport.go](https://coveralls.io/builds/21316241/source?filename=rtcicetransport.go#L80) | 15 | 17 | 88.24%
| [rtcicecandidate.go](https://coveralls.io/builds/21316241/source?filename=rtcicecandidate.go#L145) | 0 | 6 | 0.0%
| [rtcpeerconnection.go](https://coveralls.io/builds/21316241/source?filename=rtcpeerconnection.go#L304) | 15 | 28 | 53.57%
<!-- | **Total:** | **51** | **74** | **68.92%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [pkg/ice/agent.go](https://coveralls.io/builds/21316241/source?filename=pkg%2Fice%2Fagent.go#L441) | 2 | 85.8% |
<!-- | **Total:** | **2** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/21316241/badge)](https://coveralls.io/builds/21316241) |
| :-- | --: |
| Change from base [Build 1918](https://coveralls.io/builds/21314695): |  -0.03% |
| Covered Lines: | 5038 |
| Relevant Lines: | 6408 |

---
##### 💛  - [Coveralls](https://coveralls.io)

Sean-Der: Hey @mjmac 

I think I saw some of the conversation on Slack, but my 2 cents (and hopefully just repeating what was already agreed on)

I am not against adding something non-standard that is helpful. Seems like a good opportunity to to innovate.

I am not sure, but would adding any of the ones in the spec satisfy your logging needs? Pulled from [here](https://www.w3.org/TR/webrtc/#interface-definition)
```
onicecandidate of type EventHandler

The event type of this event handler is icecandidate.

onicecandidateerror of type EventHandler
The event type of this event handler is icecandidateerror.

onsignalingstatechange of type EventHandler
The event type of this event handler is signalingstatechange.

oniceconnectionstatechange of type EventHandler
The event type of this event handler is iceconnectionstatechange

onicegatheringstatechange of type EventHandler
The event type of this event handler is icegatheringstatechange.

onconnectionstatechange of type EventHandler
The event type of this event handler is connectionstatechange.
```
mjmac: Hi @Sean-Der... After talking with @backkem, I changed the callback to OnCandidatePairChange to match the ORTC spec for RTCIceTransport: https://draft.ortc.org/#dom-rtcicetransport-oncandidatepairchange

I think the question is whether or not we mind exposing this via the RTCPeerConnection API even though it's not part of the official WebRTC spec. I don't see anything in the standard RTCPeerConnection callbacks that will make the selected candidate pair available to a user-supplied callback, but I'm willing to believe that I missed something... In my use case, this is an important detail to log so that we have a record of which connection method was used in order to retroactively diagnose connectivity/performance issues.
backkem: @mjmac The ICE Transport isn't exposed on PeerConnection because WebRTC allows for separate ICE connections to be made for each media stream. The correct way to add this to the WebRTC would be:
- `RTCIceTransport` has an [onselectedcandidatepairchange ](https://w3c.github.io/webrtc-pc/#dom-rtcicetransport-onselectedcandidatepairchange). The name diverts in ORTC, maybe because the WebRTC api doesn't provide info about the selected pair. I suggest we just keep one.
- `RTCDtlsTransport` has an [iceTransport](https://w3c.github.io/webrtc-pc/#dom-rtcdtlstransport-icetransport) attribute that can be used to get the `RTCIceTransport`.
- `RTCRtpSender`, `RTCRtpReceiver` and `RTCSctpTransport` has a `transport` attribute that can be used to get the `RTCDtlsTransport`.
- `RTCPeerConnection` has a [sctp](https://w3c.github.io/webrtc-pc/#rtcpeerconnection-interface-extensions-0) attribute that can be used to get the `RTCDtlsTransport`.
- `RTCPeerConnection` has [ways](https://w3c.github.io/webrtc-pc/#rtcpeerconnection-interface-extensions) to access the `RTCRtpSender` and `RTCRtpReceiver` objects.
mjmac: @backkem: OK, I'm looking at this again. I understand now that adding a single callback on PeerConnection wouldn't be sufficient.

I'm pushing another revision which removes the callback from PeerConnection but implements 
OnSelectedCandidatePairChange on ICETransport. Let me know if that's what you had in mind.

Once we're OK on that part of it, I'll need to figure out how to expose the transport to user code so that the callback can be set. I see PeerConnection has e.g. GetSenders(), but I don't see a way to get at the ICETransport for that sender unless we export something new.
backkem: Ok, great. I guess we can implement these:
- https://w3c.github.io/webrtc-pc/#dom-rtcrtpsender-transport
- https://w3c.github.io/webrtc-pc/#dom-rtcdtlstransport-icetransport
Sean-Der: @backkem @mjmac should we move `pkg/ice` -> `internal/ice`? Just to make sure we don't leak anything that is non-standard into the public API?
backkem: Fine by me.

Patch:
diff --git a/configuration.go b/configuration.go
index a78bb2ed..b121bed0 100644
--- a/configuration.go
+++ b/configuration.go
@@ -1,7 +1,7 @@
 package webrtc
 
 import (
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 )
 
 // Configuration defines a set of parameters to configure how the
diff --git a/dtlstransport.go b/dtlstransport.go
index 7cf44a39..b408fe0e 100644
--- a/dtlstransport.go
+++ b/dtlstransport.go
@@ -69,6 +69,14 @@ func (api *API) NewDTLSTransport(transport *ICETransport, certificates []Certifi
 	return t, nil
 }
 
+// ICETransport returns the currently-configured *ICETransport or nil
+// if one has not been configured
+func (t *DTLSTransport) ICETransport() *ICETransport {
+	t.lock.RLock()
+	defer t.lock.RUnlock()
+	return t.iceTransport
+}
+
 // GetLocalParameters returns the DTLS parameters of the local DTLSTransport upon construction.
 func (t *DTLSTransport) GetLocalParameters() DTLSParameters {
 	fingerprints := []DTLSFingerprint{}
diff --git a/icecandidate.go b/icecandidate.go
index e7368fe3..d4e8a77b 100644
--- a/icecandidate.go
+++ b/icecandidate.go
@@ -6,7 +6,7 @@ import (
 	"net"
 
 	"github.com/pions/sdp/v2"
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 )
 
 // ICECandidate represents a ice candidate
@@ -141,3 +141,11 @@ func convertTypeFromICE(t ice.CandidateType) (ICECandidateType, error) {
 		return ICECandidateType(t), fmt.Errorf("unknown ICE candidate type: %s", t)
 	}
 }
+
+func (c ICECandidate) String() string {
+	ic, err := c.toICE()
+	if err != nil {
+		return fmt.Sprintf("%#v failed to convert to ICE: %s", c, err)
+	}
+	return ic.String()
+}
diff --git a/icecandidatepair.go b/icecandidatepair.go
new file mode 100644
index 00000000..417b6d77
--- /dev/null
+++ b/icecandidatepair.go
@@ -0,0 +1,24 @@
+package webrtc
+
+import "fmt"
+
+type (
+	// ICECandidatePair represents an ICE Candidate pair
+	ICECandidatePair struct {
+		Local  *ICECandidate
+		Remote *ICECandidate
+	}
+)
+
+func (p *ICECandidatePair) String() string {
+	return fmt.Sprintf("(local) %s <-> (remote) %s", p.Local, p.Remote)
+}
+
+// NewICECandidatePair returns an initialized *ICECandidatePair
+// for the given pair of ICECandidate instances
+func NewICECandidatePair(local, remote *ICECandidate) *ICECandidatePair {
+	return &ICECandidatePair{
+		Local:  local,
+		Remote: remote,
+	}
+}
diff --git a/icegatherer.go b/icegatherer.go
index d131b94d..91c5b696 100644
--- a/icegatherer.go
+++ b/icegatherer.go
@@ -4,7 +4,7 @@ import (
 	"errors"
 	"sync"
 
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 )
 
 // The ICEGatherer gathers local host, server reflexive and relay
diff --git a/iceserver.go b/iceserver.go
index e0685d9b..1fb28e93 100644
--- a/iceserver.go
+++ b/iceserver.go
@@ -1,7 +1,7 @@
 package webrtc
 
 import (
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 	"github.com/pions/webrtc/pkg/rtcerr"
 )
 
diff --git a/icetransport.go b/icetransport.go
index 1699295c..810ec36a 100644
--- a/icetransport.go
+++ b/icetransport.go
@@ -5,8 +5,8 @@ import (
 	"errors"
 	"sync"
 
+	"github.com/pions/webrtc/internal/ice"
 	"github.com/pions/webrtc/internal/mux"
-	"github.com/pions/webrtc/pkg/ice"
 )
 
 // ICETransport allows an application access to information about the ICE
@@ -19,7 +19,8 @@ type ICETransport struct {
 	// State ICETransportState
 	// gatheringState ICEGathererState
 
-	onConnectionStateChangeHdlr func(ICETransportState)
+	onConnectionStateChangeHdlr       func(ICETransportState)
+	onSelectedCandidatePairChangeHdlr func(*ICECandidatePair)
 
 	gatherer *ICEGatherer
 	conn     *ice.Conn
@@ -67,10 +68,19 @@ func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role *
 	}
 
 	agent := t.gatherer.agent
-	err := agent.OnConnectionStateChange(func(iceState ice.ConnectionState) {
+	if err := agent.OnConnectionStateChange(func(iceState ice.ConnectionState) {
 		t.onConnectionStateChange(newICETransportStateFromICE(iceState))
-	})
-	if err != nil {
+	}); err != nil {
+		return err
+	}
+	if err := agent.OnSelectedCandidatePairChange(func(local, remote *ice.Candidate) {
+		candidates, err := newICECandidatesFromICE([]*ice.Candidate{local, remote})
+		if err != nil {
+			pcLog.Warnf("Unable to convert ICE candidates to ICECandidates: %s", err)
+			return
+		}
+		t.onSelectedCandidatePairChange(NewICECandidatePair(&candidates[0], &candidates[1]))
+	}); err != nil {
 		return err
 	}
 
@@ -85,6 +95,7 @@ func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role *
 	t.lock.Unlock()
 
 	var iceConn *ice.Conn
+	var err error
 	switch *role {
 	case ICERoleControlling:
 		iceConn, err = agent.Dial(context.TODO(),
@@ -124,6 +135,23 @@ func (t *ICETransport) Stop() error {
 	return nil
 }
 
+// OnSelectedCandidatePairChange sets a handler that is invoked when a new
+// ICE candidate pair is selected
+func (t *ICETransport) OnSelectedCandidatePairChange(f func(*ICECandidatePair)) {
+	t.lock.Lock()
+	defer t.lock.Unlock()
+	t.onSelectedCandidatePairChangeHdlr = f
+}
+
+func (t *ICETransport) onSelectedCandidatePairChange(pair *ICECandidatePair) {
+	t.lock.RLock()
+	hdlr := t.onSelectedCandidatePairChangeHdlr
+	t.lock.RUnlock()
+	if hdlr != nil {
+		hdlr(pair)
+	}
+}
+
 // OnConnectionStateChange sets a handler that is fired when the ICE
 // connection state changes.
 func (t *ICETransport) OnConnectionStateChange(f func(ICETransportState)) {
diff --git a/icetransportstate.go b/icetransportstate.go
index 5442c24f..bd2d87c8 100644
--- a/icetransportstate.go
+++ b/icetransportstate.go
@@ -1,6 +1,6 @@
 package webrtc
 
-import "github.com/pions/webrtc/pkg/ice"
+import "github.com/pions/webrtc/internal/ice"
 
 // ICETransportState represents the current state of the ICE transport.
 type ICETransportState int
diff --git a/pkg/ice/agent.go b/internal/ice/agent.go
similarity index 96%
rename from pkg/ice/agent.go
rename to internal/ice/agent.go
index 66c79c22..afbfae3e 100644
--- a/pkg/ice/agent.go
+++ b/internal/ice/agent.go
@@ -28,7 +28,8 @@ const (
 
 // Agent represents the ICE agent
 type Agent struct {
-	onConnectionStateChangeHdlr func(ConnectionState)
+	onConnectionStateChangeHdlr       func(ConnectionState)
+	onSelectedCandidatePairChangeHdlr func(*Candidate, *Candidate)
 
 	// Used to block double Dial/Accept
 	opened bool
@@ -170,6 +171,22 @@ func (a *Agent) OnConnectionStateChange(f func(ConnectionState)) error {
 	})
 }
 
+// OnSelectedCandidatePairChange sets a handler that is fired when the final candidate
+// pair is selected
+func (a *Agent) OnSelectedCandidatePairChange(f func(*Candidate, *Candidate)) error {
+	return a.run(func(agent *Agent) {
+		agent.onSelectedCandidatePairChangeHdlr = f
+	})
+}
+
+func (a *Agent) onSelectedCandidatePairChange(p *candidatePair) {
+	if p != nil {
+		if a.onSelectedCandidatePairChangeHdlr != nil {
+			a.onSelectedCandidatePairChangeHdlr(p.local, p.remote)
+		}
+	}
+}
+
 func (a *Agent) listenUDP(network string, laddr *net.UDPAddr) (*net.UDPConn, error) {
 	if (laddr.Port != 0) || ((a.portmin == 0) && (a.portmax == 0)) {
 		return net.ListenUDP(network, laddr)
@@ -362,6 +379,7 @@ func (a *Agent) pingCandidate(local, remote *Candidate) {
 
 func (a *Agent) updateConnectionState(newState ConnectionState) {
 	if a.connectionState != newState {
+		iceLog.Infof("Setting new connection state: %s", newState)
 		a.connectionState = newState
 		hdlr := a.onConnectionStateChangeHdlr
 		if hdlr != nil {
@@ -390,6 +408,10 @@ func (a *Agent) setValidPair(local, remote *Candidate, selected, controlling boo
 	iceLog.Tracef("Found valid candidate pair: %s (selected? %t)", p, selected)
 
 	if selected {
+		// Notify when the selected pair changes
+		if !a.selectedPair.Equal(p) {
+			a.onSelectedCandidatePairChange(p)
+		}
 		a.selectedPair = p
 		a.validPairs = nil
 		// TODO: only set state to connected on selecting final pair?
diff --git a/pkg/ice/candidate.go b/internal/ice/candidate.go
similarity index 100%
rename from pkg/ice/candidate.go
rename to internal/ice/candidate.go
diff --git a/pkg/ice/candidatepair.go b/internal/ice/candidatepair.go
similarity index 91%
rename from pkg/ice/candidatepair.go
rename to internal/ice/candidatepair.go
index ac3aa797..520be794 100644
--- a/pkg/ice/candidatepair.go
+++ b/internal/ice/candidatepair.go
@@ -26,6 +26,16 @@ func (p *candidatePair) String() string {
 		p.Priority(), p.local.Priority(), p.local, p.remote, p.remote.Priority())
 }
 
+func (p *candidatePair) Equal(other *candidatePair) bool {
+	if p == nil && other == nil {
+		return true
+	}
+	if p == nil || other == nil {
+		return false
+	}
+	return p.local.Equal(other.local) && p.remote.Equal(other.remote)
+}
+
 // RFC 5245 - 5.7.2.  Computing Pair Priority and Ordering Pairs
 // Let G be the priority for the candidate provided by the controlling
 // agent.  Let D be the priority for the candidate provided by the
diff --git a/pkg/ice/candidaterelatedaddress.go b/internal/ice/candidaterelatedaddress.go
similarity index 100%
rename from pkg/ice/candidaterelatedaddress.go
rename to internal/ice/candidaterelatedaddress.go
diff --git a/pkg/ice/candidatetype.go b/internal/ice/candidatetype.go
similarity index 100%
rename from pkg/ice/candidatetype.go
rename to internal/ice/candidatetype.go
diff --git a/pkg/ice/errors.go b/internal/ice/errors.go
similarity index 100%
rename from pkg/ice/errors.go
rename to internal/ice/errors.go
diff --git a/pkg/ice/ice.go b/internal/ice/ice.go
similarity index 100%
rename from pkg/ice/ice.go
rename to internal/ice/ice.go
diff --git a/pkg/ice/ice_test.go b/internal/ice/ice_test.go
similarity index 100%
rename from pkg/ice/ice_test.go
rename to internal/ice/ice_test.go
diff --git a/pkg/ice/logging.go b/internal/ice/logging.go
similarity index 100%
rename from pkg/ice/logging.go
rename to internal/ice/logging.go
diff --git a/pkg/ice/networktype.go b/internal/ice/networktype.go
similarity index 100%
rename from pkg/ice/networktype.go
rename to internal/ice/networktype.go
diff --git a/pkg/ice/networktype_test.go b/internal/ice/networktype_test.go
similarity index 100%
rename from pkg/ice/networktype_test.go
rename to internal/ice/networktype_test.go
diff --git a/pkg/ice/transport.go b/internal/ice/transport.go
similarity index 100%
rename from pkg/ice/transport.go
rename to internal/ice/transport.go
diff --git a/pkg/ice/transport_test.go b/internal/ice/transport_test.go
similarity index 100%
rename from pkg/ice/transport_test.go
rename to internal/ice/transport_test.go
diff --git a/pkg/ice/url.go b/internal/ice/url.go
similarity index 100%
rename from pkg/ice/url.go
rename to internal/ice/url.go
diff --git a/pkg/ice/url_test.go b/internal/ice/url_test.go
similarity index 100%
rename from pkg/ice/url_test.go
rename to internal/ice/url_test.go
diff --git a/pkg/ice/util.go b/internal/ice/util.go
similarity index 100%
rename from pkg/ice/util.go
rename to internal/ice/util.go
diff --git a/peerconnection.go b/peerconnection.go
index 78cb74f8..b9702b0e 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -14,7 +14,7 @@ import (
 
 	"github.com/pions/rtcp"
 	"github.com/pions/sdp/v2"
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 	"github.com/pions/webrtc/pkg/logging"
 	"github.com/pions/webrtc/pkg/rtcerr"
 	"github.com/pkg/errors"
diff --git a/rtpreceiver.go b/rtpreceiver.go
index fcd6d1dd..004a303d 100644
--- a/rtpreceiver.go
+++ b/rtpreceiver.go
@@ -38,6 +38,14 @@ func (api *API) NewRTPReceiver(kind RTPCodecType, transport *DTLSTransport) (*RT
 	}, nil
 }
 
+// Transport returns the currently-configured *DTLSTransport or nil
+// if one has not yet been configured
+func (r *RTPReceiver) Transport() *DTLSTransport {
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+	return r.transport
+}
+
 // Track returns the RTCRtpTransceiver track
 func (r *RTPReceiver) Track() *Track {
 	r.mu.RLock()
diff --git a/rtpsender.go b/rtpsender.go
index dbd63ef2..c19571cd 100644
--- a/rtpsender.go
+++ b/rtpsender.go
@@ -44,6 +44,14 @@ func (api *API) NewRTPSender(track *Track, transport *DTLSTransport) (*RTPSender
 	}, nil
 }
 
+// Transport returns the currently-configured *DTLSTransport or nil
+// if one has not yet been configured
+func (r *RTPSender) Transport() *DTLSTransport {
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+	return r.transport
+}
+
 // Send Attempts to set the parameters controlling the sending of media.
 func (r *RTPSender) Send(parameters RTPSendParameters) error {
 	r.mu.Lock()
diff --git a/settingengine.go b/settingengine.go
index 580388d0..41acfd64 100644
--- a/settingengine.go
+++ b/settingengine.go
@@ -3,7 +3,7 @@ package webrtc
 import (
 	"time"
 
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 )
 
 // SettingEngine allows influencing behavior in ways that are not


Test patch:
diff --git a/icecandidate_test.go b/icecandidate_test.go
index 38674c26..536a11a4 100644
--- a/icecandidate_test.go
+++ b/icecandidate_test.go
@@ -5,7 +5,7 @@ import (
 	"testing"
 
 	"github.com/pions/sdp/v2"
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 	"github.com/stretchr/testify/assert"
 )
 
diff --git a/iceserver_test.go b/iceserver_test.go
index 2eed7b51..4bb17597 100644
--- a/iceserver_test.go
+++ b/iceserver_test.go
@@ -3,7 +3,7 @@ package webrtc
 import (
 	"testing"
 
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 	"github.com/pions/webrtc/pkg/rtcerr"
 	"github.com/stretchr/testify/assert"
 )
diff --git a/icetransportstate_test.go b/icetransportstate_test.go
index b925d931..cb2c62bf 100644
--- a/icetransportstate_test.go
+++ b/icetransportstate_test.go
@@ -3,7 +3,7 @@ package webrtc
 import (
 	"testing"
 
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 	"github.com/stretchr/testify/assert"
 )
 
diff --git a/pkg/ice/agent_test.go b/internal/ice/agent_test.go
similarity index 79%
rename from pkg/ice/agent_test.go
rename to internal/ice/agent_test.go
index f10d450f..0ec47188 100644
--- a/pkg/ice/agent_test.go
+++ b/internal/ice/agent_test.go
@@ -114,6 +114,61 @@ func TestPairPriority(t *testing.T) {
 	}
 }
 
+func TestOnSelectedCandidatePairChange(t *testing.T) {
+	// avoid deadlocks?
+	defer test.TimeOut(1 * time.Second).Stop()
+
+	a, err := NewAgent(&AgentConfig{})
+	if err != nil {
+		t.Fatalf("Failed to create agent: %s", err)
+	}
+	callbackCalled := make(chan struct{}, 1)
+	if err = a.OnSelectedCandidatePairChange(func(local, remote *Candidate) {
+		close(callbackCalled)
+	}); err != nil {
+		t.Fatalf("Failed to set agent OnCandidatePairChange callback: %s", err)
+	}
+
+	hostLocal, err := NewCandidateHost(
+		"udp",
+		net.ParseIP("192.168.1.1"), 19216,
+		1,
+	)
+	if err != nil {
+		t.Fatalf("Failed to construct local host candidate: %s", err)
+	}
+
+	relayRemote, err := NewCandidateRelay(
+		"udp",
+		net.ParseIP("1.2.3.4"), 12340,
+		1,
+		"4.3.2.1", 43210,
+	)
+	if err != nil {
+		t.Fatalf("Failed to construct remote relay candidate: %s", err)
+	}
+
+	// select the pair
+	if err = a.run(func(agent *Agent) {
+		agent.setValidPair(hostLocal, relayRemote, true, false)
+	}); err != nil {
+		t.Fatalf("Failed to setValidPair(): %s", err)
+	}
+	// ensure that the callback fired on setting the pair
+	<-callbackCalled
+	// set the same pair; this should not invoke the callback
+	// if the callback is invoked now it will panic due
+	// to second close of the channel
+	if err = a.run(func(agent *Agent) {
+		agent.setValidPair(hostLocal, relayRemote, true, false)
+	}); err != nil {
+		t.Fatalf("Failed to setValidPair(): %s", err)
+	}
+	if err := a.Close(); err != nil {
+		t.Fatalf("Error on agent.Close(): %s", err)
+	}
+}
+
 type BadAddr struct{}
 
 func (ba *BadAddr) Network() string {
diff --git a/peerconnection_media_test.go b/peerconnection_media_test.go
index ce2d2f52..4f95aacf 100644
--- a/peerconnection_media_test.go
+++ b/peerconnection_media_test.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"math/rand"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -257,3 +258,74 @@ func TestPeerConnection_Media_Shutdown(t *testing.T) {
 	}
 	onTrackFiredLock.Unlock()
 }
+
+func TestPeerConnection_Media_Sender_Transports_OnSelectedCandidatePairChange(t *testing.T) {
+	iceComplete := make(chan bool)
+
+	api := NewAPI()
+	lim := test.TimeOut(time.Second * 30)
+	defer lim.Stop()
+
+	api.mediaEngine.RegisterDefaultCodecs()
+	pcOffer, pcAnswer, err := api.newPair()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	opusTrack, err := pcOffer.NewTrack(DefaultPayloadTypeOpus, rand.Uint32(), "audio", "pion1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	vp8Track, err := pcOffer.NewTrack(DefaultPayloadTypeVP8, rand.Uint32(), "video", "pion2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err = pcOffer.AddTrack(opusTrack); err != nil {
+		t.Fatal(err)
+	} else if _, err = pcAnswer.AddTrack(vp8Track); err != nil {
+		t.Fatal(err)
+	}
+
+	pcAnswer.OnICEConnectionStateChange(func(iceState ICEConnectionState) {
+		if iceState == ICEConnectionStateConnected {
+			go func() {
+				time.Sleep(3 * time.Second) // TODO PeerConnection.Close() doesn't block for all subsystems
+				close(iceComplete)
+			}()
+		}
+	})
+
+	senderCalledCandidateChange := int32(0)
+	for _, sender := range pcOffer.GetSenders() {
+		dtlsTransport := sender.Transport()
+		if dtlsTransport == nil {
+			continue
+		}
+		if iceTransport := dtlsTransport.ICETransport(); iceTransport != nil {
+			iceTransport.OnSelectedCandidatePairChange(func(pair *ICECandidatePair) {
+				atomic.StoreInt32(&senderCalledCandidateChange, 1)
+			})
+		}
+	}
+
+	err = signalPair(pcOffer, pcAnswer)
+	if err != nil {
+		t.Fatal(err)
+	}
+	<-iceComplete
+
+	if atomic.LoadInt32(&senderCalledCandidateChange) == 0 {
+		t.Fatalf("Sender ICETransport OnSelectedCandidateChange was never called")
+	}
+
+	err = pcOffer.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = pcAnswer.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+}
diff --git a/peerconnection_test.go b/peerconnection_test.go
index 8a0ad4bc..674b1c46 100644
--- a/peerconnection_test.go
+++ b/peerconnection_test.go
@@ -10,7 +10,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/pions/webrtc/pkg/ice"
+	"github.com/pions/webrtc/internal/ice"
 
 	"github.com/pions/webrtc/pkg/rtcerr"
 	"github.com/stretchr/testify/assert"