Allow one session to join multiple rooms

[devfans]

Hi, I was checking the plugin implementation, now thinking it looks possible to allow one session to join multiple rooms. Currently, I only need the publisher to be across rooms though. Wondering are you planning to implement this feature? Or is it impossible to add this feature for some logic I didn't notice?

Thanks!

[mrhegemon]

We are building a naf-jitsi-adapter with sfu support. This should work with hubs but will need many reticulum services. https://myxr.social

[devfans]

Hi @gfodor, wondering which commit of this plugin is the hubs live env building with? Because I was building from the master branch of this plugin today and put the .so file on the Janus server and restarted the process, then the process will throw a segment error when processing JSEP. Also, I see the branch feature/user-room-m2m, so does it already support one Janus session to join multiple rooms? Thanks

[gfodor]

Hey Stefan, we are not running master of this plugin for hubs currently, but a version before the recent changes and the websocket multithreading. @mqp may have an idea of what you are running into is a new bug.

On Mon, Apr 27, 2020 at 4:45 AM Stefan Liu notifications@github.com wrote:

Hi @gfodor https://github.com/gfodor, wondering which commit of this plugin is the hubs live env building with? Because I was building from the master branch of this plugin today and put the .so file on the Janus server and restarted the process, then the process will throw a segment error when processing JSEP. Also, I see the branch feature/user-room-m2m, so does it already support one Janus session to join multiple rooms? Thanks

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/mozilla/janus-plugin-sfu/issues/55#issuecomment-619930442, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABVW5DGNQ6F2F27BGITJN3ROVV4LANCNFSM4MQWS42A .

[devfans]

I see, thanks. The issue was probably caused by plugin API compatibility since I saw some recent updates that were added for supporting the new Janus version.

[mqp]

Thanks for letting me know about this, I'll take a look at it tomorrow. I assume you're trying it with the latest Janus?

On Mon, Apr 27, 2020, 22:23 Stefan Liu notifications@github.com wrote:

I see, thanks. The issue was probably caused by plugin API compatibility since I saw some recent updates that were added for supporting the new Janus version.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mozilla/janus-plugin-sfu/issues/55#issuecomment-620387996, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADIUXNWP4GW4MTQ25PJ7YLROZR53ANCNFSM4MQWS42A .

[devfans]

Hi, I was trying with Janus 0.7.x I think, I saw this commit https://github.com/mozilla/janus-plugin-sfu/issues/55 was added to support newer Janus version, so that's probably why I was running into an issue.

[mqp]

Yes, unfortunately there's no way to keep the plugin backwards-compatible with old Janus versions. Commit 14a33464726166fa0d3a20bd452ad05d2f7c53a6 is the last one that should be compatible with 0.7.x.

[vincentfretin]

With the current code in master, for a publisher to be in several rooms at the same time, the following needs to be modified:

• process_join needs to be modified to have a list of rooms in parameters. This is a breaking change so better doing a process_join_multi_rooms with new MessageKind::MultiJoin something like that, and you call switchboard.join_publisher for each room (and notify the other publishers in the room) and set from.join_state with room_ids plural instead of a single room_id.
• process_block/process_unblock needs changes to iterate over publisher rooms to find the session (with notifications:true) of participant we want to block. Actually we can just use get_publisher there, we have an optimized struct now. In process_join we should maybe check notifications: true instead of data: true (the hack) to know if it's a publisher?
• In destroy_session, you need to iterate over the rooms the publisher are in to notify the other participants he left.

I was just checking what needs to be modified in the code to support this use case now that I'm familiar with the code, but I don't have this use case for now. I know @arpu you were interested having such a feature.

[devfans]

The use case was a publisher’s voice needed to broadcast to lots of small rooms. I made a patch to support that https://github.com/devfans/janus-plugin-sfu/tree/mcc

[vincentfretin]

Ah ah nice @devfans you almost exactly did what I said. :D It's just a little bit hacky with room_id.split("-") to have several rooms instead of my MultiJoin message I proposed, but it works I guess. :) Here the link with all the changes to better see what changed with your 4 commits https://github.com/devfans/janus-plugin-sfu/compare/master...devfans:mcc

I see you did a change in process_data (broadcast to all rooms) and data_recipients_for (using only the first/main room). Can you explain the use case you had here so I better understand the changes? Because for me the changes in process_data may not be what I expect if I use this for chat. I want conversation to be isolated for a room, eventually a moderator can broadcast to all rooms why not if we want to do some sort of breakout rooms, a use case I'm really interested in. Maybe we need to add a room_ids parameter for the MessageKind::Data, if not specified broadcast to all rooms, otherwise broadcast to the specified rooms.

To move this further, I think we can start doing a PR just for process_block/process_unblock to use get_publisher like I said, this should be a small change without breaking anything. => #80 After that a second PR with changes similar to what you did and like I proposed. If other can explain their use cases it would be great so we can take decisions how to implement this so it covers all your use cases.

[devfans]

oh right, it was used for Hubs conference, which required the speakers’s voice to be broadcast to the guest rooms. So the solution was either the speakers to joins all the rooms and publish streams without subscribing to the guest rooms, or all guests need to joins the speakers room to subscribe to the speakers but without publishing. So the main room you mentioned is the original primary room in the context.

[vincentfretin]

Ok I understand your use case, in the end this was mainly to have more users listening to the speaker :-) If you were using Hubs Cloud, I understand better why you did the changes like this, this way you didn't have to change the js code I guess, no changes in naf-janus-adapter was required. Note that the changes you did in process_data weren't required for your case I think because Hubs doesn't use the Data message via janus, but uses the Phoenix channel. process_data is called only when you use NAF.connection.adapter.reliableTransport = "websocket"; and/or NAF.connection.adapter.unreliableTransport = "websocket"; Hubs is using a function for those that send data to the Phoenix socket.