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 (except Discord.js v13, see below)

NOTE: This PR DOES NOT add Screen sharing (yet), but it does add webcam support and voice

Known bugs:

Windows support must be worked on, medooze-media-server doesn't compile and gives node-gyp errors, if we can't get webrtc to work on windows, we have to find a way to somehow disable webrtc without another set of dupe files.
Discord.js doesn't work in the current /voice/ path, it expects the webrtc in the root of a server, on another port possibly, see this log for more info:

/home/ce1cecl/mellick-bot/node_modules/@discordjs/voice/dist/index.js:353
      this.socket.once("close", () => reject(new Error("Cannot perform IP discovery - socket closed")));
                                             ^

Error: Cannot perform IP discovery - socket closed
    at Socket.<anonymous> (/home/ce1cecl/mellick-bot/node_modules/@discordjs/voice/dist/index.js:353:46)
    at Object.onceWrapper (node:events:627:28)
    at Socket.emit (node:events:525:35)
    at socketCloseNT (node:dgram:754:8)
    at processTicksAndRejections (node:internal/process/task_queues:82:21)
Emitted 'error' event on VoiceConnection instance at:
    at VoiceConnection2.onNetworkingError (/home/ce1cecl/mellick-bot/node_modules/@discordjs/voice/dist/index.js:1531:10)
    at Networking.emit (node:events:513:28)
    at /home/ce1cecl/mellick-bot/node_modules/@discordjs/voice/dist/index.js:689:32
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

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.

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

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);
 		});
 	}
 }

CE1CECL · 2024-12-24T18:20:52Z

Going to put this on draft, because I tried to build this on windows nativly with node.js and I, even with my last PR commit, cannot tsc Spacebar. It needs some more work.

MathMan05 · 2024-12-24T19:23:19Z

I hope you luck with this, I'd like to see WebRTC working

CE1CECL · 2024-12-28T17:06:06Z

@dank074 you said 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, when I was testing the webrtc webcam feature internally, it worked on chrome but not on firefox (it wouldn't pass through either way), does this have to do with something?

CE1CECL · 2024-12-28T17:49:33Z

Sorry, I'm trying to update the spacebar base with the latest. I'll reopen it once I push my code.

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

dank074 · 2024-12-28T22:35:01Z

@dank074 you said 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, when I was testing the webrtc webcam feature internally, it worked on chrome but not on firefox (it wouldn't pass through either way), does this have to do with something?

Yes, audio will also be broken on Firefox since it uses different codec ids for opus/h264/etc than chrome. The client will send SelectProtocol message containing the codec ids that the server should agree to use

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

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 marked this pull request as ready for review December 30, 2023 21:38

MaddyUnderStars requested changes Jan 2, 2024

View reviewed changes

CE1CECL marked this pull request as draft December 24, 2024 18:20

CE1CECL closed this Dec 28, 2024

CE1CECL force-pushed the master branch from b57ac74 to 105b3de Compare December 28, 2024 17:43

Initial Support for Spacebar WebRTC

c22fbd1

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

CE1CECL reopened this Dec 28, 2024

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

CE1CECL commented Dec 24, 2024

MathMan05 commented Dec 24, 2024

CE1CECL commented Dec 28, 2024

CE1CECL commented Dec 28, 2024

dank074 commented Dec 28, 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 24, 2024

MathMan05 commented Dec 24, 2024

CE1CECL commented Dec 28, 2024

CE1CECL commented Dec 28, 2024

dank074 commented Dec 28, 2024

CE1CECL commented Dec 28, 2023 •

edited

Loading

CE1CECL Jan 5, 2024 •

edited

Loading

dank074 commented Nov 23, 2024 •

edited

Loading