-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
First version of SFU API #2
Conversation
…rom MembraneWebRTC
f0deacf
to
0cc228f
Compare
Signed-off-by: Austin Putman <austinp@geometer.io> Co-authored-by: Austin Putman <austinp@geometer.io>
Awesome! Cherry-picked
Good idea, this also is something that @mat-hek proposed in #2 (comment). Cherry-picked I am also wondering if |
872eca6
to
c611020
Compare
assets/js/membraneWebRTC.ts
Outdated
let mediaEvent = generateMediaEvent("join", { | ||
relayAudio: relayAudio, | ||
relayVideo: relayVideo, | ||
receiveMedia: this.receiveMedia, | ||
metadata: peerMetadata, | ||
tracksMetadata: Array.from(this.localTrackIdToMetadata.values()), | ||
}); | ||
this.callbacks.onSendMediaEvent(serializeMediaEvent(mediaEvent)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sendMediaEvent
calling generateMediaEvent
, serializeMediaEvent
and onSendMediaEvent
would be useful
assets/js/membraneWebRTC.ts
Outdated
|
||
/** | ||
* Leaves the room. This function should be called when user leaves the room | ||
* in a clean way e.g. by clicking dedicated, custom button `dissconnetc`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* in a clean way e.g. by clicking dedicated, custom button `dissconnetc`. | |
* in a clean way e.g. by clicking a dedicated, custom button `disconnect`. |
lib/membrane_sfu/media_event.ex
Outdated
case event do | ||
%{"type" => "leave"} -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you already check that in the header
@@ -88,7 +105,7 @@ defmodule Membrane.SFU.MediaEvent do | |||
end | |||
|
|||
defp do_deserialize(%{"type" => "join"} = event) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will be a match error on an unknown event. We'd probably add a fallback clause returning an error.
lib/membrane_sfu_app.ex
Outdated
@@ -0,0 +1,13 @@ | |||
defmodule Membrane.SFUApp do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defmodule Membrane.SFUApp do | |
defmodule Membrane.SFU.App do |
mix.exs
Outdated
|
||
defp compile_ts(_) do | ||
Mix.shell().info("Installing npm dependencies") | ||
{result, exit_status} = System.cmd("npm", ["install"], cd: "assets") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should rather be npm ci
In reference to
We were torn on this. I think either way is valid. |
…JoinError` respectively
No description provided.