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: #636 --------- Co-authored-by: Jonas Gloning <[email protected]> Closes #636, Closes #1089, Closes #1032, Closes #832, Closes #780, Closes #653
peers · Jun 22, 2023 · e3b67a6 · e3b67a6
1 parent 9e8b6e8
commit e3b67a6
Show file tree

Hide file tree

Showing 8 changed files with 235 additions and 25 deletions.
diff --git a/e2e/mediachannel/close.html b/e2e/mediachannel/close.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html lang="en">
+	<head>
+		<meta charset="UTF-8" />
+		<meta name="viewport" content="width=device-width, initial-scale=1.0" />
+		<title></title>
+		<link rel="stylesheet" href="../style.css" />
+	</head>
+	<body>
+		<h1>MediaChannel</h1>
+		<canvas id="sender-stream" width="200" height="100"></canvas>
+		<video id="receiver-stream" autoplay></video>
+		<script>
+			const canvas = document.getElementById("sender-stream");
+			const ctx = canvas.getContext("2d");
+
+			// Set the canvas background color to white
+			ctx.fillStyle = "white";
+			ctx.fillRect(0, 0, canvas.width, canvas.height);
+
+			// Draw the text "Alice" in black
+			ctx.font = "30px sans-serif";
+			ctx.fillStyle = "black";
+			ctx.fillText(window.location.hash.substring(1), 50, 50);
+		</script>
+		<div id="inputs">
+			<input type="text" id="receiver-id" placeholder="Receiver ID" />
+			<button id="call-btn" disabled>Call</button>
+			<button id="close-btn">Hang up</button>
+		</div>
+		<div id="messages"></div>
+		<div id="result"></div>
+		<div id="error-message"></div>
+		<video></video>
+		<script src="/dist/peerjs.js"></script>
+		<script src="close.js" type="module"></script>
+	</body>
+</html>
diff --git a/e2e/mediachannel/close.js 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
@@ -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
@@ -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
@@ -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
@@ -39,10 +39,8 @@ export class DataConnection
 	private static readonly MAX_BUFFERED_AMOUNT = 8 * 1024 * 1024;
 
 	private _negotiator: Negotiator<DataConnectionEvents, DataConnection>;
-	/**
-	 * 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
@@ -24,10 +24,12 @@ export type MediaConnectionEvents = {
  */
 export class MediaConnection extends BaseConnection<MediaConnectionEvents> {
 	private static readonly ID_PREFIX = "mc_";
+	readonly label: string;
 
 	private _negotiator: Negotiator<MediaConnectionEvents, MediaConnection>;
 	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<MediaConnectionEvents> {
 		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<MediaConnectionEvents> {
 		}
 	}
 
+	/** 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
@@ -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 = <DataConnection>(<unknown>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 = <DataConnection>(<unknown>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) {