From e3b67a697e49672baf4ae24ed98f74203ec24687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Tani=C3=A7a?= Date: Wed, 21 Jun 2023 22:03:17 -0600 Subject: [PATCH] Fix MediaStream remote close by using an aux RTCDataChannel (#963) ### Problem `MediaConnection.close()` doesn't propagate the `close` event to the remote peer. ### Solution The proposed solution uses a similar approach to the `DataConnection`, where an aux data channel is created for the connection. This way, when we `MediaConnection.close()` the data channel will be closed and the `close` signal will be propagated to the remote peer. #### Notes I was not sure if there was another cleaner way of achieving this, without the extra data channel, but this seems to work pretty well (at least until a better solution comes up). This should fix: https://github.com/peers/peerjs/issues/636 --------- Co-authored-by: Jonas Gloning <34194370+jonasgloning@users.noreply.github.com> Closes #636, Closes #1089, Closes #1032, Closes #832, Closes #780, Closes #653 --- e2e/mediachannel/close.html | 38 ++++++++++++++++++ e2e/mediachannel/close.js | 71 ++++++++++++++++++++++++++++++++++ e2e/mediachannel/close.page.ts | 61 +++++++++++++++++++++++++++++ e2e/mediachannel/close.spec.ts | 24 ++++++++++++ lib/baseconnection.ts | 12 ++++++ lib/dataconnection.ts | 10 +---- lib/mediaconnection.ts | 15 +++++++ lib/negotiator.ts | 29 ++++++-------- 8 files changed, 235 insertions(+), 25 deletions(-) create mode 100644 e2e/mediachannel/close.html create mode 100644 e2e/mediachannel/close.js create mode 100644 e2e/mediachannel/close.page.ts create mode 100644 e2e/mediachannel/close.spec.ts diff --git a/e2e/mediachannel/close.html b/e2e/mediachannel/close.html new file mode 100644 index 000000000..595a0dbec --- /dev/null +++ b/e2e/mediachannel/close.html @@ -0,0 +1,38 @@ + + + + + + + + + +

MediaChannel

+ + + +
+ + + +
+
+
+
+ + + + + diff --git a/e2e/mediachannel/close.js b/e2e/mediachannel/close.js new file mode 100644 index 000000000..8c1db5767 --- /dev/null +++ b/e2e/mediachannel/close.js @@ -0,0 +1,71 @@ +/** + * @type {typeof import("../peerjs").Peer} + */ +const Peer = window.Peer; + +document.getElementsByTagName("title")[0].innerText = + window.location.hash.substring(1); + +const callBtn = document.getElementById("call-btn"); +console.log(callBtn); +const receiverIdInput = document.getElementById("receiver-id"); +const closeBtn = document.getElementById("close-btn"); +const messages = document.getElementById("messages"); +const errorMessage = document.getElementById("error-message"); + +const stream = window["sender-stream"].captureStream(30); +// const stream = await navigator.mediaDevices.getUserMedia({video: true, audio: true}) +const peer = new Peer({ debug: 3 }); +/** + * @type {import("peerjs").MediaConnection} + */ +let mediaConnection; +peer + .once("open", (id) => { + messages.textContent = `Your Peer ID: ${id}`; + }) + .once("error", (error) => { + errorMessage.textContent = JSON.stringify(error); + }) + .once("call", (call) => { + mediaConnection = call; + mediaConnection.on("stream", function (stream) { + console.log("stream", stream); + const video = document.getElementById("receiver-stream"); + console.log("video element", video); + video.srcObject = stream; + video.play(); + }); + mediaConnection.once("close", () => { + messages.textContent = "Closed!"; + }); + call.answer(stream); + messages.innerText = "Connected!"; + }); + +callBtn.addEventListener("click", async () => { + console.log("calling"); + + /** @type {string} */ + const receiverId = receiverIdInput.value; + if (receiverId) { + mediaConnection = peer.call(receiverId, stream); + mediaConnection.on("stream", (stream) => { + console.log("stream", stream); + const video = document.getElementById("receiver-stream"); + console.log("video element", video); + video.srcObject = stream; + video.play(); + messages.innerText = "Connected!"; + }); + mediaConnection.on("close", () => { + messages.textContent = "Closed!"; + }); + } +}); + +closeBtn.addEventListener("click", () => { + mediaConnection.close(); +}); + +callBtn.disabled = false; diff --git a/e2e/mediachannel/close.page.ts b/e2e/mediachannel/close.page.ts new file mode 100644 index 000000000..30412ae15 --- /dev/null +++ b/e2e/mediachannel/close.page.ts @@ -0,0 +1,61 @@ +import { browser, $ } from "@wdio/globals"; +class SerializationPage { + get receiverId() { + return $("input[id='receiver-id']"); + } + get callBtn() { + return $("button[id='call-btn']"); + } + + get messages() { + return $("div[id='messages']"); + } + + get closeBtn() { + return $("button[id='close-btn']"); + } + + get errorMessage() { + return $("div[id='error-message']"); + } + + get result() { + return $("div[id='result']"); + } + + waitForMessage(right: string) { + return browser.waitUntil( + async () => { + const messages = await this.messages.getText(); + return messages.startsWith(right); + }, + { timeoutMsg: `Expected message to start with ${right}`, timeout: 10000 }, + ); + } + + async open() { + await browser.switchWindow("Alice"); + await browser.url(`/e2e/mediachannel/close.html#Alice`); + await this.callBtn.waitForEnabled(); + + await browser.switchWindow("Bob"); + await browser.url(`/e2e/mediachannel/close.html#Bob`); + await this.callBtn.waitForEnabled(); + } + async init() { + await browser.url("/e2e/alice.html"); + await browser.waitUntil(async () => { + const title = await browser.getTitle(); + return title === "Alice"; + }); + await browser.pause(1000); + await browser.newWindow("/e2e/bob.html"); + await browser.waitUntil(async () => { + const title = await browser.getTitle(); + return title === "Bob"; + }); + await browser.pause(1000); + } +} + +export default new SerializationPage(); diff --git a/e2e/mediachannel/close.spec.ts b/e2e/mediachannel/close.spec.ts new file mode 100644 index 000000000..d6fc8bba6 --- /dev/null +++ b/e2e/mediachannel/close.spec.ts @@ -0,0 +1,24 @@ +import P from "./close.page.js"; +import { browser } from "@wdio/globals"; + +fdescribe("MediaStream", () => { + beforeAll(async () => { + await P.init(); + }); + fit("should close the remote stream", async () => { + await P.open(); + await P.waitForMessage("Your Peer ID: "); + const bobId = (await P.messages.getText()).split("Your Peer ID: ")[1]; + await browser.switchWindow("Alice"); + await P.waitForMessage("Your Peer ID: "); + await P.receiverId.setValue(bobId); + await P.callBtn.click(); + await P.waitForMessage("Connected!"); + await browser.switchWindow("Bob"); + await P.waitForMessage("Connected!"); + await P.closeBtn.click(); + await P.waitForMessage("Closed!"); + await browser.switchWindow("Alice"); + await P.waitForMessage("Closed!"); + }); +}); diff --git a/lib/baseconnection.ts b/lib/baseconnection.ts index 855d67a41..7512b7efa 100644 --- a/lib/baseconnection.ts +++ b/lib/baseconnection.ts @@ -34,9 +34,15 @@ export abstract class BaseConnection< connectionId: string; peerConnection: RTCPeerConnection; + abstract get dataChannel(): RTCDataChannel; abstract get type(): ConnectionType; + /** + * The optional label passed in or assigned by PeerJS when the connection was initiated. + */ + abstract readonly label: string; + /** * Whether the media connection is active (e.g. your call has been answered). * You can check this if you want to set a maximum wait time for a one-sided call. @@ -64,4 +70,10 @@ export abstract class BaseConnection< * @internal */ abstract handleMessage(message: ServerMessage): void; + + /** + * Called by the Negotiator when the DataChannel is ready. + * @internal + * */ + abstract _initializeDataChannel(dc: RTCDataChannel): void; } diff --git a/lib/dataconnection.ts b/lib/dataconnection.ts index e1a35d63d..e6d532ef1 100644 --- a/lib/dataconnection.ts +++ b/lib/dataconnection.ts @@ -39,10 +39,8 @@ export class DataConnection private static readonly MAX_BUFFERED_AMOUNT = 8 * 1024 * 1024; private _negotiator: Negotiator; - /** - * The optional label passed in or assigned by PeerJS when the connection was initiated. - */ readonly label: string; + /** * The serialization format of the data sent over the connection. * {@apilink SerializationType | possible values} @@ -119,12 +117,8 @@ export class DataConnection } /** Called by the Negotiator when the DataChannel is ready. */ - initialize(dc: RTCDataChannel): void { + override _initializeDataChannel(dc: RTCDataChannel): void { this._dc = dc; - this._configureDataChannel(); - } - - private _configureDataChannel(): void { if (!util.supports.binaryBlob || util.supports.reliable) { this.dataChannel.binaryType = "arraybuffer"; } diff --git a/lib/mediaconnection.ts b/lib/mediaconnection.ts index 1a270a0c8..d10c26ac0 100644 --- a/lib/mediaconnection.ts +++ b/lib/mediaconnection.ts @@ -24,10 +24,12 @@ export type MediaConnectionEvents = { */ export class MediaConnection extends BaseConnection { private static readonly ID_PREFIX = "mc_"; + readonly label: string; private _negotiator: Negotiator; private _localStream: MediaStream; private _remoteStream: MediaStream; + private _dc: RTCDataChannel; /** * For media connections, this is always 'media'. @@ -43,6 +45,10 @@ export class MediaConnection extends BaseConnection { return this._remoteStream; } + get dataChannel(): RTCDataChannel { + return this._dc; + } + constructor(peerId: string, provider: Peer, options: any) { super(peerId, provider, options); @@ -61,6 +67,15 @@ export class MediaConnection extends BaseConnection { } } + /** Called by the Negotiator when the DataChannel is ready. */ + override _initializeDataChannel(dc: RTCDataChannel): void { + this._dc = dc; + + this.dataChannel.onclose = () => { + logger.log(`DC#${this.connectionId} dc closed for:`, this.peer); + this.close(); + }; + } addStream(remoteStream) { logger.log("Receiving stream", remoteStream); diff --git a/lib/negotiator.ts b/lib/negotiator.ts index eccc419fb..83c9093c8 100644 --- a/lib/negotiator.ts +++ b/lib/negotiator.ts @@ -28,17 +28,15 @@ export class Negotiator< // What do we need to do now? if (options.originator) { - if (this.connection.type === ConnectionType.Data) { - const dataConnection = (this.connection); + const dataConnection = this.connection; - const config: RTCDataChannelInit = { ordered: !!options.reliable }; + const config: RTCDataChannelInit = { ordered: !!options.reliable }; - const dataChannel = peerConnection.createDataChannel( - dataConnection.label, - config, - ); - dataConnection.initialize(dataChannel); - } + const dataChannel = peerConnection.createDataChannel( + dataConnection.label, + config, + ); + dataConnection._initializeDataChannel(dataChannel); this._makeOffer(); } else { @@ -136,7 +134,7 @@ export class Negotiator< provider.getConnection(peerId, connectionId) ); - connection.initialize(dataChannel); + connection._initializeDataChannel(dataChannel); }; // MEDIACONNECTION. @@ -177,14 +175,11 @@ export class Negotiator< const peerConnectionNotClosed = peerConnection.signalingState !== "closed"; let dataChannelNotClosed = false; - if (this.connection.type === ConnectionType.Data) { - const dataConnection = (this.connection); - const dataChannel = dataConnection.dataChannel; + const dataChannel = this.connection.dataChannel; - if (dataChannel) { - dataChannelNotClosed = - !!dataChannel.readyState && dataChannel.readyState !== "closed"; - } + if (dataChannel) { + dataChannelNotClosed = + !!dataChannel.readyState && dataChannel.readyState !== "closed"; } if (peerConnectionNotClosed || dataChannelNotClosed) {