Skip to content

Commit

Permalink
Include sdpMid and sdpMLineIndex for ICECandidates returned by OneICE…
Browse files Browse the repository at this point in the history
…Candidate (#2990)

#### Description
Currently, Pion returns an empty `sdpMid` and a 0 `sdpMLineIndex`. This
PR ensures Pion returns the corresponding `sdpMid` and `sdpMLineIndex`
for ICE candidates for clients that expects it. Fixes trickle issues.

#### Changes

1. `ICECandidates`: New fields `SDPMid` and `SDPMLineIndex`.
2. `ICEGatherer`: `SetMediaStreamIdentification` and return the correct
`SDPMid` and `SDPMLineIndex`.
3. `extractICEDetails`: Return a struct instead of multiple values.
4. `extractICEDetails` refactored the media description selection to a
different function.
5. Added new tests.

#### Reference issue
Fixes #2690
Fixes #1833
JoeTurki authored Jan 9, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent c50ca41 commit 5edce95
Showing 10 changed files with 567 additions and 92 deletions.
34 changes: 18 additions & 16 deletions icecandidate.go
Original file line number Diff line number Diff line change
@@ -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
}
@@ -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
@@ -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 {
@@ -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()
@@ -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
@@ -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",
@@ -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
@@ -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.
@@ -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
}

@@ -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
@@ -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 */)
@@ -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
69 changes: 69 additions & 0 deletions icegatherer_test.go
Original file line number Diff line number Diff line change
@@ -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
@@ -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
}
@@ -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
19 changes: 12 additions & 7 deletions peerconnection.go
Original file line number Diff line number Diff line change
@@ -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()
}
@@ -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
}
}
@@ -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)
}
@@ -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
}
183 changes: 183 additions & 0 deletions peerconnection_go_test.go
Original file line number Diff line number Diff line change
@@ -1665,3 +1665,186 @@ func TestPeerConnectionNoNULLCipherDefault(t *testing.T) {
<-peerConnectionClosed
closePairNow(t, offerPC, answerPC)
}

// https://github.com/pion/webrtc/issues/2690
func TestPeerConnectionTrickleMediaStreamIdentification(t *testing.T) {
const remoteSdp = `v=0
o=- 1735985477255306 1 IN IP4 127.0.0.1
s=VideoRoom 1234
t=0 0
a=group:BUNDLE 0 1
a=ice-options:trickle
a=fingerprint:sha-256 61:BF:17:29:C0:EF:B2:77:75:79:64:F9:D8:D0:03:6C:5A:D3:9A:BC:E5:F4:5A:05:4C:3C:3B:A0:B4:2B:CF:A8
a=extmap-allow-mixed
a=msid-semantic: WMS *
m=audio 9 UDP/TLS/RTP/SAVPF 111
c=IN IP4 127.0.0.1
a=sendonly
a=mid:0
a=rtcp-mux
a=ice-ufrag:xv3r
a=ice-pwd:NT22yM6JeOsahq00U9ZJS/
a=ice-options:trickle
a=setup:actpass
a=rtpmap:111 opus/48000/2
a=rtcp-fb:111 transport-cc
a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level
a=extmap:4 urn:ietf:params:rtp-hdrext:sdes:mid
a=fmtp:111 useinbandfec=1
a=msid:janus janus0
a=ssrc:2280306597 cname:janus
m=video 9 UDP/TLS/RTP/SAVPF 96 97
c=IN IP4 127.0.0.1
a=sendonly
a=mid:1
a=rtcp-mux
a=ice-ufrag:xv3r
a=ice-pwd:NT22yM6JeOsahq00U9ZJS/
a=ice-options:trickle
a=setup:actpass
a=rtpmap:96 VP8/90000
a=rtcp-fb:96 ccm fir
a=rtcp-fb:96 nack
a=rtcp-fb:96 nack pli
a=rtcp-fb:96 goog-remb
a=rtcp-fb:96 transport-cc
a=extmap:2 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time
a=extmap:3 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01
a=extmap:4 urn:ietf:params:rtp-hdrext:sdes:mid
a=extmap:12 http://www.webrtc.org/experiments/rtp-hdrext/playout-delay
a=extmap:13 urn:3gpp:video-orientation
a=rtpmap:97 rtx/90000
a=fmtp:97 apt=96
a=ssrc-group:FID 4099488402 29586368
a=msid:janus janus1
a=ssrc:4099488402 cname:janus
a=ssrc:29586368 cname:janus
`

mediaEngine := &MediaEngine{}

assert.NoError(t, mediaEngine.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{MimeType: MimeTypeVP8, ClockRate: 90000, Channels: 0, SDPFmtpLine: "", RTCPFeedback: nil},
PayloadType: 96,
}, RTPCodecTypeVideo))
assert.NoError(t, mediaEngine.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{MimeType: MimeTypeOpus, ClockRate: 48000, Channels: 0, SDPFmtpLine: "", RTCPFeedback: nil},
PayloadType: 111,
}, RTPCodecTypeAudio))

