Initial Support for Spacebar WebRTC #1108

CE1CECL · 2023-12-28T03:37:03Z

please note: WebRTC was ONLY tested using a test client that was since deleted by spacebar owners, no other clients were tested, nor bot libraries were either (discord.js didn't work for me since it always requires a https connection)

NOTE: This PR DOES NOT add Screen sharing, but adds webcam support and voice
~~BREAKING CHANGES: Windows can no longer be supported after this PR is merged, because medooze-media-server doesn't support windows~~ WebRTC can now be disabled with a env var to continue support for Windows

Known bugs:

Muting, unmuting, deafening, and undeafening makes the voice call reconnect for unknown reason.
Defend does not work on test client because of the reconnect bug.
Video support can be buggy, requiring both cameras on to fully work (why?)
You tell me what other issues there are: this has been a worked on thing by Me, @MaddyUnderStars & was initially created by @SamuelScheit

src/api/Server.ts

TheArcaneBrony

Needs a tiny amount of work :)

.gitignore

nginx.conf

src/api/Server.ts

tmp/HOST

tmp/IPv4

tmp/NAME

tmp/PORT

tmp/PROT

Puyodead1 · 2023-12-28T19:46:12Z

if there isn't already, there should be a way to disable webrtc from loading, because I use windows for development, and I am not switching anytime soon. either config, or at the very least an environment variable

TheArcaneBrony · 2023-12-28T20:28:27Z

Worst case you could use WSL I guess?
But yeah, this is probably important, even if Windows remains a minimally supported OS.

Signed-off-by: Christopher Lentocha <[email protected]>

CE1CECL · 2023-12-29T18:41:19Z

if there isn't already, there should be a way to disable webrtc from loading, because I use windows for development, and I am not switching anytime soon. either config, or at the very least an environment variable

Added but it needs testing directly on Windows, which I don't have, I have Linux.

CE1CECL · 2023-12-29T18:43:45Z

Worst case you could use WSL I guess? But yeah, this is probably important, even if Windows remains a minimally supported OS.

No. WSL Is NOT a option, WSL requires Hyper-v which is garbage in general,, slow, etc... WSL is more like emulation than a layer like WineHQ

TheArcaneBrony · 2023-12-30T13:54:07Z

what? Wine is a stdlib overlay + bincompat layer, WSL is a full on vm (and it isnt slow either)

CE1CECL · 2023-12-30T21:39:10Z

Marked as ready since its possible to disable webrtc

MaddyUnderStars · 2024-01-02T10:31:15Z

src/bundle/WinServer.ts

You don't need to include a separate file for windows, you could use dynamic imports or just, a top level if statement?

You don't need to include a separate file for windows, you could use dynamic imports or just, a top level if statement?

I tried, it doesn't seem to work out, because import is required to be used (const require doesn't seem to work) and needs to be at the top without an "if" statement. What I did in my last commit was the easiest and probably cleanest, and understandable by someone else.

You can just (if you want)
diff -aprNu src/bundle/Server.ts src/bundle/WinServer.ts

MaddyUnderStars · 2024-01-02T10:33:43Z

src/gateway/opcodes/VoiceStateUpdate.ts

-			guild_id: voiceState.guild_id,
+			user_id: this.user_id,


I believe this is incorrect. That parameter is used to determine the recipients of this gateway event. Your edit only sends the event to the user who left the channel, which means other users in the voice channel won't be updated.

MaddyUnderStars · 2024-01-02T10:34:37Z

src/util/entities/VoiceState.ts

