Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include sdpMid and sdpMLineIndex for ICECandidates returned by OneICECandidate #2990

Merged
merged 1 commit into from
Jan 9, 2025
Merged
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
34 changes: 18 additions & 16 deletions icecandidate.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@ type ICECandidate struct {
RelatedAddress string `json:"relatedAddress"`
RelatedPort uint16 `json:"relatedPort"`
TCPType string `json:"tcpType"`
SDPMid string `json:"sdpMid"`
SDPMLineIndex uint16 `json:"sdpMLineIndex"`
}

// Conversion for package ice

func newICECandidatesFromICE(iceCandidates []ice.Candidate) ([]ICECandidate, error) {
func newICECandidatesFromICE(iceCandidates []ice.Candidate, sdpMid string, sdpMLineIndex uint16) ([]ICECandidate, error) {
candidates := []ICECandidate{}

for _, i := range iceCandidates {
c, err := newICECandidateFromICE(i)
c, err := newICECandidateFromICE(i, sdpMid, sdpMLineIndex)
if err != nil {
return nil, err
}
Expand All @@ -40,7 +42,7 @@ func newICECandidatesFromICE(iceCandidates []ice.Candidate) ([]ICECandidate, err
return candidates, nil
}

func newICECandidateFromICE(i ice.Candidate) (ICECandidate, error) {
func newICECandidateFromICE(i ice.Candidate, sdpMid string, sdpMLineIndex uint16) (ICECandidate, error) {
typ, err := convertTypeFromICE(i.Type())
if err != nil {
return ICECandidate{}, err
Expand All @@ -51,15 +53,17 @@ func newICECandidateFromICE(i ice.Candidate) (ICECandidate, error) {
}

c := ICECandidate{
statsID: i.ID(),
Foundation: i.Foundation(),
Priority: i.Priority(),
Address: i.Address(),
Protocol: protocol,
Port: uint16(i.Port()),
Component: i.Component(),
Typ: typ,
TCPType: i.TCPType().String(),
statsID: i.ID(),
Foundation: i.Foundation(),
Priority: i.Priority(),
Address: i.Address(),
Protocol: protocol,
Port: uint16(i.Port()),
Component: i.Component(),
Typ: typ,
TCPType: i.TCPType().String(),
SDPMid: sdpMid,
SDPMLineIndex: sdpMLineIndex,
}

if i.RelatedAddress() != nil {
Expand Down Expand Up @@ -155,8 +159,6 @@ func (c ICECandidate) String() string {
// ToJSON returns an ICECandidateInit
// as indicated by the spec https://w3c.github.io/webrtc-pc/#dom-rtcicecandidate-tojson
func (c ICECandidate) ToJSON() ICECandidateInit {
zeroVal := uint16(0)
emptyStr := ""
candidateStr := ""

candidate, err := c.toICE()
Expand All @@ -166,7 +168,7 @@ func (c ICECandidate) ToJSON() ICECandidateInit {

return ICECandidateInit{
Candidate: fmt.Sprintf("candidate:%s", candidateStr),
SDPMid: &emptyStr,
SDPMLineIndex: &zeroVal,
SDPMid: &c.SDPMid,
SDPMLineIndex: &c.SDPMLineIndex,
}
}
65 changes: 65 additions & 0 deletions icecandidate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,54 @@ func TestConvertTypeFromICE(t *testing.T) {
})
}

func TestNewIdentifiedICECandidateFromICE(t *testing.T) {
config := ice.CandidateHostConfig{
Network: "udp",
Address: "::1",
Port: 1234,
Component: 1,
Foundation: "foundation",
Priority: 128,
}
ice, err := ice.NewCandidateHost(&config)
assert.NoError(t, err)

ct, err := newICECandidateFromICE(ice, "1", 2)
assert.NoError(t, err)

assert.Equal(t, "1", ct.SDPMid)
assert.Equal(t, uint16(2), ct.SDPMLineIndex)
}

func TestNewIdentifiedICECandidatesFromICE(t *testing.T) {
ic, err := ice.NewCandidateHost(&ice.CandidateHostConfig{
Network: "udp",
Address: "::1",
Port: 1234,
Component: 1,
Foundation: "foundation",
Priority: 128,
})

assert.NoError(t, err)

candidates := []ice.Candidate{ic, ic, ic}

sdpMid := "1"
sdpMLineIndex := uint16(2)

results, err := newICECandidatesFromICE(candidates, sdpMid, sdpMLineIndex)

assert.NoError(t, err)

assert.Equal(t, 3, len(results))

for _, result := range results {
assert.Equal(t, sdpMid, result.SDPMid)
assert.Equal(t, sdpMLineIndex, result.SDPMLineIndex)
}
}

func TestICECandidate_ToJSON(t *testing.T) {
candidate := ICECandidate{
Foundation: "foundation",
Expand All @@ -183,3 +231,20 @@ func TestICECandidate_ToJSON(t *testing.T) {
assert.Equal(t, uint16(0), *candidateInit.SDPMLineIndex)
assert.Equal(t, "candidate:foundation 1 udp 128 1.0.0.1 1234 typ host", candidateInit.Candidate)
}

func TestICECandidateZeroSDPid(t *testing.T) {
candidate := ICECandidate{}

assert.Equal(t, candidate.SDPMid, "")
assert.Equal(t, candidate.SDPMLineIndex, uint16(0))
}

func TestICECandidateSDPMid_ToJSON(t *testing.T) {
candidate := ICECandidate{}

candidate.SDPMid = "0"
candidate.SDPMLineIndex = 1

assert.Equal(t, candidate.SDPMid, "0")
assert.Equal(t, candidate.SDPMLineIndex, uint16(1))
}
32 changes: 30 additions & 2 deletions icegatherer.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ type ICEGatherer struct {
onGatheringCompleteHandler atomic.Value // func()

api *API

// Used to set the corresponding media stream identification tag and media description index
// for ICE candidates generated by this gatherer.
sdpMid atomic.Value // string
sdpMLineIndex atomic.Uint32 // uint16
}

// NewICEGatherer creates a new NewICEGatherer.
Expand All @@ -60,6 +65,8 @@ func (api *API) NewICEGatherer(opts ICEGatherOptions) (*ICEGatherer, error) {
validatedServers: validatedServers,
api: api,
log: api.settingEngine.LoggerFactory.NewLogger("ice"),
sdpMid: atomic.Value{},
sdpMLineIndex: atomic.Uint32{},
}, nil
}

Expand Down Expand Up @@ -169,8 +176,16 @@ func (g *ICEGatherer) Gather() error {
onGatheringCompleteHandler = handler
}

sdpMid := ""

if mid, ok := g.sdpMid.Load().(string); ok {
sdpMid = mid
}

sdpMLineIndex := uint16(g.sdpMLineIndex.Load())

if candidate != nil {
c, err := newICECandidateFromICE(candidate)
c, err := newICECandidateFromICE(candidate, sdpMid, sdpMLineIndex)
if err != nil {
g.log.Warnf("Failed to convert ice.Candidate: %s", err)
return
Expand All @@ -188,6 +203,12 @@ func (g *ICEGatherer) Gather() error {
return agent.GatherCandidates()
}

// set media stream identification tag and media description index for this gatherer
func (g *ICEGatherer) setMediaStreamIdentification(mid string, mLineIndex uint16) {
g.sdpMid.Store(mid)
g.sdpMLineIndex.Store(uint32(mLineIndex))
}

// Close prunes all local candidates, and closes the ports.
func (g *ICEGatherer) Close() error {
return g.close(false /* shouldGracefullyClose */)
Expand Down Expand Up @@ -264,7 +285,14 @@ func (g *ICEGatherer) GetLocalCandidates() ([]ICECandidate, error) {
return nil, err
}

return newICECandidatesFromICE(iceCandidates)
sdpMid := ""
if mid, ok := g.sdpMid.Load().(string); ok {
sdpMid = mid
}

sdpMLineIndex := uint16(g.sdpMLineIndex.Load())

return newICECandidatesFromICE(iceCandidates, sdpMid, sdpMLineIndex)
}

// OnLocalCandidate sets an event handler which fires when a new local ICE candidate is available
Expand Down
69 changes: 69 additions & 0 deletions icegatherer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,72 @@ func TestICEGatherer_AlreadyClosed(t *testing.T) {
assert.ErrorIs(t, err, errICEAgentNotExist)
})
}

func TestNewICEGathererSetMediaStreamIdentification(t *testing.T) {
// Limit runtime in case of deadlocks
lim := test.TimeOut(time.Second * 20)
defer lim.Stop()

report := test.CheckRoutines(t)
defer report()

opts := ICEGatherOptions{
ICEServers: []ICEServer{{URLs: []string{"stun:stun.l.google.com:19302"}}},
}

gatherer, err := NewAPI().NewICEGatherer(opts)
if err != nil {
t.Error(err)
}

expectedMid := "5"
expectedMLineIndex := uint16(1)

gatherer.setMediaStreamIdentification(expectedMid, expectedMLineIndex)

if gatherer.State() != ICEGathererStateNew {
t.Fatalf("Expected gathering state new")
}

gatherFinished := make(chan struct{})
gatherer.OnLocalCandidate(func(i *ICECandidate) {
if i == nil {
close(gatherFinished)
} else {
assert.Equal(t, expectedMid, i.SDPMid)
assert.Equal(t, expectedMLineIndex, i.SDPMLineIndex)
}
})

if err = gatherer.Gather(); err != nil {
t.Error(err)
}

<-gatherFinished

params, err := gatherer.GetLocalParameters()
if err != nil {
t.Error(err)
}

if params.UsernameFragment == "" ||
params.Password == "" {
t.Fatalf("Empty local username or password frag")
}

candidates, err := gatherer.GetLocalCandidates()
if err != nil {
t.Error(err)
}

if len(candidates) == 0 {
t.Fatalf("No candidates gathered")
}

for _, c := range candidates {
assert.Equal(t, expectedMid, c.SDPMid)
assert.Equal(t, expectedMLineIndex, c.SDPMLineIndex)
}

assert.NoError(t, gatherer.Close())
}
6 changes: 3 additions & 3 deletions icetransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ func (t *ICETransport) GetSelectedCandidatePair() (*ICECandidatePair, error) {
return nil, err
}

local, err := newICECandidateFromICE(icePair.Local)
local, err := newICECandidateFromICE(icePair.Local, "", 0)
if err != nil {
return nil, err
}

remote, err := newICECandidateFromICE(icePair.Remote)
remote, err := newICECandidateFromICE(icePair.Remote, "", 0)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -118,7 +118,7 @@ func (t *ICETransport) Start(gatherer *ICEGatherer, params ICEParameters, role *
return err
}
if err := agent.OnSelectedCandidatePairChange(func(local, remote ice.Candidate) {
candidates, err := newICECandidatesFromICE([]ice.Candidate{local, remote})
candidates, err := newICECandidatesFromICE([]ice.Candidate{local, remote}, "", 0)
if err != nil {
t.log.Warnf("%w: %s", errICECandiatesCoversionFailed, err)
return
Expand Down
19 changes: 12 additions & 7 deletions peerconnection.go
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,11 @@ func (pc *PeerConnection) SetLocalDescription(desc SessionDescription) error {
})
}

mediaSection, ok := selectCandidateMediaSection(desc.parsed)
if ok {
pc.iceGatherer.setMediaStreamIdentification(mediaSection.SDPMid, mediaSection.SDPMLineIndex)
}

if pc.iceGatherer.State() == ICEGathererStateNew {
return pc.iceGatherer.Gather()
}
Expand Down Expand Up @@ -1151,26 +1156,26 @@ func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error {
}
}

remoteUfrag, remotePwd, candidates, err := extractICEDetails(desc.parsed, pc.log)
iceDetails, err := extractICEDetails(desc.parsed, pc.log)
if err != nil {
return err
}

if isRenegotiation && pc.iceTransport.haveRemoteCredentialsChange(remoteUfrag, remotePwd) {
if isRenegotiation && pc.iceTransport.haveRemoteCredentialsChange(iceDetails.Ufrag, iceDetails.Password) {
// An ICE Restart only happens implicitly for a SetRemoteDescription of type offer
if !weOffer {
if err = pc.iceTransport.restart(); err != nil {
return err
}
}

if err = pc.iceTransport.setRemoteCredentials(remoteUfrag, remotePwd); err != nil {
if err = pc.iceTransport.setRemoteCredentials(iceDetails.Ufrag, iceDetails.Password); err != nil {
return err
}
}

for i := range candidates {
if err = pc.iceTransport.AddRemoteCandidate(&candidates[i]); err != nil {
for i := range iceDetails.Candidates {
if err = pc.iceTransport.AddRemoteCandidate(&iceDetails.Candidates[i]); err != nil {
return err
}
}
Expand Down Expand Up @@ -1218,7 +1223,7 @@ func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error {
}

pc.ops.Enqueue(func() {
pc.startTransports(iceRole, dtlsRoleFromRemoteSDP(desc.parsed), remoteUfrag, remotePwd, fingerprint, fingerprintHash)
pc.startTransports(iceRole, dtlsRoleFromRemoteSDP(desc.parsed), iceDetails.Ufrag, iceDetails.Password, fingerprint, fingerprintHash)
if weOffer {
pc.startRTP(false, &desc, currentTransceivers)
}
Expand Down Expand Up @@ -1806,7 +1811,7 @@ func (pc *PeerConnection) AddICECandidate(candidate ICECandidateInit) error {
return err
}

c, err := newICECandidateFromICE(candidate)
c, err := newICECandidateFromICE(candidate, "", 0)
if err != nil {
return err
}
Expand Down
Loading
Loading