Skip to content

Conversation

@josephfrazier
Copy link
Contributor

Browsers have gotten better at renegotiation (now that Firefox 38 is out), so let's consider turning this back on.

This reverts commit 06bc43d, reversing
changes made to bacea36.

resolves onsip#168

Conflicts:
	src/Session.js
@josephfrazier
Copy link
Contributor Author

In my local checkout, I merged this into 873c9da (master branch at time of writing) and built a test page for trying out hold/unhold (and therefore receiveReinvite) with different combinations of browsers/audio/video/inviteWithoutSdp. I'll see if I can break anything, and update this with my findings.

@josephfrazier
Copy link
Contributor Author

Ok, after a bunch of testing, I found that there's still a couple of problems with cross-browser WebRTC. #182 is one such problem, and another is as follows: If Firefox makes an audio call with SDP to Chrome, and Chrome answers with audio then tries to hold(), Firefox refuses to renegotiate printing the following error:

DOMException [InvalidSessionDescriptionError: "ICE restart is unsupported at this time (new remote description changes either the ice-ufrag or ice-pwd)ice-ufrag (old): XNaL6MUvAo6+hfTmice-ufrag (new): mQDDAkDOY9Yf9wJoice-pwd (old): 5MmxCArSUOdddCA7HQgOE5+6ice-pwd (new): WX9lUGO1SxoGP3hu0h4szV+V"
code: 0
nsresult: 0x0]

However, Firefox does send a 488 in this case, and the call carries on as usual (note that hold() also disabled the audio MediaStreamTrack, but unhold() will re-enable it), instead of with a "crashed PeerConnection" as in #19.

Given that making this change allows the browsers to at least try renegotiating (and gracefully falling back to sending either a 488 or a 500 if it fails), I think this is fine to merge in.

josephfrazier pushed a commit that referenced this pull request Jun 1, 2015
@josephfrazier josephfrazier merged commit 52da89c into onsip:master Jun 1, 2015
@josephfrazier josephfrazier deleted the rerenegotiate branch June 1, 2015 20:51
@wpp
Copy link
Contributor

wpp commented Jun 8, 2015

Hi @joseph-onsip,

glad you're working on this! Hopefully I've got some time this week to test this as well and perhaps help you out a little. I'd be looking into switching/adding/removing streams on existing PeerConnections. Looks like your most recent work is on master as of now?

BTW: AFAIK cross-browser renegotiation is at least a couple of months out since Chrome and FF use different implementations of multistream (the link you posted for FF 38):

Also, it is important to note that Google Chrome’s current implementation of multistream is not going to be interoperable; this is because Chrome has not yet implemented the specification for multistream (called “unified plan” – check on their progress in the Google Chromium Bug tracker). Instead they are still using an older Google proposal (called “plan B”). These two approaches are mutually incompatible.

But in case someone is looking for a temporary work-around there is a polyfill which might help out.

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

Successfully merging this pull request may close these issues.

3 participants