search

rtc-io / rtc-quickconnect

An opinionated approach to creating WebRTC apps (both media and datachannels supported)
Apache License 2.0
186 stars 43 forks source link

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

Open magestican opened 7 years ago

magestican commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 :

https://github.com/lynckia/licode/issues/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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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.