Minor style fixes for active TCP candidate support

Author: stv0g

agent.go

@@ -623,7 +623,7 @@ func (a *Agent) addPair(local, remote Candidate) *CandidatePair {
 			a.log.Errorf("Failed to cast local candidate to CandidateHost")
 			return nil
 		}
-		localCandidateHost.port = localAddress.Port // this causes a data race with candidateBase.Port()
+		localCandidateHost.port = localAddress.Port // This causes a data race with candidateBase.Port()
 		local.start(a, packetConn, a.startedCh)
 	}
 	p := newCandidatePair(local, remote, a.isControlling)

agent_active_tcp_test.go

@@ -48,6 +48,7 @@ func TestAgentActiveTCP(t *testing.T) {
 		listenIPAddress         net.IP
 		selectedPairNetworkType string
 	}
+
 	testCases := []testCase{
 		{
 			name:                    "TCP4 connection",
@@ -56,29 +57,28 @@ func TestAgentActiveTCP(t *testing.T) {
 			selectedPairNetworkType: tcp,
 		},
 		{
-			name:                    "UDP is preferred over TCP4", // fails some time
+			name:                    "UDP is preferred over TCP4", // This fails some time
 			networkTypes:            supportedNetworkTypes(),
 			listenIPAddress:         getLocalIPAddress(t, NetworkTypeTCP4),
 			selectedPairNetworkType: udp,
 		},
 	}
 
 	if ipv6Available(t) {
-		tcpv6Cases := []testCase{
-			{
+		testCases = append(testCases,
+			testCase{
 				name:                    "TCP6 connection",
 				networkTypes:            []NetworkType{NetworkTypeTCP6},
 				listenIPAddress:         getLocalIPAddress(t, NetworkTypeTCP6),
 				selectedPairNetworkType: tcp,
 			},
-			{
-				name:                    "UDP is preferred over TCP6", // fails some time
+			testCase{
+				name:                    "UDP is preferred over TCP6", // This fails some time
 				networkTypes:            supportedNetworkTypes(),
 				listenIPAddress:         getLocalIPAddress(t, NetworkTypeTCP6),
 				selectedPairNetworkType: udp,
 			},
-		}
-		testCases = append(testCases, tcpv6Cases...)
+		)
 	}
 
 	for _, testCase := range testCases {

candidate_base.go

@@ -188,6 +188,44 @@ func (c *candidateBase) LocalPreference() uint16 {
 	return defaultLocalPreference
 }
 
+// TypePreference returns the type preference for this candidate
+func (c *candidateBase) TypePreference() uint16 {
+	pref := c.Type().Preference()
+	if pref == 0 {
+		return 0
+	}
+
+	if c.NetworkType().IsTCP() {
+		var tcpPriorityOffset uint16 = defaultTCPPriorityOffset
+		if c.agent() != nil {
+			tcpPriorityOffset = c.agent().tcpPriorityOffset
+		}
+
+		pref -= tcpPriorityOffset
+	}
+
+	return pref
+}
+
+// Priority computes the priority for this ICE Candidate
+// See: https://www.rfc-editor.org/rfc/rfc8445#section-5.1.2.1
+func (c *candidateBase) Priority() uint32 {
+	if c.priorityOverride != 0 {
+		return c.priorityOverride
+	}
+
+	// The local preference MUST be an integer from 0 (lowest preference) to
+	// 65535 (highest preference) inclusive.  When there is only a single IP
+	// address, this value SHOULD be set to 65535.  If there are multiple
+	// candidates for a particular component for a particular data stream
+	// that have the same type, the local preference MUST be unique for each
+	// one.
+
+	return (1<<24)*uint32(c.TypePreference()) +
+		(1<<8)*uint32(c.LocalPreference()) +
+		(1<<0)*uint32(256-c.Component())
+}
+
 // RelatedAddress returns *CandidateRelatedAddress
 func (c *candidateBase) RelatedAddress() *CandidateRelatedAddress {
 	return c.relatedAddress
@@ -343,29 +381,6 @@ func (c *candidateBase) writeTo(raw []byte, dst Candidate) (int, error) {
 	return n, nil
 }
 
-// Priority computes the priority for this ICE Candidate
-func (c *candidateBase) Priority() uint32 {
-	if c.priorityOverride != 0 {
-		return c.priorityOverride
-	}
-
-	// The local preference MUST be an integer from 0 (lowest preference) to
-	// 65535 (highest preference) inclusive.  When there is only a single IP
-	// address, this value SHOULD be set to 65535.  If there are multiple
-	// candidates for a particular component for a particular data stream
-	// that have the same type, the local preference MUST be unique for each
-	// one.
-
-	var tcpPriorityOffset uint16 = defaultTCPPriorityOffset
-	if c.agent() != nil {
-		tcpPriorityOffset = c.agent().tcpPriorityOffset
-	}
-
-	return (1<<24)*uint32(c.Type().Preference(c.networkType, tcpPriorityOffset)) +
-		(1<<8)*uint32(c.LocalPreference()) +
-		uint32(256-c.Component())
-}
-
 // Equal is used to compare two candidateBases
 func (c *candidateBase) Equal(other Candidate) bool {
 	return c.NetworkType() == other.NetworkType() &&

candidatetype.go

@@ -38,24 +38,19 @@ func (c CandidateType) String() string {
 // The RECOMMENDED values are 126 for host candidates, 100
 // for server reflexive candidates, 110 for peer reflexive candidates,
 // and 0 for relayed candidates.
-func (c CandidateType) Preference(networkType NetworkType, tcpOffset uint16) uint16 {
-	var result uint16
+func (c CandidateType) Preference() uint16 {
 	switch c {
 	case CandidateTypeHost:
-		result = 126
+		return 126
 	case CandidateTypePeerReflexive:
-		result = 110
+		return 110
 	case CandidateTypeServerReflexive:
-		result = 100
+		return 100
 	case CandidateTypeRelay, CandidateTypeUnspecified:
 		return 0
 	default:
 		return 0
 	}
-	if networkType.IsTCP() {
-		return result - tcpOffset
-	}
-	return result
 }
 
 func containsCandidateType(candidateType CandidateType, candidateTypeList []CandidateType) bool {

gather.go

@@ -25,7 +25,7 @@ const (
 	stunGatherTimeout = time.Second * 5
 )
 
-type connAndPort struct {
+type connConfig struct {
 	conn    net.PacketConn
 	port    int
 	tcpType TCPType
@@ -161,18 +161,18 @@ func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []Networ
 		}
 
 		for network := range networks {
-			var connectionConfigurations []connAndPort
+			var connConfigs []connConfig
 
 			switch network {
 			case tcp:
 				// Handle ICE TCP active mode
-				connectionConfigurations = append(connectionConfigurations, connAndPort{nil, 0, TCPTypeActive})
+				connConfigs = append(connConfigs, connConfig{nil, 0, TCPTypeActive})
 
 				// Handle ICE TCP passive mode
 				if a.tcpMux != nil {
-					connectionConfigurations = a.getTCPMuxConnections(mappedIP, ip, network, connectionConfigurations)
+					connConfigs = a.getTCPMuxConns(mappedIP, ip, network, connConfigs)
 				}
-				if len(connectionConfigurations) == 0 {
+				if len(connConfigs) == 0 {
 					// Didn't succeed with any, try the next network.
 					continue
 				}
@@ -186,36 +186,36 @@ func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []Networ
 				}
 
 				if udpConn, ok := conn.LocalAddr().(*net.UDPAddr); ok {
-					connectionConfigurations = append(connectionConfigurations, connAndPort{conn, udpConn.Port, TCPTypeUnspecified})
+					connConfigs = append(connConfigs, connConfig{conn, udpConn.Port, TCPTypeUnspecified})
 				} else {
 					a.log.Warnf("Failed to get port of UDPAddr from ListenUDPInPortRange: %s %s %s", network, ip, a.localUfrag)
 					continue
 				}
 			}
 
-			for _, connectionConfiguration := range connectionConfigurations {
+			for _, connConfig := range connConfigs {
 				hostConfig := CandidateHostConfig{
 					Network:   network,
 					Address:   address,
-					Port:      connectionConfiguration.port,
+					Port:      connConfig.port,
 					Component: ComponentRTP,
-					TCPType:   connectionConfiguration.tcpType,
+					TCPType:   connConfig.tcpType,
 				}
 
 				c, err := NewCandidateHost(&hostConfig)
 				if err != nil {
-					closeConnAndLog(connectionConfiguration.conn, a.log, "failed to create host candidate: %s %s %d: %v", network, mappedIP, connectionConfiguration.port, err)
+					closeConnAndLog(connConfig.conn, a.log, "failed to create host candidate: %s %s %d: %v", network, mappedIP, connConfig.port, err)
 					continue
 				}
 
 				if a.mDNSMode == MulticastDNSModeQueryAndGather {
 					if err = c.setIP(ip); err != nil {
-						closeConnAndLog(connectionConfiguration.conn, a.log, "failed to create host candidate: %s %s %d: %v", network, mappedIP, connectionConfiguration.port, err)
+						closeConnAndLog(connConfig.conn, a.log, "failed to create host candidate: %s %s %d: %v", network, mappedIP, connConfig.port, err)
 						continue
 					}
 				}
 
-				if err := a.addCandidate(ctx, c, connectionConfiguration.conn); err != nil {
+				if err := a.addCandidate(ctx, c, connConfig.conn); err != nil {
 					if closeErr := c.close(); closeErr != nil {
 						a.log.Warnf("Failed to close candidate: %v", closeErr)
 					}
@@ -226,7 +226,7 @@ func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []Networ
 	}
 }
 
-func (a *Agent) getTCPMuxConnections(mappedIP net.IP, ip net.IP, network string, conns []connAndPort) []connAndPort {
+func (a *Agent) getTCPMuxConns(mappedIP net.IP, ip net.IP, network string, conns []connConfig) []connConfig {
 	var muxConns []net.PacketConn
 	if multi, ok := a.tcpMux.(AllConnsGetter); ok {
 		a.log.Debugf("GetAllConns by ufrag: %s", a.localUfrag)
@@ -249,7 +249,7 @@ func (a *Agent) getTCPMuxConnections(mappedIP net.IP, ip net.IP, network string,
 	// Extract the port for each PacketConn we got.
 	for _, conn := range muxConns {
 		if tcpConn, ok := conn.LocalAddr().(*net.TCPAddr); ok {
-			conns = append(conns, connAndPort{conn, tcpConn.Port, TCPTypePassive})
+			conns = append(conns, connConfig{conn, tcpConn.Port, TCPTypePassive})
 		} else {
 			a.log.Warnf("Failed to get port of connection from TCPMux: %s %s %s", network, ip, a.localUfrag)
 		}