From e175ef0e490965fbc3b3091cafee576feb0f130b Mon Sep 17 00:00:00 2001 From: Juliusz Chroboczek Date: Wed, 10 Jun 2020 20:25:25 +0200 Subject: [PATCH] Replace changed connections. We used to destroy and recreate connections, we now replace them atomically. --- static/sfu.js | 76 +++++++++++++++++++++++++++++++++++++-------------- webclient.go | 60 ++++++++++++++++++++++++---------------- 2 files changed, 92 insertions(+), 44 deletions(-) diff --git a/static/sfu.js b/static/sfu.js index 9872436..9bec93f 100644 --- a/static/sfu.js +++ b/static/sfu.js @@ -139,14 +139,10 @@ document.getElementById('unpresentbutton').onclick = function(e) { }; function changePresentation() { - let found = false; - for(let id in up) { - if(up[id].kind === 'local') - found = true; + let id = findUpMedia('local'); + if(id) { + addLocalMedia(id); } - delUpMediaKind('local'); - if(found) - addLocalMedia(); } function setVisibility(id, visible) { @@ -158,8 +154,8 @@ function setVisibility(id, visible) { } function setButtonsVisibility() { - let local = findUpMedia('local'); - let share = findUpMedia('screenshare') + let local = !!findUpMedia('local'); + let share = !!findUpMedia('screenshare') // don't allow multiple presentations setVisibility('presentbutton', permissions.present && !local); setVisibility('unpresentbutton', local); @@ -344,15 +340,21 @@ async function setMediaChoices() { mediaChoicesDone = true; } -async function addLocalMedia() { +async function addLocalMedia(id) { if(!getUserPass()) return; let audio = mapMediaOption(document.getElementById('audioselect').value); let video = mapMediaOption(document.getElementById('videoselect').value); - if(!audio && !video) + if(!audio && !video) { + if(id) + delUpMedia(id); return; + } + + if(id) + stopUpMedia(id); let constraints = {audio: audio, video: video}; let stream = null; @@ -360,13 +362,16 @@ async function addLocalMedia() { stream = await navigator.mediaDevices.getUserMedia(constraints); } catch(e) { console.error(e); + if(id) + delUpMedia(id); return; } setMediaChoices(); - let id = await newUpStream(); + id = await newUpStream(id); let c = up[id]; + c.kind = 'local'; c.stream = stream; stream.getTracks().forEach(t => { @@ -418,16 +423,32 @@ async function addShareMedia(setup) { setButtonsVisibility() } +function stopUpMedia(id) { + let c = up[id]; + if(!c) { + console.error('Stopping unknown up media'); + return; + } + if(!c.stream) + return; + c.stream.getTracks().forEach(t => { + try { + t.stop(); + } catch(e) { + } + }); +} + function delUpMedia(id) { let c = up[id]; if(!c) { - console.error("Deleting unknown up media"); + console.error('Deleting unknown up media'); return; } - c.close(true); + stopUpMedia(id); delMedia(id); + c.close(true); delete(up[id]); - setButtonsVisibility() } @@ -447,9 +468,9 @@ function delUpMediaKind(kind) { function findUpMedia(kind) { for(let id in up) { if(up[id].kind === kind) - return true; + return id; } - return false; + return null; } function muteLocalTracks(mute) { @@ -680,6 +701,14 @@ function sendRequest(value) { async function gotOffer(id, labels, offer) { let c = down[id]; + if(c) { + // a new offer with a known id does not indicate renegotiation, + // but a new connection that replaces the old one. + delete(down[id]) + c.close(false); + c = null; + } + if(!c) { let pc = new RTCPeerConnection({ iceServers: iceServers, @@ -1113,15 +1142,20 @@ function chatResizer(e) { document.getElementById('resizer').addEventListener('mousedown', chatResizer, false); -async function newUpStream() { - let id = randomid(); - if(up[id]) - throw new Error('Eek!'); +async function newUpStream(id) { + if(!id) { + id = randomid(); + if(up[id]) + throw new Error('Eek!'); + } let pc = new RTCPeerConnection({ iceServers: iceServers, }); if(!pc) throw new Error("Couldn't create peer connection"); + if(up[id]) { + up[id].close(false); + } up[id] = new Connection(id, pc); pc.onnegotiationneeded = async e => { diff --git a/webclient.go b/webclient.go index 30d2c24..c29083d 100644 --- a/webclient.go +++ b/webclient.go @@ -221,10 +221,17 @@ func addUpConn(c *webClient, id string) (*rtpUpConnection, error) { if c.up == nil { c.up = make(map[string]*rtpUpConnection) } - if c.up[id] != nil || (c.down != nil && c.down[id] != nil) { + if c.down != nil && c.down[id] != nil { conn.pc.Close() return nil, errors.New("Adding duplicate connection") } + + old := c.up[id] + if old != nil { + decrementVideoTracks(c, old) + old.pc.Close() + } + c.up[id] = conn conn.pc.OnICECandidate(func(candidate *webrtc.ICECandidate) { @@ -248,7 +255,21 @@ func delUpConn(c *webClient, id string) bool { delete(c.up, id) c.mu.Unlock() + decrementVideoTracks(c, conn) + + go func(clients []client) { + for _, c := range clients { + c.pushConn(conn.id, nil, nil, "") + } + }(c.Group().getClients(c)) + + conn.pc.Close() + return true +} + +func decrementVideoTracks(c *webClient, conn *rtpUpConnection) { conn.mu.Lock() + defer conn.mu.Unlock() for _, track := range conn.tracks { if track.track.Kind() == webrtc.RTPCodecTypeVideo { count := atomic.AddUint32(&c.group.videoCount, @@ -259,17 +280,6 @@ func delUpConn(c *webClient, id string) bool { } } } - conn.mu.Unlock() - - go func(clients []client) { - for _, c := range clients { - c.pushConn(conn.id, nil, nil, "") - } - }(c.Group().getClients(c)) - - conn.pc.Close() - - return true } func getDownConn(c *webClient, id string) *rtpDownConnection { @@ -307,13 +317,18 @@ func addDownConn(c *webClient, id string, remote upConnection) (*rtpDownConnecti c.mu.Lock() defer c.mu.Unlock() + if c.up != nil && c.up[id] != nil { + conn.pc.Close() + return nil, errors.New("Adding duplicate connection") + } + if c.down == nil { c.down = make(map[string]*rtpDownConnection) } - if c.down[id] != nil || (c.up != nil && c.up[id] != nil) { - conn.pc.Close() - return nil, errors.New("Adding duplicate connection") + old := c.down[id] + if old != nil { + old.pc.Close() } conn.pc.OnICECandidate(func(candidate *webrtc.ICECandidate) { @@ -441,13 +456,9 @@ func sendICE(c *webClient, id string, candidate *webrtc.ICECandidate) error { } func gotOffer(c *webClient, id string, offer webrtc.SessionDescription, labels map[string]string) error { - var err error - up, ok := c.up[id] - if !ok { - up, err = addUpConn(c, id) - if err != nil { - return err - } + up, err := addUpConn(c, id) + if err != nil { + return err } if c.username != "" { up.label = c.username @@ -718,13 +729,16 @@ func clientLoop(c *webClient, conn *websocket.Conn) error { case a := <-c.actionCh: switch a := a.(type) { case pushConnAction: - found := delDownConn(c, a.id) if a.conn == nil { + found := delDownConn(c, a.id) if found { c.write(clientMessage{ Type: "close", Id: a.id, }) + } else { + log.Printf("Deleting unknown " + + "down connection") } continue }