Skip to content
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

Same video stream for each user connected #480

Closed
CSantosM opened this issue Mar 30, 2020 · 25 comments
Closed

Same video stream for each user connected #480

CSantosM opened this issue Mar 30, 2020 · 25 comments

Comments

@CSantosM
Copy link
Contributor

Expected behavior

Unique video stream per user.

Observerd behavior

Testing the simple openvidu-ionic app, I have realised that since 6.0.8 version (included) the video stream received by cordova-plugin-iosrtc is the same that first one.

This behaviour is not happening with the 6.0.7 cordova-plugin-iosrtc version.

Steps to reproduce the problem

To reproduce this issue, we only have to join to the room with ionic ios app and connect multiple devices to the same session.
From the ios app view, the remote streams will be the same that the first remote user connected.

Platform information

  • Cordova version: 9.0.0
  • Plugin version: 6.0.9
  • iOS version: 13.3 and 13.1.3
  • Xcode version: 11.3.1
@hthetiot
Copy link
Contributor

@CSantosM do you use Janus ?

@hthetiot
Copy link
Contributor

Most likely related to this #467

@CSantosM
Copy link
Contributor Author

No, i do not

@hthetiot
Copy link
Contributor

22812929-802D-4337-96E2-7FBA026ACD10

@CSantosM i do not reproduce that issue using 6.0.9 on Sylaps.com using https://apps.apple.com/us/app/sylaps/id954016392

@hthetiot
Copy link
Contributor

hthetiot commented Apr 1, 2020

@CSantosM Can you check MediaStream.id and MediaTracks.id for remote streams.
What 6.0.8 changes does is keep the media.id from the remote instead of faking them.

https://github.com/cordova-rtc/cordova-plugin-iosrtc/pull/467/files#diff-379e16e943b1b62acdbc68ad8a80241fL20

Do you use any MCU/SFU ?

@CSantosM
Copy link
Contributor Author

CSantosM commented Apr 2, 2020

Hello @hthetiot,
Checking MediaStream.id and MediaStreamTrack.id.
I've added a print log into a PluginMediaStream.swift and PluginMediaStreamTrack.swift

The logs are the following:
When localUser(ionic - ios) is connected:

OpenViduLog: MediaStreamTrack ID =>  -6336181468174245674
OpenViduLog: MediaStreamTrack ID =>  133574601189058849
OpenViduLog: MediaStream ID =>  FFFB9244-1BD4-44B4-9B72-F53F4D2B9735

Then, When a Chrome browser join to the session. The stream looks correct:

OpenViduLog: MediaStreamTrack ID =>  6014604358066097781
OpenViduLog: MediaStreamTrack ID =>  -3513255213319178647
OpenViduLog: MediaStreamTrack ID =>  6014604358066097781
OpenViduLog: MediaStreamTrack ID =>  -3513255213319178647
OpenViduLog: MediaStream ID =>  default

Then another different user is conncted. The same stream as above will appear.

OpenViduLog: MediaStreamTrack ID =>  3716680559757491135
OpenViduLog: MediaStreamTrack ID =>  810162222955606700
OpenViduLog: MediaStreamTrack ID =>  3716680559757491135
OpenViduLog: MediaStreamTrack ID =>  810162222955606700
OpenViduLog: MediaStream ID =>  default

The mediaStreamTracks are different each others.

OpenVidu uses exclusively SFU.

It helps?

@hthetiot
Copy link
Contributor

hthetiot commented Apr 3, 2020

OpenVidu uses exclusively SFU.

Your SFU is not generating unique MediaStream or MediaTrack ID :)
In the past iosrtc was creating unique ID, we now reuse the ID from SDP.

What SFU are you using please, so i can make joke about them.

@CSantosM

@j1elo
Copy link

j1elo commented Apr 3, 2020

Are you talking here about the a=msid attribute in SDP?

The SFU in use is Kurento, which doesn't support Unified Plan nor it needs to, given that it only works with one single audio and one single video per WebRTC connection. With these conditions, both PlanB and Unified Plan are equivalent in practice.

Any endpoint should be able to rely on proper SDP Offer/Answer negotiation to convey that the a=msid attribute is not supported, by the fact that it is missing on the SDP Answer. So, after receiving such Answer, the local endpoint is expected to adapt to that (just like any other negotiated attribute).

Using an internally-generated UUID is the thing to do for endpoints that really require it. That's by the way what libwebrtc does (see commit), and the standard text mentions too (see PR).

@hthetiot
Copy link
Contributor

hthetiot commented Apr 6, 2020

@j1elo Thank you for detailed explanation.
Can we consider that generating UUID when using PlanB and not 36 char is a acceptable solution?

The thing is OpenEasyRTC and many other WebRTC library does relly on msid to match on both side, and even if i use PlanB on Chrome, Firefox or Safari since 2016, msid always matched.

@hthetiot
Copy link
Contributor

hthetiot commented Apr 6, 2020

I will look into WebRTC changes here https://chromium.googlesource.com/external/webrtc/+/5b1477839d8569291b88dfe950089d0ebf34bc8f and see if needed on our side then release 6.1.0

@hthetiot hthetiot modified the milestones: 6.0.x, 6.1.x, 6.1.0 Apr 6, 2020
@hthetiot
Copy link
Contributor

hthetiot commented Apr 6, 2020

The thing is Kurento does give msid, it's just named default... may be this issue is that internaly iosRTC use those, may be the fix is not to generate UUID here but making sure iosrtc internaly make the difference using an internal uuid.

@hthetiot
Copy link
Contributor

hthetiot commented Apr 10, 2020

I will try to release 6.0.10 in the next 48 hours for you guys :)
With a fix.

@hthetiot
Copy link
Contributor

I need more time to make it as good as https://chromium.googlesource.com/external/webrtc/+/5b1477839d8569291b88dfe950089d0ebf34bc8f keep you posted.

@micaelgallego
Copy link

micaelgallego commented Apr 19, 2020

Thank you!

If you need any help on our side (OpenVidu team), please ping us.

@hthetiot
Copy link
Contributor

@micaelgallego I have a proper fix on a locale branch with M75 for now i did #493 dirty fix can you check.

In any case, PR are always welcome.

@hthetiot hthetiot modified the milestones: 6.1.0, 6.0.x, 6.0.11 Apr 20, 2020
@hthetiot
Copy link
Contributor

ping @CSantosM see #480 (comment)

@pabloFuente
Copy link

pabloFuente commented Apr 21, 2020

We will give it a try ASAP and give ypu feedback. Thank you very much!

@CSantosM
Copy link
Contributor Author

Hi @hthetiot ,

We have tested this branch and it looks like the problem has been solved.
We have connected three users in the same session and we have not reproduced the bug because of the unique mediastream id.

Great, let us know when this bug fix will be released.

Regards

@hthetiot
Copy link
Contributor

@CSantosM Ok will release

@hthetiot
Copy link
Contributor

@micaelgallego
Copy link

Thanks!

@CSantosM
Copy link
Contributor Author

CSantosM commented May 4, 2020

Hey @hthetiot

Where can I found the 6.0.11 release? The latest version shown is 6.0.10.

Regards

@hthetiot
Copy link
Contributor

hthetiot commented May 4, 2020

Npm ?

Screen Shot 2020-05-04 at 7 04 52 PM

Screen Shot 2020-05-04 at 7 05 31 PM

I also just pushed tag if that what you where looking for.

@hthetiot
Copy link
Contributor

hthetiot commented May 4, 2020

@CSantosM
Copy link
Contributor Author

CSantosM commented May 4, 2020

Ok, thanks @hthetiot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants