Make videoroom compatible with multistream

[golgetahir]

Hi @atoppi I am using Janode in the multistream version, so far these changes need to be done for making it work on the plug-in end. I hope it will be helpful, if you decide to merge it I can contribute further by developing a sample for it etc.

[atoppi]

Thanks @golgetahir for the effort !
I think this PR can be considered a starting draft for the multistream support in the VR plugin but in general we have just scratched the surface of the problem here :-)

The main effort when implementing multistream in janode VR is not (only) the new syntax integration, since the compatibility with the existing applications and the coherence with the existing code should be on top of the list.

We should deeply understand the consequences of breaking the design [1 audio + 1 video] per PC. Also I'm thinking about abstractions we added to janode: e.g. in the VideoRoom plugin the Handle has a feed property that is used in many places (e.g. any time you see this.feed). With multistream having that property does not have sense anymore. There are a bunch of other abstractions in the library that might be hit by the switch to multistream, so I guess many other changes are needed and all of them should be tested for backward comp.

On top of that, I'd like to review the syntax of the new multistream APIs since one of the mission of janode is streamline and some-how make friendlier the rough edges of the janus API. It is ok to use the same Janus objects and formats in the beginning, but I guess something could be made "better" in terms of janode conventions.

Nevertheless I'll consider this a kick-off for a long awaited feature I guess, far from being complete in its current status, but very very welcome as a contribution.

[golgetahir]

Hi @atoppi thanks a lot for your care on the subject. Of course, we are on the same page that this is just a patch on the plug-in to somewhat support the new architecture, also I am using a custom version of the server side handler. Though I am willing to work as a contribution if you guide me a bit about how shall we complete the task. If it makes sense for you too feel free to reach me, [email protected] is my e-mail address and we can work closer you can assign me tasks etc.

[atoppi]

Hi @golgetahir and everyone interested in this topic!
In the last few days I thought about how to proceed here and came to the conclusion that maybe the best approach would be to create a whole new vr multistream plugin either by implementing a totally different unit (e.g. videoroom-multi-plugin.js) or by introducing some kind of versioning in the existing plugin (I'd prefer the former solution to be honest).

I feel that making the existing plugin compatible with multistream:

1. without breaking previous apps
2. supporting both flavors of API in the same file unit

would lead to over-complicated and hardly maintainable/readable code.

[atoppi]

@golgetahir I ended up importing some multistream events and properties in the videoroom plugin in order to make existing apps compatibile with janus 1.x.
Would you please update the PR by solving conflicts?

Janus 1.x has some major differences when dealing with particular scenarios: e.g. subscriber renegotiations are totally different, since 0.x sends back an updated JSEP, while 1.x sends a publisher list event with an updated streams array, and it's up to the application decide to renegotiate or not through the update API.

I deliberately decided not to import the code for joining a subscriber in multistream mode (e.g. adding the streams property to join API), like this PR is doing, since that would lead the server to interact in 1.x mode and I guess that would break a whole bunch of existing applications if misused.

Unfortunately I still have to find the time to better understand the consequences of the migration.
Nonetheless I feel we are doing some steps in the right direction, thanks for the support!

[golgetahir]

Hi @atoppi sorry that it took a while for me to take a look at, had some nasty events ( earthquake ) in Turkey but I am returning my normal routines now, I updated the PR but I think nothing left from this PR is going to be merged if you deleted the join APIs streams method ( if I understand it correctly ).

I think you are right about the breaking changes. If there is anything I can do please let me know.

[atoppi]

@golgetahir I saw on the news, what a catastrophic and terrible event... hope you are all doing well and safe!

I guess the PR is complete now, thanks again for the effort!

[atoppi]

Sorry for the huge delay, I forgot this was a pending PR.
I have superseded this PR with the commit above, thanks again!

(potential problems with existing applications will be handled by applications or by opening new issues)