From 8168c2a9e64ed63a92d25f602c2e00f19156f245 Mon Sep 17 00:00:00 2001 From: Juliusz Chroboczek Date: Thu, 14 Jan 2021 14:56:15 +0100 Subject: [PATCH] Rework the up connection state machine. It is now more similar to the down connection, using the onclose callback for resource management. --- README.FRONTEND | 13 ++++--- static/galene.js | 84 ++++++++++++++++++++-------------------------- static/protocol.js | 36 +++++++++++--------- 3 files changed, 63 insertions(+), 70 deletions(-) diff --git a/README.FRONTEND b/README.FRONTEND index cc75e8e..ef6427d 100644 --- a/README.FRONTEND +++ b/README.FRONTEND @@ -40,11 +40,10 @@ serverConnection.ondownstream = ...; ``` The `onconnected` callback is called when we connect to the server. The -`onclose` callback is called when the socket is closed; you should use it -to close all your up streams (down streams will be closed by the server). -The `onusermessage` callback indicates an application-specific message, -either from another user or from the server; the field `kind` indicates -the kind of message. +`onclose` callback is called when the socket is closed; all streams will +have been closed by the time it is called. The `onusermessage` callback +indicates an application-specific message, either from another user or +from the server; the field `kind` indicates the kind of message. Once you have joined a group (see below), the remaining callbacks may trigger. The `onuser` callback is used to indicate that a user has joined @@ -126,8 +125,8 @@ a change in the status of the stream; states `connected` and `complete` indicate a functioning stream; other states indicate that the stream is not working right now but might recover in the future. -The `onclose` callback is called when the stream is destroyed by the -server. +The `onclose` callback is called when the stream is destroyed, either by +the server or in response to a call to the `close` method. ## Pushing outgoing video streams diff --git a/static/galene.js b/static/galene.js index e24b329..2d1177f 100644 --- a/static/galene.js +++ b/static/galene.js @@ -309,7 +309,6 @@ function gotConnected() { * @param {string} reason */ function gotClose(code, reason) { - delUpMediaKind(null); setConnected(false); if(code != 1000) { console.warn('Socket close', code, reason); @@ -374,7 +373,7 @@ getButtonElement('presentbutton').onclick = async function(e) { getButtonElement('unpresentbutton').onclick = function(e) { e.preventDefault(); - delUpMediaKind('local'); + closeUpMediaKind('local'); resizePeers(); }; @@ -482,13 +481,13 @@ document.getElementById('sharebutton').onclick = function(e) { document.getElementById('unsharebutton').onclick = function(e) { e.preventDefault(); - delUpMediaKind('screenshare'); + closeUpMediaKind('screenshare'); resizePeers(); }; document.getElementById('stopvideobutton').onclick = function(e) { e.preventDefault(); - delUpMediaKind('video'); + closeUpMediaKind('video'); resizePeers(); }; @@ -757,10 +756,6 @@ function newUpStream(id) { c.onerror = function(e) { console.error(e); displayError(e); - delUpMedia(c); - }; - c.onabort = function() { - delUpMedia(c); }; c.onnegotiationcompleted = function() { setMaxVideoThroughput(c, getMaxVideoThroughput()); @@ -928,23 +923,16 @@ function setFilter(c, f) { c.userdata.filter.stop(); c.userdata.filter = null; } - if(!c.onclose) { - console.warn("onclose not set, this shouldn't happen"); - c.onclose = null; - } return } if(c.userdata.filter) setFilter(c, null); - if(c.onclose) - throw new Error('onclose already taken'); if(f.inputStream != c.stream) throw new Error('Setting filter for wrong stream'); c.stream = f.outputStream; c.userdata.filter = f; - c.onclose = () => setFilter(c, null); } /** @@ -1028,13 +1016,10 @@ async function addLocalMedia(id) { if(!audio && !video) { if(old) - delUpMedia(old); + old.close(); return; } - if(old) - stopUpMedia(old); - let constraints = {audio: audio, video: video}; /** @type {MediaStream} */ let stream = null; @@ -1043,7 +1028,7 @@ async function addLocalMedia(id) { } catch(e) { displayError(e); if(old) - delUpMedia(old); + old.close(); return; } @@ -1058,8 +1043,22 @@ async function addLocalMedia(id) { try { let f = new Filter(stream, filter); setFilter(c, f); + c.onclose = () => { + stopStream(stream); + setFilter(c, null); + delMedia(c.id); + } } catch(e) { displayWarning(e); + c.onclose = () => { + stopStream(c.stream); + delMedia(c.id); + } + } + } else { + c.onclose = () => { + stopStream(c.stream); + delMedia(c.id); } } @@ -1110,11 +1109,13 @@ async function addShareMedia() { let c = newUpStream(); c.kind = 'screenshare'; c.stream = stream; + c.onclose = () => { + stopStream(stream); + delMedia(c.id); + } stream.getTracks().forEach(t => { c.pc.addTrack(t, stream); - t.onended = e => { - delUpMedia(c); - }; + t.onended = e => c.close(); c.labels[t.id] = 'screenshare'; }); c.onstats = gotUpStats; @@ -1144,12 +1145,14 @@ async function addFileMedia(file) { c.kind = 'video'; c.stream = stream; c.onclose = function() { + stopStream(c.stream); let media = /** @type{HTMLVideoElement} */ (document.getElementById('media-' + this.id)); if(media && media.src) { URL.revokeObjectURL(media.src); media.src = null; } + delMedia(c.id); } stream.onaddtrack = function(e) { @@ -1186,7 +1189,7 @@ async function addFileMedia(file) { if(Object.keys(c.labels).length === 0) { stream.onaddtrack = null; stream.onremovetrack = null; - delUpMedia(c); + c.close(); } }; await setMedia(c, true, false, video); @@ -1195,13 +1198,10 @@ async function addFileMedia(file) { } /** - * @param {Stream} c + * @param {MediaStream} s */ -function stopUpMedia(c) { - setFilter(c, null); - if(!c.stream) - return; - c.stream.getTracks().forEach(t => { +function stopStream(s) { + s.getTracks().forEach(t => { try { t.stop(); } catch(e) { @@ -1211,31 +1211,18 @@ function stopUpMedia(c) { } /** - * @param {Stream} c - */ -function delUpMedia(c) { - stopUpMedia(c); - c.close(); - delMedia(c.id); - delete(serverConnection.up[c.id]); - setButtonsVisibility(); -} - -/** - * delUpMediaKind reoves all up media of the given kind. If kind is - * falseish, it removes all up media. + * closeUpMediaKind closes all up connections that correspond to a given + * kind of media. If kind is null, it closes all up connections. + * * @param {string} kind */ -function delUpMediaKind(kind) { +function closeUpMediaKind(kind) { for(let id in serverConnection.up) { let c = serverConnection.up[id]; if(kind && c.kind != kind) continue - delUpMedia(c); + c.close(); } - - setButtonsVisibility(); - hideVideo(); } /** @@ -1501,6 +1488,7 @@ function delMedia(id) { media.srcObject = null; mediadiv.removeChild(peer); + setButtonsVisibility(); resizePeers(); hideVideo(); } diff --git a/static/protocol.js b/static/protocol.js index 794bba6..19b914c 100644 --- a/static/protocol.js +++ b/static/protocol.js @@ -238,12 +238,10 @@ ServerConnection.prototype.connect = async function(url) { sc.permissions = {}; for(let id in sc.up) { let c = sc.up[id]; - delete(sc.up[id]); c.close(); } for(let id in sc.down) { let c = sc.down[id]; - delete(sc.down[id]); c.close(); } if(sc.group && sc.onjoined) @@ -408,9 +406,9 @@ ServerConnection.prototype.newUpStream = function(id) { let pc = new RTCPeerConnection(sc.rtcConfiguration); if(!pc) throw new Error("Couldn't create peer connection"); - if(sc.up[id]) { + if(sc.up[id]) sc.up[id].close(); - } + let c = new Stream(this, id, pc, true); sc.up[id] = c; @@ -530,7 +528,6 @@ ServerConnection.prototype.gotOffer = async function(id, labels, source, usernam // SDP is rather inflexible as to what can be renegotiated. // Unless the server indicates that this is a renegotiation with // all parameters unchanged, tear down the existing connection. - delete(sc.down[id]); c.close(true); c = null; } @@ -669,7 +666,6 @@ ServerConnection.prototype.gotClose = function(id) { let c = this.down[id]; if(!c) throw new Error('unknown down stream'); - delete(this.down[id]); c.close(); }; @@ -888,19 +884,17 @@ function Stream(sc, id, pc, up) { */ Stream.prototype.close = function(nocallback) { let c = this; + + if(!c.sc) { + console.warn('Closing closed stream'); + return; + } + if(c.statsHandler) { clearInterval(c.statsHandler); c.statsHandler = null; } - if(c.stream) { - c.stream.getTracks().forEach(t => { - try { - t.stop(); - } catch(e) { - } - }); - } c.pc.close(); if(c.up && c.localDescriptionSent) { @@ -913,9 +907,21 @@ Stream.prototype.close = function(nocallback) { } } + if(c.up) { + if(c.sc.up[c.id] === c) + delete(c.sc.up[c.id]); + else + console.warn('Closing unknown stream'); + } else { + if(c.sc.down[c.id] === c) + delete(c.sc.down[c.id]); + else + console.warn('Closing unknown stream'); + } + c.sc = null; + if(!nocallback && c.onclose) c.onclose.call(c); - c.sc = null; }; /**