From 8fa1c2c54a7819352e5bbf159be378e90ba061af Mon Sep 17 00:00:00 2001 From: Joe Turki Date: Tue, 7 Jan 2025 19:09:36 -0600 Subject: [PATCH] Add sdpMid and sdpMLineIndex to ICECandidates --- icecandidate.go | 34 +++---- icecandidate_test.go | 65 ++++++++++++++ icegatherer.go | 32 ++++++- icegatherer_test.go | 69 ++++++++++++++ icetransport.go | 6 +- peerconnection.go | 19 ++-- peerconnection_go_test.go | 183 ++++++++++++++++++++++++++++++++++++++ peerconnection_js.go | 2 +- sdp.go | 110 ++++++++++++++--------- sdp_test.go | 139 ++++++++++++++++++++++++----- 10 files changed, 567 insertions(+), 92 deletions(-) diff --git a/icecandidate.go b/icecandidate.go index c36d520543a..7080272575b 100644 --- a/icecandidate.go +++ b/icecandidate.go @@ -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, } } diff --git a/icecandidate_test.go b/icecandidate_test.go index 8b3fa132254..c553f2c1882 100644 --- a/icecandidate_test.go +++ b/icecandidate_test.go @@ -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)) +} diff --git a/icegatherer.go b/icegatherer.go index b15c72b545d..9cb4acac464 100644 --- a/icegatherer.go +++ b/icegatherer.go @@ -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 diff --git a/icegatherer_test.go b/icegatherer_test.go index 6327824286c..6f7bd8db553 100644 --- a/icegatherer_test.go +++ b/icegatherer_test.go @@ -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()) +} diff --git a/icetransport.go b/icetransport.go index 448929ca705..a78f8b76a6b 100644 --- a/icetransport.go +++ b/icetransport.go @@ -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 diff --git a/peerconnection.go b/peerconnection.go index 6c518013fd0..dc61b2b54ae 100644 --- a/peerconnection.go +++ b/peerconnection.go @@ -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,12 +1156,12 @@ 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 { @@ -1164,13 +1169,13 @@ func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error { } } - 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 } diff --git a/peerconnection_go_test.go b/peerconnection_go_test.go index 3de9acf5f2a..c7d15bc7562 100644 --- a/peerconnection_go_test.go +++ b/peerconnection_go_test.go @@ -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()) +} diff --git a/peerconnection_js.go b/peerconnection_js.go index 493afa02a08..c7e968ca257 100644 --- a/peerconnection_js.go +++ b/peerconnection_js.go @@ -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 } diff --git a/sdp.go b/sdp.go index 10a24edc405..b07a55326c9 100644 --- a/sdp.go +++ b/sdp.go @@ -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 { diff --git a/sdp_test.go b/sdp_test.go index 46a3b8de504..e704a057c55 100644 --- a/sdp_test.go +++ b/sdp_test.go @@ -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) {