api := NewAPI(WithMediaEngine(mediaEngine))
pc, err := api.NewPeerConnection(Configuration{
ICEServers: []ICEServer{
{
URLs: []string{"stun:stun.l.google.com:19302"},
},
},
})
assert.NoError(t, err)

pc.OnICECandidate(func(candidate *ICECandidate) {
if candidate == nil {
return
}

assert.NotEmpty(t, candidate.SDPMid)

assert.Contains(t, []string{"0", "1"}, candidate.SDPMid)
assert.Contains(t, []uint16{0, 1}, candidate.SDPMLineIndex)
})

assert.NoError(t, pc.SetRemoteDescription(SessionDescription{
Type: SDPTypeOffer,
SDP: remoteSdp,
}))

gatherComplete := GatheringCompletePromise(pc)
ans, _ := pc.CreateAnswer(nil)
assert.NoError(t, pc.SetLocalDescription(ans))

<-gatherComplete

assert.NoError(t, pc.Close())

assert.Equal(t, PeerConnectionStateClosed, pc.ConnectionState())
}

func TestTranceiverMediaStreamIdentification(t *testing.T) {
const videoMid = "0"
const audioMid = "1"

mediaEngine := &MediaEngine{}

assert.NoError(t, mediaEngine.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{MimeType: MimeTypeVP8, ClockRate: 90000, Channels: 0, SDPFmtpLine: "", RTCPFeedback: nil},
PayloadType: 96,
}, RTPCodecTypeVideo))
assert.NoError(t, mediaEngine.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{MimeType: MimeTypeOpus, ClockRate: 48000, Channels: 0, SDPFmtpLine: "", RTCPFeedback: nil},
PayloadType: 111,
}, RTPCodecTypeAudio))

api := NewAPI(WithMediaEngine(mediaEngine))
pcOfferer, pcAnswerer, err := api.newPair(Configuration{
ICEServers: []ICEServer{
{
URLs: []string{"stun:stun.l.google.com:19302"},
},
},
})
assert.NoError(t, err)

pcOfferer.OnICECandidate(func(candidate *ICECandidate) {
if candidate == nil {
return
}

assert.NotEmpty(t, candidate.SDPMid)
assert.Contains(t, []string{videoMid, audioMid}, candidate.SDPMid)
assert.Contains(t, []uint16{0, 1}, candidate.SDPMLineIndex)
})

pcAnswerer.OnICECandidate(func(candidate *ICECandidate) {
if candidate == nil {
return
}

assert.NotEmpty(t, candidate.SDPMid)
assert.Contains(t, []string{videoMid, audioMid}, candidate.SDPMid)
assert.Contains(t, []uint16{0, 1}, candidate.SDPMLineIndex)
})

videoTransceiver, err := pcOfferer.AddTransceiverFromKind(RTPCodecTypeVideo, RTPTransceiverInit{
Direction: RTPTransceiverDirectionRecvonly,
})
assert.NoError(t, err)

audioTransceiver, err := pcOfferer.AddTransceiverFromKind(RTPCodecTypeAudio, RTPTransceiverInit{
Direction: RTPTransceiverDirectionRecvonly,
})
assert.NoError(t, err)

assert.NoError(t, videoTransceiver.SetMid(videoMid))
assert.NoError(t, audioTransceiver.SetMid(audioMid))

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

assert.NoError(t, pcOfferer.SetLocalDescription(offer))

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

answer, err := pcAnswerer.CreateAnswer(nil)
assert.NoError(t, err)

assert.NoError(t, pcAnswerer.SetLocalDescription(answer))

answerGatherComplete := GatheringCompletePromise(pcOfferer)
offerGatherComplete := GatheringCompletePromise(pcAnswerer)

<-answerGatherComplete
<-offerGatherComplete

assert.NoError(t, pcOfferer.Close())
assert.NoError(t, pcAnswerer.Close())
}
2 changes: 1 addition & 1 deletion peerconnection_js.go
Original file line number Diff line number Diff line change
@@ -667,7 +667,7 @@ func valueToICECandidate(val js.Value) *ICECandidate {
return nil
}

