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

Refactor onWebRTCommCallOpenedEvent #66

Closed
atsakiridis opened this issue Dec 13, 2016 · 2 comments
Closed

Refactor onWebRTCommCallOpenedEvent #66

atsakiridis opened this issue Dec 13, 2016 · 2 comments
Assignees

Comments

@atsakiridis
Copy link
Collaborator

Seems that that right now it is triggered from a couple of places, which don't seem right. Instead, I think we should trigger it from one place: onRtcPeerConnectionIceChangeEvent, called from onicechange and for iceConnectionState == connected.

Just keep in mind this commit, and make sure that there are no audio issues with new logic

@atsakiridis atsakiridis self-assigned this Dec 13, 2016
@atsakiridis
Copy link
Collaborator Author

Some further notes after doing some coding & testing:

  • I tried to remove onWebRTCommCallOpenedEvent from all previous places and introduce it to onRtcPeerConnectionIceChangeEvent for 'connected', but it turns out that there are issues with Firefox which doesn't transition to connected for some reason and hence we never get media
  • Tried to leave onWebRTCommCallOpenedEvent only in onRtcPeerConnectionOnAddStreamEvent where it existed previously and this worked without issues
  • Still to be consistent with mobile SDKs I think we should try to add it at onRtcPeerConnectionIceChangeEvent but first fix the issues with the state. I think the issue once again might be erroneous STUN/TURN settings

@atsakiridis
Copy link
Collaborator Author

Did some more digging and the issue isn't the STUN/TURN settings. Especially for web the web app needs to know the remote stream at the point where onWebRTCommCallOpenedEvent is called. So we should keep that event in onRtcPeerConnectionOnAddStreamEvent and only there.

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

1 participant