@@ -84,7 +84,7 @@ export class VoiceState extends BaseClass {
 	@Column({ nullable: true })
 	self_stream?: boolean;

-	@Column()
+	@Column({ nullable: true })


This doesn't need to be nullable, it should just be set to false by default. Regardless, you didn't write migrations for this change.

This doesn't need to be nullable, it should just be set to false by default.

What do you mean by "false by default"?

Regardless, you didn't write migrations for this change.

Wasn't sure if I would keep the change but I will write mig's once the first question gets answered.

MaddyUnderStars · 2024-01-02T10:37:28Z

src/webrtc/Server.ts

-			server: this.server,
+			noServer: true,


Rory&, this was done so that the websocket server would not be responsible for upgrading http connections, which is now done above with the uncommented update listener. This was done so that voice ws is mounted on /voice

I don't think this is necessary though, I would rather it just use a different port? This makes routing through nginx less difficult when using bundle.

src/webrtc/events/Message.ts

src/webrtc/opcodes/BackendVersion.ts

src/webrtc/opcodes/Identify.ts

src/webrtc/opcodes/SelectProtocol.ts

src/webrtc/opcodes/index.ts

Signed-off-by: Christopher Lentocha <[email protected]>

MathMan05 · 2024-09-09T21:19:46Z

Worst case you could use WSL I guess? But yeah, this is probably important, even if Windows remains a minimally supported OS.

No. WSL Is NOT a option, WSL requires Hyper-v which is garbage in general,, slow, etc... WSL is more like emulation than a layer like WineHQ

WSL 2 needs Hyper-v, the original WSL is to windows what wine is to linux

TheArcaneBrony · 2024-10-19T07:54:04Z

honestly, hyper-v/WSL2 is completely fine, the slow part is the 9PFS mount for your windows drive

CE1CECL · 2024-11-22T13:41:37Z

Worst case you could use WSL I guess? But yeah, this is probably important, even if Windows remains a minimally supported OS.

No. WSL Is NOT a option, WSL requires Hyper-v which is garbage in general,, slow, etc... WSL is more like emulation than a layer like WineHQ

WSL 2 needs Hyper-v, the original WSL is to windows what wine is to linux

I wonder if the original WSL will work or not with this? It should, and if it does, then users should use that to enable webrtc until a real native fix comes out.

MathMan05 · 2024-11-22T15:37:16Z

this hasn't had work for quite a while, there's a different WebRTC branch now that has a different blocker, and most of us don't know enough rust to fix it

CE1CECL · 2024-11-22T15:47:20Z

this hasn't had work for quite a while, there's a different WebRTC branch now that has a different blocker, and most of us don't know enough rust to fix it

Ah, I see! Is there a reason this was re-done from scratch?

MathMan05 · 2024-11-22T15:48:17Z

not 100% sure tbh, I just want to see WebRTC working lol

CE1CECL · 2024-11-22T15:50:23Z

not 100% sure tbh, I just want to see WebRTC working lol

May I ask what on this PR isnt working exactly?

MathMan05 · 2024-11-22T16:49:04Z

not sure what is wrong with it, I'm not the person who deals with this stuff, I just make a client lol

dank074 · 2024-11-23T19:36:34Z

I just tried this now and it seems to work just fine on WSL at least for testing purposes for those who develop on Windows. Luckily VS Code makes it easy to develop on WSL. I don't know if anyone was ever planning to use Windows for production anyway.

Issues I found:

Like the PR description states, both cameras have be turned on, but even then, it shows you your own camera instead of the other user's camera. I think it might be doing the same for sound, it's just harder to test sound.
EDIT: It's a bug when calling attachTrack you are attaching it to the current client and not the other clients in the call. I changed it and now video is working as expected, even without having both cameras on
Reconnect bug was solved by puyo in his branch dev/webrtc (his changes on VoiceStateUpdate.ts seemed to solve the issue for this branch as well)
changes.patch
H264 seems to work for me. I just had to modify the SDP.json file and match Discord parameters
Codec id is hardcoded in SDP.json but it should be dynamic since it will differ between Firefox and Chrome. SelectProtocol message should determine the codec id
Cannot build Spacebar on Windows since medooze cannot be installed on Windows at all (node-gyp build fails)

I think a possible solution for that last bullet point is probably to make the webrtc server its own separate package which runs independently and communicates with the spacebar instance using RabbitMQ. This will mean that Spacebar will be webrtc-server implementation agnostic, and the end user can choose which implementation to pair with Spacebar. It will also mean that the webrtc server won't necessarily have to be a NodeJS server.

CE1CECL · 2024-11-24T18:49:30Z

I just tried this now and it seems to work just fine on WSL at least for testing purposes for those who develop on Windows. Luckily VS Code makes it easy to develop on WSL. I don't know if anyone was ever planning to use Windows for production anyway.

Issues I found:
* Like the PR description states, both cameras have be turned on, but even then, it shows you your own camera instead of the other user's camera. I think it might be doing the same for sound, it's just harder to test sound.
  EDIT: It's a bug when calling `attachTrack` you are attaching it to the current client and not the other clients in the call. I changed it and now video is working as expected, even without having both cameras on

May I ask what changes were exactly done?

* Reconnect bug was solved by puyo in his branch dev/webrtc (his changes on VoiceStateUpdate.ts seemed to solve the issue for this branch as well)
  [changes.patch](https://github.com/user-attachments/files/17882959/changes.patch)

I saw this is the same changes as the portion from 6beee77 thanks! I'll add it to the branch after I test it and fix the merge conflicts.

* H264 seems to work for me. I just had to modify the SDP.json file and match Discord parameters

* Codec id is hardcoded in SDP.json but it should be dynamic since it will differ between Firefox and Chrome. SelectProtocol message should determine the codec id

* Cannot build Spacebar on Windows since medooze cannot be installed on Windows at all (node-gyp build fails)
I think a possible solution for that last bullet point is probably to make the webrtc server its own separate package which runs independently and communicates with the spacebar instance using RabbitMQ. This will mean that Spacebar will be webrtc-server implementation agnostic, and the end user can choose which implementation to pair with Spacebar. It will also mean that the webrtc server won't necessarily have to be a NodeJS server.

CE1CECL · 2024-11-24T19:01:48Z

Unless you did this, this is my current guess:

diff --git a/src/webrtc/opcodes/Video.ts b/src/webrtc/opcodes/Video.ts
index aa97c18..c451063 100644
--- a/src/webrtc/opcodes/Video.ts
+++ b/src/webrtc/opcodes/Video.ts
@@ -76,7 +76,7 @@ function createStream(
                if (!client.in.stream) return;
 
                client.in.stream?.getTracks().forEach((track) => {
-                       attachTrack.call(this, track, client.websocket.user_id);
+                       attachTrack.call(this, track, this.user_id);
                });
        });
 }
