meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.23k stars 2.48k forks source link

[sip] ICE fails with Chrome 45 for incoming video calls #328

Closed saghul closed 9 years ago

saghul commented 9 years ago

Checked this one on your public service (saghul@sip2sip.info should be in your logs xD). Firefox works fine, sigh. (audio only calls work fine). Tried Canary (47) to no avail either...

Here is the SDP the browser gets: https://gist.github.com/saghul/23b018f974dd12f51526

What's also interesting and looks like another bug, is the fact that if another call is made after this one failed, the DTLS direction is set to passive, instead of actpass: https://gist.github.com/saghul/416561bb971fb3d3bcdd

So it looks like something doesn't get 100% cleaned up.

lminiero commented 9 years ago

Is this only happening on Chrome 45 and following, or did this always fail in Chrome?

This might be silly, and might be a red herring, but I see video before audio in the SDP. I'm wondering if there may be a problem with this in Janus, as when creating the ICE stack we don't look at the order of the media but more generally check if a specific medium has been negotiated or not and proceed with the setup accordingly:

https://github.com/meetecho/janus-gateway/blob/master/ice.c#L2177

Anyway, that shouldn't be the case as we leave the ID creation to libnice (unless libnice expects us to respect the SDP order when creating streams and components?). Besides, if this works with Firefox but not Chrome, this seems to rule out a problem in Janus.

That said, you should look at the answer the browser sends and at the candidates that have been exchanged. Looking at both chrome://webrtc-internals and admin API in your setup might help too, in particular to check why ICE is failing, if trickle candidates are being mixed up and so on.

saghul commented 9 years ago

I'll debug further, just wanted to put it up here so others can attempt to reproduce.

Chrome used to work (I recall testing 43) I'll see if I can re-test that version.

saghul commented 9 years ago

Chrome M43 and M44 do work, and the video stream comes first as well. The SDP looks just fine :-O https://gist.github.com/saghul/1347fcbea8248c6156dc

saghul commented 9 years ago

Besides, if this works with Firefox but not Chrome, this seems to rule out a problem in Janus.

@lminiero I think the second issue I mentioned (creating the incorrect a=setup line after a failed first call) is actually a bug in Janus.

lminiero commented 9 years ago

@lminiero I think the second issue I mentioned (creating the incorrect a=setup line after a failed first call) is actually a bug in Janus

Yep for that we need a fix: I was referring to the ICE failure though.

saghul commented 9 years ago

Sure. Will dig further tomorrow, right now my brain is no longer functioning properly :-/

lminiero commented 9 years ago

Just checked and the DTLS role is always reset when we create a new agent. I think in your case a new call after a failure is actually being handled by Janus as an update to an existing session.

saghul commented 9 years ago

Quick update: upgrading to libnice 1.13 hasn't changed a thing.

lminiero commented 9 years ago

Have you looked into the video-before-audio SDP thing?

saghul commented 9 years ago

Yep, on Chrome M43 and M44 it was also coming first, and working fine. See https://github.com/meetecho/janus-gateway/issues/328#issuecomment-140775167

lminiero commented 9 years ago

What I meant is, have you tried having your SIP client generate audio before video in the SDP, and check if Chrome digested that? If so, it may be a bug in Chrome which should be reported there.

saghul commented 9 years ago

Ah, that. No, I haven't tried it. I'll add that to the to-test list :-)

saghul commented 9 years ago

Ok, after ignoring all bogus interfaces for ICE in Janus I got meaningful results. It works if the video stream is the second stream, not if it's the first one. I'm puzzled, since this used to work in M44 and does work in FF. Oh well, I guess something is broken in Chrome.

Thanks a lot for the pointer @lminiero!

lminiero commented 9 years ago

There is one more thing you might want to check. Try launching Janus with -U -u: this will force Janus to always do BUNDLE and rtcp-mux without waiting for them to be negotiated. As a result, a single stream and single component will be created, and so all candidates will refer to that alone. If this way it works, it might mean the issue in in Janus and how it gets rid of the extra streams/components when BUNDLE and rtcp-mux are enforced in the normal scenario.

saghul commented 9 years ago

You're the man! :sparkles: Toggling those made ICE succeed, and audio works, but video is broken. Looking at the release notes for M45 this is the only one that looks related.

Thanks a lot for the help troubleshooting Lorenzo! I plan to modify Blink Cocoa to make it consistent with Blink Qt and put the streams in "audio, video" order, but Janus should probably handle this gracefully. Let me know how I can help, since I'm afraid I'm not familitar with that part of the code.

saghul commented 9 years ago

IIRC (just came back from vacation) this was fixed :-)