Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions peerconnection.go
Original file line number Diff line number Diff line change
Expand Up @@ -2070,8 +2070,15 @@ func (pc *PeerConnection) RemoteDescription() *SessionDescription {

// AddICECandidate accepts an ICE candidate string and adds it
// to the existing set of candidates.
func (pc *PeerConnection) AddICECandidate(candidate ICECandidateInit) error {
remoteDesc := pc.RemoteDescription()
func (pc *PeerConnection) AddICECandidate(candidate ICECandidateInit) error { //nolint:cyclop
pc.mu.Lock()
defer pc.mu.Unlock()

remoteDesc := pc.currentRemoteDescription
if pc.pendingRemoteDescription != nil {
remoteDesc = pc.pendingRemoteDescription
}

if remoteDesc == nil {
return &rtcerr.InvalidStateError{Err: ErrNoRemoteDescription}
}
Expand Down Expand Up @@ -2112,7 +2119,23 @@ func (pc *PeerConnection) AddICECandidate(candidate ICECandidateInit) error {
return err
}

return pc.iceTransport.AddRemoteCandidate(&c)
err = pc.iceTransport.AddRemoteCandidate(&c)
if err != nil {
return err
}

if pc.pendingRemoteDescription != nil {
if err := pc.pendingRemoteDescription.UpdateWithCandidate(candidate); err != nil {
return err
}
}
if pc.currentRemoteDescription != nil {
if err := pc.currentRemoteDescription.UpdateWithCandidate(candidate); err != nil {
return err
}
Comment on lines +2128 to +2135
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This updates the candidates' attributes while startRTP is reading them. which cases the test to fail with race detected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but why does startRtp look at the candidates in the SDP instead of getting them from the ICE agent? Also why does anything RTP not consider ICE a virtual socket? 🤔

Copy link
Copy Markdown
Member

@JoTurk JoTurk May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry i missed this so this is partly done for historic reasons. during early calls (we'll change this with ice v5, where we'll move more ownership to ice), also check the mux in this repo, especially at the close handling, currently we have multiple semi ownership for sockets.
There is a plan to move most of the ownership and write to one place, also so we can do low level socket optimizations and similar stuff.

}

return nil
}

// Return true if the sdp contains a specific ufrag.
Expand Down
33 changes: 33 additions & 0 deletions peerconnection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package webrtc

import (
"runtime"
"strings"
"sync"
"sync/atomic"
"testing"
Expand Down Expand Up @@ -968,3 +969,35 @@ func TestICETrickleCapabilityString(t *testing.T) {
assert.Equal(t, tt.expected, tt.value.String())
}
}

func TestPeerConnection_AddICECandidateUpdatesRemoteDescription(t *testing.T) {
pc, err := NewPeerConnection(Configuration{})
assert.NoError(t, err)

_, err = pc.AddTransceiverFromKind(RTPCodecTypeAudio)
assert.NoError(t, err)

_, err = pc.CreateDataChannel("test-channel", nil)
assert.NoError(t, err)

offer, err := pc.CreateOffer(nil)
assert.NoError(t, err)

assert.NoError(t, pc.SetRemoteDescription(offer))

candidateStr := "candidate:1 1 udp 1 1.2.3.4 1234 typ relay raddr 0.0.0.0 rport 0"
index := uint16(0)
assert.NoError(t, pc.AddICECandidate(ICECandidateInit{
Candidate: candidateStr,
SDPMLineIndex: &index,
}))

remoteSDP := pc.RemoteDescription().SDP
assert.Contains(t, remoteSDP, candidateStr)

candidateIndex := strings.Index(remoteSDP, candidateStr)
dataChannelIndex := strings.Index(remoteSDP, "m=application")
assert.True(t, candidateIndex < dataChannelIndex, "Candidate should appear before the data channel m-line")

assert.NoError(t, pc.Close())
}
41 changes: 41 additions & 0 deletions sessiondescription.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,47 @@ func (sd *SessionDescription) Unmarshal() (*sdp.SessionDescription, error) {
return sd.parsed, nil
}

// UpdateWithCandidate adds a candidate to the SDP.
func (sd *SessionDescription) UpdateWithCandidate(candidate ICECandidateInit) error { //nolint:cyclop
// Work on a fresh parse rather than the cached sd.parsed: that structure
// may be concurrently read by the operations queue (e.g. dtlsRoleFromSDP,
// startRTP) so mutating its MediaDescriptions would race.
parsed := &sdp.SessionDescription{}
if err := parsed.UnmarshalString(sd.SDP); err != nil {
return fmt.Errorf("%w: %w", ErrSDPUnmarshalling, err)
}

var targetMedia *sdp.MediaDescription
if candidate.SDPMid != nil {
for _, m := range parsed.MediaDescriptions {
if mid, ok := m.Attribute(sdp.AttrKeyMID); ok && mid == *candidate.SDPMid {
targetMedia = m

break
}
}
} else if candidate.SDPMLineIndex != nil {
if int(*candidate.SDPMLineIndex) < len(parsed.MediaDescriptions) {
targetMedia = parsed.MediaDescriptions[*candidate.SDPMLineIndex]
}
}

if targetMedia == nil {
return nil
}

candidateValue := strings.TrimPrefix(candidate.Candidate, "candidate:")
targetMedia.WithValueAttribute("candidate", candidateValue)

marshaled, err := parsed.Marshal()
if err != nil {
return err
}
sd.SDP = string(marshaled)

return nil
}

func hasICETrickleOption(desc *sdp.SessionDescription) bool {
if value, ok := desc.Attribute(sdp.AttrKeyICEOptions); ok && hasTrickleOptionValue(value) {
return true
Expand Down
Loading