@@ -165,7 +165,7 @@ function handleSSRC(this: WebSocket, type: "audio" | "video", ssrcs: SSRCs) {
                        if (client.websocket.user_id === this.user_id) return;
                        if (!client.out.stream) return;
 
-                       attachTrack.call(this, track, client.websocket.user_id);
+                       attachTrack.call(this, track, this.user_id);
                });
        }
 }

dank074 · 2024-11-24T20:04:15Z

Unless you did this, this is my current guess:

diff --git a/src/webrtc/opcodes/Video.ts b/src/webrtc/opcodes/Video.ts
index aa97c18..c451063 100644
--- a/src/webrtc/opcodes/Video.ts
+++ b/src/webrtc/opcodes/Video.ts
@@ -76,7 +76,7 @@ function createStream(
                if (!client.in.stream) return;
 
                client.in.stream?.getTracks().forEach((track) => {
-                       attachTrack.call(this, track, client.websocket.user_id);
+                       attachTrack.call(this, track, this.user_id);
                });
        });
 }
@@ -165,7 +165,7 @@ function handleSSRC(this: WebSocket, type: "audio" | "video", ssrcs: SSRCs) {
                        if (client.websocket.user_id === this.user_id) return;
                        if (!client.out.stream) return;
 
-                       attachTrack.call(this, track, client.websocket.user_id);
+                       attachTrack.call(this, track, this.user_id);
                });
        }
 }

I just did this

diff --git a/src/webrtc/opcodes/Video.ts b/src/webrtc/opcodes/Video.ts
index 1a288f8e..2d646d4b 100644
--- a/src/webrtc/opcodes/Video.ts
+++ b/src/webrtc/opcodes/Video.ts
@@ -165,7 +165,7 @@ function handleSSRC(this: WebSocket, type: "audio" | "video", ssrcs: SSRCs) {
 			if (client.websocket.user_id === this.user_id) return;
 			if (!client.out.stream) return;
 
-			attachTrack.call(this, track, client.websocket.user_id);
+			attachTrack.call(client.websocket, track, this.user_id);
 		});
 	}
 }

This comment was marked as resolved.

Sign in to view

MaddyUnderStars requested changes Dec 28, 2023

View reviewed changes

src/api/Server.ts Outdated Show resolved Hide resolved

TheArcaneBrony requested changes Dec 28, 2023

View reviewed changes

Initial Support for Spacebar WebRTC

