fishjam-dev / react-native-membrane-webrtc

Apache License 2.0
78 stars 8 forks source link

Add handler for other message types #149

Open Tonyhaenn opened 10 months ago

Tonyhaenn commented 10 months ago

It'd be helpful if there was a handler to handle non-media messages from the server. I have a case where I want the server to close the connection. I can send a message over the phoenix channel to tell the client that the channel is closing in N ms, but the client has no way of catching that message and handling it at all. So my server just forcibly closes the channel and then the client code has to handle an error.

I think this is because the library is handling opening and managing the socket channel for us. So we don't have access to the channel to attach custom listeners (which is possible in the membrane-webrtc-js)

incubo4u commented 10 months ago

Hi, thank you for your insight. You're right; this library shouldn't manage the socket channel. We plan to remove the dependency on the Phoenix library entirely. Users of this library should be responsible for managing the signaling channel. We have that task high on our backlog, and we plan to implement this soon.

Tonyhaenn commented 10 months ago

I'm not necessarily advocating for separating out the signaling channel into a peer dependency that needs to be managed separately. There's something to be said for a "batteries included" library that enables end to end functionality. Simply exposing the underlying phoenix library might be sufficient

incubo4u commented 10 months ago

We are currently using react-native-membrane-webrtc as a client library for RTC-Engine. RTC-Engine does not impose a specific communication channel for transmitting events. Initially, we utilized Phoenix Channels for this purpose as it was most convenient in our Elixir application. However, we acknowledge that users may have reasons to avoid this implementation.

We want to provide flexibility to users, allowing them to choose the communication channel for events. We do not want to enforce the use of Elixir Phoenix channels. If someone decides to use this library, they should also decide how to transmit events. react-native-membrane-webrtc will be used solely for interpreting these events.

Simultaneously, we are developing Jellyfish Media Server and its client libraries, which are designed to be "battery included." Users do not need to implement anything themselves. In Jellyfish, events are sent via websockets without the Phoenix overlay.

So, we have two approaches: a low-level one where users configure everything themselves, and a simpler path for those who prefer minimal configuration, such as Jellyfish and its SDK.

Tonyhaenn commented 10 months ago

@incubo4u Is this a "close won't fix" sort of resolution? I didn't see a recent PR that addressed it or any updates to the documentation.

incubo4u commented 10 months ago

Hi @Tonyhaenn 👋 I want to clarify that closing the issue doesn’t mean we consider it a “close won’t fix.” We truly appreciate your report, and it’s definitely on our radar 📡 While there hasn’t been recent activity in the form of a PR or updates to the documentation, please know that it doesn’t imply dismissal 😊 Your concerns are important to us, and our team is actively working on addressing the issue. We recognize its significance and are committed to finding a solution. If you have any further questions or suggestions, feel free to let us know!