iceCandidate, err := newICECandidateFromICE(c)
iceCandidate, err := newICECandidateFromICE(c, "", 0)
if err != nil {
return nil
}
110 changes: 68 additions & 42 deletions sdp.go
Original file line number Diff line number Diff line change
@@ -782,18 +782,26 @@ func extractFingerprint(desc *sdp.SessionDescription) (string, string, error) {
return parts[1], parts[0], nil
}

func extractICEDetailsFromMedia(media *sdp.MediaDescription, log logging.LeveledLogger) (string, string, []ICECandidate, error) {
// identifiedMediaDescription contains a MediaDescription with sdpMid and sdpMLineIndex
type identifiedMediaDescription struct {
MediaDescription *sdp.MediaDescription
SDPMid string
SDPMLineIndex uint16
}

func extractICEDetailsFromMedia(media *identifiedMediaDescription, log logging.LeveledLogger) (string, string, []ICECandidate, error) {
remoteUfrag := ""
remotePwd := ""
candidates := []ICECandidate{}
descr := media.MediaDescription

if ufrag, haveUfrag := media.Attribute("ice-ufrag"); haveUfrag {
if ufrag, haveUfrag := descr.Attribute("ice-ufrag"); haveUfrag {
remoteUfrag = ufrag
}
if pwd, havePwd := media.Attribute("ice-pwd"); havePwd {
if pwd, havePwd := descr.Attribute("ice-pwd"); havePwd {
remotePwd = pwd
}
for _, a := range media.Attributes {
for _, a := range descr.Attributes {
if a.IsICECandidate() {
c, err := ice.UnmarshalCandidate(a.Value)
if err != nil {
@@ -804,7 +812,7 @@ func extractICEDetailsFromMedia(media *sdp.MediaDescription, log logging.Leveled
return "", "", nil, err
}

candidate, err := newICECandidateFromICE(c)
candidate, err := newICECandidateFromICE(c, media.SDPMid, media.SDPMLineIndex)
if err != nil {
return "", "", nil, err
}
@@ -816,59 +824,77 @@ func extractICEDetailsFromMedia(media *sdp.MediaDescription, log logging.Leveled
return remoteUfrag, remotePwd, candidates, nil
}

func extractICEDetails(desc *sdp.SessionDescription, log logging.LeveledLogger) (string, string, []ICECandidate, error) { // nolint:gocognit
remoteCandidates := []ICECandidate{}
remotePwd := ""
remoteUfrag := ""
type sdpICEDetails struct {
Ufrag string
Password string
Candidates []ICECandidate
}

func extractICEDetails(desc *sdp.SessionDescription, log logging.LeveledLogger) (*sdpICEDetails, error) { // nolint:gocognit
details := &sdpICEDetails{
Candidates: []ICECandidate{},
}

// Ufrag and Pw are allow at session level and thus have highest prio
if ufrag, haveUfrag := desc.Attribute("ice-ufrag"); haveUfrag {
remoteUfrag = ufrag
details.Ufrag = ufrag
}
if pwd, havePwd := desc.Attribute("ice-pwd"); havePwd {
remotePwd = pwd
details.Password = pwd
}

bundleID := extractBundleID(desc)
missing := true
mediaDescr, ok := selectCandidateMediaSection(desc)
if ok {
ufrag, pwd, candidates, err := extractICEDetailsFromMedia(mediaDescr, log)
if err != nil {
return nil, err
}

if details.Ufrag == "" && ufrag != "" {
details.Ufrag = ufrag
details.Password = pwd
}

for _, m := range desc.MediaDescriptions {
mid := getMidValue(m)
details.Candidates = candidates
}

if details.Ufrag == "" {
return nil, ErrSessionDescriptionMissingIceUfrag
} else if details.Password == "" {
return nil, ErrSessionDescriptionMissingIcePwd
}

return details, nil
}

// Select the first media section or the first bundle section
// Currently Pion uses the first media section to gather candidates.
// https://github.com/pion/webrtc/pull/2950
func selectCandidateMediaSection(sessionDescription *sdp.SessionDescription) (descr *identifiedMediaDescription, ok bool) {
bundleID := extractBundleID(sessionDescription)

for mLineIndex, mediaDescr := range sessionDescription.MediaDescriptions {
mid := getMidValue(mediaDescr)
// If bundled, only take ICE detail from bundle master section
if bundleID != "" {
if mid == bundleID {
ufrag, pwd, candidates, err := extractICEDetailsFromMedia(m, log)
if err != nil {
return "", "", nil, err
}
if remoteUfrag == "" && ufrag != "" {
remoteUfrag = ufrag
remotePwd = pwd
}
remoteCandidates = candidates
return &identifiedMediaDescription{
MediaDescription: mediaDescr,
SDPMid: mid,
SDPMLineIndex: uint16(mLineIndex),
}, true
}
} else if missing {
} else {
// For not-bundled, take ICE details from the first media section
ufrag, pwd, candidates, err := extractICEDetailsFromMedia(m, log)
if err != nil {
return "", "", nil, err
}
if remoteUfrag == "" && ufrag != "" {
remoteUfrag = ufrag
remotePwd = pwd
}
remoteCandidates = candidates
missing = false
return &identifiedMediaDescription{
MediaDescription: mediaDescr,
SDPMid: mid,
SDPMLineIndex: uint16(mLineIndex),
}, true
}
}

if remoteUfrag == "" {
return "", "", nil, ErrSessionDescriptionMissingIceUfrag
} else if remotePwd == "" {
return "", "", nil, ErrSessionDescriptionMissingIcePwd
}

return remoteUfrag, remotePwd, remoteCandidates, nil
return nil, false
}

func haveApplicationMediaSection(desc *sdp.SessionDescription) bool {
139 changes: 118 additions & 21 deletions sdp_test.go
Original file line number Diff line number Diff line change
@@ -130,7 +130,7 @@ func TestExtractICEDetails(t *testing.T) {
},
}

_, _, _, err := extractICEDetails(s, nil)
_, err := extractICEDetails(s, nil)
assert.Equal(t, err, ErrSessionDescriptionMissingIcePwd)
})

@@ -141,7 +141,7 @@ func TestExtractICEDetails(t *testing.T) {
},
}

_, _, _, err := extractICEDetails(s, nil)
_, err := extractICEDetails(s, nil)
assert.Equal(t, err, ErrSessionDescriptionMissingIceUfrag)
})

@@ -154,10 +154,10 @@ func TestExtractICEDetails(t *testing.T) {
MediaDescriptions: []*sdp.MediaDescription{},
}

ufrag, pwd, _, err := extractICEDetails(s, nil)
assert.Equal(t, ufrag, defaultUfrag)
assert.Equal(t, pwd, defaultPwd)
details, err := extractICEDetails(s, nil)
assert.NoError(t, err)
assert.Equal(t, details.Ufrag, defaultUfrag)
assert.Equal(t, details.Password, defaultPwd)
})

t.Run("ice details at media level", func(t *testing.T) {
@@ -172,14 +172,14 @@ func TestExtractICEDetails(t *testing.T) {
},
}

ufrag, pwd, _, err := extractICEDetails(s, nil)
assert.Equal(t, ufrag, defaultUfrag)
assert.Equal(t, pwd, defaultPwd)
details, err := extractICEDetails(s, nil)
assert.NoError(t, err)
assert.Equal(t, details.Ufrag, defaultUfrag)
assert.Equal(t, details.Password, defaultPwd)
})

t.Run("ice details at session preferred over media", func(t *testing.T) {
s := &sdp.SessionDescription{
descr := &sdp.SessionDescription{
Attributes: []sdp.Attribute{
{Key: "ice-ufrag", Value: defaultUfrag},
{Key: "ice-pwd", Value: defaultPwd},
@@ -194,14 +194,14 @@ func TestExtractICEDetails(t *testing.T) {
},
}

ufrag, pwd, _, err := extractICEDetails(s, nil)
assert.Equal(t, ufrag, defaultUfrag)
assert.Equal(t, pwd, defaultPwd)
details, err := extractICEDetails(descr, nil)
assert.NoError(t, err)
assert.Equal(t, details.Ufrag, defaultUfrag)
assert.Equal(t, details.Password, defaultPwd)
})

t.Run("ice details from bundle media section", func(t *testing.T) {
s := &sdp.SessionDescription{
descr := &sdp.SessionDescription{
Attributes: []sdp.Attribute{
{Key: "group", Value: "BUNDLE 5 2"},
},
@@ -223,19 +223,20 @@ func TestExtractICEDetails(t *testing.T) {
},
}

ufrag, pwd, _, err := extractICEDetails(s, nil)
assert.Equal(t, ufrag, defaultUfrag)
assert.Equal(t, pwd, defaultPwd)
details, err := extractICEDetails(descr, nil)
assert.NoError(t, err)
assert.Equal(t, details.Ufrag, defaultUfrag)
assert.Equal(t, details.Password, defaultPwd)
})

t.Run("ice details from first media section", func(t *testing.T) {
s := &sdp.SessionDescription{
descr := &sdp.SessionDescription{
MediaDescriptions: []*sdp.MediaDescription{
{
Attributes: []sdp.Attribute{
{Key: "ice-ufrag", Value: defaultUfrag},
{Key: "ice-pwd", Value: defaultPwd},
{Key: "mid", Value: "5"},
},
},
{
@@ -247,10 +248,10 @@ func TestExtractICEDetails(t *testing.T) {
},
}

ufrag, pwd, _, err := extractICEDetails(s, nil)
assert.Equal(t, ufrag, defaultUfrag)
assert.Equal(t, pwd, defaultPwd)
details, err := extractICEDetails(descr, nil)
assert.NoError(t, err)
assert.Equal(t, details.Ufrag, defaultUfrag)
assert.Equal(t, details.Password, defaultPwd)
})

t.Run("Missing pwd at session level", func(t *testing.T) {
@@ -261,9 +262,105 @@ func TestExtractICEDetails(t *testing.T) {
},
}

_, _, _, err := extractICEDetails(s, nil)
_, err := extractICEDetails(s, nil)
assert.Equal(t, err, ErrSessionDescriptionMissingIcePwd)
})

t.Run("Extracts candidate from media section", func(t *testing.T) {
s := &sdp.SessionDescription{
Attributes: []sdp.Attribute{
{Key: "group", Value: "BUNDLE video audio"},
},
MediaDescriptions: []*sdp.MediaDescription{
{
MediaName: sdp.MediaName{
Media: "audio",
},
Attributes: []sdp.Attribute{
{Key: "ice-ufrag", Value: "ufrag"},
{Key: "ice-pwd", Value: "pwd"},
{Key: "ice-options", Value: "google-ice"},
{Key: "candidate", Value: "1 1 udp 2122162783 192.168.84.254 46492 typ host generation 0"},
},
},
{
MediaName: sdp.MediaName{
Media: "video",
},
Attributes: []sdp.Attribute{
{Key: "ice-ufrag", Value: "ufrag"},
{Key: "ice-pwd", Value: "pwd"},
{Key: "ice-options", Value: "google-ice"},
{Key: "mid", Value: "video"},
{Key: "candidate", Value: "1 1 udp 2122162783 192.168.84.254 46492 typ host generation 0"},
},
},
},
}

details, err := extractICEDetails(s, nil)
assert.NoError(t, err)
assert.Equal(t, details.Ufrag, "ufrag")
assert.Equal(t, details.Password, "pwd")
assert.Equal(t, details.Candidates[0].Address, "192.168.84.254")
assert.Equal(t, details.Candidates[0].Port, uint16(46492))
assert.Equal(t, details.Candidates[0].Typ, ICECandidateTypeHost)
assert.Equal(t, details.Candidates[0].SDPMid, "video")
assert.Equal(t, details.Candidates[0].SDPMLineIndex, uint16(1))
})
}

func TestSelectCandidateMediaSection(t *testing.T) {
t.Run("no media section", func(t *testing.T) {
descr := &sdp.SessionDescription{}

media, ok := selectCandidateMediaSection(descr)
assert.False(t, ok)
assert.Nil(t, media)
})

t.Run("no bundle", func(t *testing.T) {
descr := &sdp.SessionDescription{
MediaDescriptions: []*sdp.MediaDescription{
{Attributes: []sdp.Attribute{{Key: "mid", Value: "0"}}},
{Attributes: []sdp.Attribute{{Key: "mid", Value: "1"}}},
},
}

media, ok := selectCandidateMediaSection(descr)
assert.True(t, ok)
assert.NotNil(t, media)
assert.NotNil(t, media.MediaDescription)
assert.Equal(t, "0", media.SDPMid)
assert.Equal(t, uint16(0), media.SDPMLineIndex)
})

t.Run("with bundle", func(t *testing.T) {
descr := &sdp.SessionDescription{
Attributes: []sdp.Attribute{
{Key: "group", Value: "BUNDLE 5 2"},
},
MediaDescriptions: []*sdp.MediaDescription{
{
Attributes: []sdp.Attribute{
{Key: "mid", Value: "2"},
},
},
{
Attributes: []sdp.Attribute{
{Key: "mid", Value: "5"},
},
},
},
}

media, ok := selectCandidateMediaSection(descr)
assert.True(t, ok)
assert.NotNil(t, media)
assert.NotNil(t, media.MediaDescription)
assert.Equal(t, "5", media.SDPMid)
assert.Equal(t, uint16(1), media.SDPMLineIndex)
})
}

func TestTrackDetailsFromSDP(t *testing.T) {

0 comments on commit 5edce95

Please sign in to comment.