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

Adapter.js throws an exception when removeStream is called #89

Open
magestican opened this issue Aug 21, 2017 · 6 comments
Open

Adapter.js throws an exception when removeStream is called #89

magestican opened this issue Aug 21, 2017 · 6 comments

Comments

@magestican
Copy link
Contributor

I am working on a Safari project and I am using adapter.js to better handle cross browser communication.

I am having an issue when I try to remove a stream for a particular user (PeerConnection), which is :

Error removing media track DOMException: Argument 1 of RTCPeerConnection.removeTrack does not implement interface RTCRtpSender

This happens on this line :
https://github.com/rtc-io/rtc-quickconnect/blob/master/index.js#L737

I get that RTCPeerConnection gets overriden by adapter.js, but is there a way this function can be re-implemented (removeStream) to better suit adapter.js ?

Thanks

@silviapfeiffer
Copy link
Member

rtc.io wasn't built to work with adapter.js - it's possible that the two compete over certain functionality. I think it would make more sense to run rtc.io without adapter.js and identify which things need to be adjusted to work on Safari.

What problems did you encounter that made you use adapter.js on top of rtc.io?

@magestican
Copy link
Contributor Author

This is the error I am experiencing on safari :

TypeError: Error creating RTCPeerConnection initializeRTCPeerConnection
This seems to have been reported in a some other of other library as well, they seem to suggest that the media encoding on safari is to blame :

lynckia/licode#915

I have found that initialization of RTCPeerConnection happens differently on safari as well based on :
https://opensource.apple.com/source/WebCore/WebCore-7602.1.50.1.1/ChangeLog

I believe it is quickconnect's intention to support as many browsers as possible, what do you suggest should be changed to support safari?

@warrenjmcdonald
Copy link

Given the work already done on adapter.js to support Mozilla, Chrome, Edge and Safari, is it a good idea to try to replicate all that in rtc.io.

Adapter.js is getting input from a lot of circles and being widely adopted, would it not make sense to leverage that.

@betimer
Copy link
Contributor

betimer commented Aug 22, 2017

Could we know the logic why when calling removeStream that it checks if removeTrack exists, if exists then make use of removeTrack otherwise call the original removeStream?
https://github.com/rtc-io/rtc-quickconnect/blob/master/index.js#L728

Could it just call removeStream directly?
Thank you

@warrenjmcdonald
Copy link

I think the bigger question is how will rtc.io transition to the new WebRTC APIs track centric, not stream centric and to support media attributes via RTCRTPSender/Receiver.
The Safari release is without legacy WebRTC API, which is why we have been using adapter.js

@silviapfeiffer
Copy link
Member

rtc.io was developed in parallel to adapter.js which is why it's not making use of it. It would indeed be nice to be able to rely on it, but that would require refactoring work for which we currently don't have the resources. We will indeed eventually move from stream centric handling to track centric handling and to the RTCRTPSender/Receiver attributes. There's just not yet a strong need for our team at NICTA/Data61 to do this. Seeing as this is an open source project, we're of course happy for a contribution of such a change.

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

No branches or pull requests

4 participants