1e0f8f9

Signed-off-by: Christopher Lentocha <[email protected]>

CE1CECL force-pushed the master branch from 25156df to 1e0f8f9 Compare December 28, 2023 23:46

CE1CECL changed the title ~~Initial Support for Spacebar WebRTC & Dynamic URL Support~~ Initial Support for Spacebar WebRTC Dec 28, 2023

CE1CECL added 3 commits December 28, 2023 18:48

Forgot to npm i

9ab328a

Signed-off-by: Christopher Lentocha <[email protected]>

Force known working version of WebRTC packages

261e555

Signed-off-by: Christopher Lentocha <[email protected]>

WEBRTC_DISABLE=1 support for Windows Users

036e3b2

Signed-off-by: Christopher Lentocha <[email protected]>

CE1CECL marked this pull request as ready for review December 30, 2023 21:38

MaddyUnderStars requested changes Jan 2, 2024

View reviewed changes

Force Disable WebRTC if Windows is detected

b57ac74

Signed-off-by: Christopher Lentocha <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial Support for Spacebar WebRTC #1108

Initial Support for Spacebar WebRTC #1108

CE1CECL commented Dec 28, 2023 •

edited

Loading

This comment was marked as resolved.

TheArcaneBrony left a comment

Puyodead1 commented Dec 28, 2023

TheArcaneBrony commented Dec 28, 2023

CE1CECL commented Dec 29, 2023

CE1CECL commented Dec 29, 2023

TheArcaneBrony commented Dec 30, 2023

CE1CECL commented Dec 30, 2023

MaddyUnderStars Jan 2, 2024

CE1CECL Jan 5, 2024

MaddyUnderStars Jan 2, 2024

MaddyUnderStars Jan 2, 2024

CE1CECL Jan 5, 2024 •

edited

Loading

MaddyUnderStars Jan 2, 2024

MathMan05 commented Sep 9, 2024

TheArcaneBrony commented Oct 19, 2024

CE1CECL commented Nov 22, 2024

MathMan05 commented Nov 22, 2024

CE1CECL commented Nov 22, 2024

MathMan05 commented Nov 22, 2024

CE1CECL commented Nov 22, 2024

MathMan05 commented Nov 22, 2024

dank074 commented Nov 23, 2024 •

edited

Loading

CE1CECL commented Nov 24, 2024

CE1CECL commented Nov 24, 2024

dank074 commented Nov 24, 2024

Initial Support for Spacebar WebRTC #1108

Are you sure you want to change the base?

Initial Support for Spacebar WebRTC #1108

Conversation

CE1CECL commented Dec 28, 2023 • edited Loading

This comment was marked as resolved.

TheArcaneBrony left a comment

Choose a reason for hiding this comment

Puyodead1 commented Dec 28, 2023

TheArcaneBrony commented Dec 28, 2023

CE1CECL commented Dec 29, 2023

CE1CECL commented Dec 29, 2023

TheArcaneBrony commented Dec 30, 2023

CE1CECL commented Dec 30, 2023

MaddyUnderStars Jan 2, 2024

Choose a reason for hiding this comment

CE1CECL Jan 5, 2024

Choose a reason for hiding this comment

MaddyUnderStars Jan 2, 2024

Choose a reason for hiding this comment

MaddyUnderStars Jan 2, 2024

Choose a reason for hiding this comment

CE1CECL Jan 5, 2024 • edited Loading

Choose a reason for hiding this comment

MaddyUnderStars Jan 2, 2024

Choose a reason for hiding this comment

MathMan05 commented Sep 9, 2024

TheArcaneBrony commented Oct 19, 2024

CE1CECL commented Nov 22, 2024

MathMan05 commented Nov 22, 2024

CE1CECL commented Nov 22, 2024

MathMan05 commented Nov 22, 2024

CE1CECL commented Nov 22, 2024

MathMan05 commented Nov 22, 2024

dank074 commented Nov 23, 2024 • edited Loading

CE1CECL commented Nov 24, 2024

CE1CECL commented Nov 24, 2024

dank074 commented Nov 24, 2024

CE1CECL commented Dec 28, 2023 •

edited

Loading

CE1CECL Jan 5, 2024 •

edited

Loading

dank074 commented Nov 23, 2024 •

edited

Loading