search

versatica / JsSIP

JsSIP, the JavaScript SIP library
https://jssip.net
Other
2.42k stars 743 forks source link

Improve ICE connectivity and avoid ghost calls #239

Closed dpocock closed 10 years ago

dpocock commented 10 years ago

Currently, JsSIP does not perform connectivity checks until after a call is answered.

This behavior is permitted by RFC 5245, but it is not encouraged, from s12.1.1: http://tools.ietf.org/html/rfc5245#section-12.1.1 "Alternatively, an agent MAY delay sending an answer until the 200 OK; however, this results in a poor user experience and is NOT RECOMMENDED."

I suspect the change is needed about here: https://github.com/versatica/JsSIP/blob/master/src/RTCSession.js#L884 where it is sending a 180 reply, it should actually:

This issue has been discussed on the mailing list: https://groups.google.com/forum/#!topic/jssip/oQEf-adPs_E

sickpig commented 10 years ago

Hi all,

I can confirm we're experiencing the same problem described by Daniel with jssip 0.3.6, i.e. ghost calls and audio delay after answering.

It's very frequent, but frequent enough to be annoying.

Andrea

On Sun, Aug 17, 2014 at 8:37 PM, Daniel Pocock notifications@github.com wrote:

Currently, JsSIP does not perform connectivity checks until after a call is answered.

  • sometimes there are ghost calls - phone rings, callee answers, the ICE checks fail, nobody hears anything, they eventually hang up feeling frustrated with WebRTC
  • after answering, there can be a delay while the ICE checks are executed

This behavior is permitted by RFC 5245, but it is not encouraged, from s12.1.1: http://tools.ietf.org/html/rfc5245#section-12.1.1 "Alternatively, an agent MAY delay sending an answer until the 200 OK; however, this results in a poor user experience and is NOT RECOMMENDED."

I suspect the change is needed about here: https://github.com/versatica/JsSIP/blob/master/src/RTCSession.js#L884 where it is sending a 180 reply, it should actually:

  • send a 183 with ICE candidates
  • wait for the browser to advise that ICE completed successfully
  • then send the 180 (if required to do so)
  • and only after all that should the application receive the event newRTCSession

This issue has been discussed on the mailing list: https://groups.google.com/forum/#!topic/jssip/oQEf-adPs_E

— Reply to this email directly or view it on GitHub https://github.com/versatica/JsSIP/issues/239.

ibc commented 10 years ago

The problem with the proposed solution is that the callee may be prompted for getUserMedia() even before he is notified about an incoming call. And even worse, the call may never be notified to the callee if the caller cancels it before the ICE is connected.

Even worse: if the callee replies 183 with SDP and the ICE/DTLS gets connected before the callee accepts the call, media flow would start (do you want your video to be sent to the caller even before you decide to accept the call?). This could be mitigate by pausing the callee local MediaStream (so no audio/video is sent), but the PeerConnection.onaddstream would fire before the callee accepts the call. This means that the callee may receive audio/video from the caller even before the callee accepts the call, and worse: the caller does NOT know that he is already sending audio/video.

I've another proposal in mind:

To be clear: this proposal DOES NOT improve the media connection delay, but clearly improves the user experience by not firing the "started" event until there is confirmed media connection.

In order to improve the media connection delay there are two approachs:

  1. The above described by Daniel (which is fully covered by the RFC 5245). The big problem is what I comment at the beginning of this comment, which IMHO is too critical and annoying.
  2. Implement Trickle-ICE in JsSIP by following http://tools.ietf.org/html/draft-ivov-mmusic-trickle-ice-sip-02.

Note that with Trickle-ICE same problems as described above may happen. Perhaps it is responsability of the caller to not send audio/video until it receives the SIP 200.

Opinions? I'd go for option 2 (Trickle ICE) given that it would minimize the ICE gathering process in the caller and callee (something that option 1 does not improve at all).

saghul commented 10 years ago

FWIW, I'm also in favor of option 2. Handling multiple 183 responses in case of forking is problematic, to put it mildly, I wouldn't recommend to go in that direction.

My 2 cents.

jmillan commented 10 years ago

Hi,

I'd go for the Tricke-ICE implementation from one hand and also make the session "started" event relative to the RTP status (connected / completed) instead of the sent or received "200 OK" response. The "200 OK" response sent/reception could fire a new event "accepted" instead.

sickpig commented 10 years ago

no strong opinion here, I'm ok with any solutions that will solve the ghost calls problem.

On Fri, Sep 5, 2014 at 12:20 PM, Iñaki Baz Castillo < notifications@github.com> wrote:

The problem with the proposed solution is that the callee may be prompted by getUserMedia() even before he is notified about an incoming call. And even worse, the call may never be notified to the callee if the caller cancels it before the ICE is connected.

Even worse: if the callee replies 183 with SDP and the ICE/DTLS gets connected before the callee accepts the call, media flow would start (do you want your video to be sent to the caller even before you decide to accept the call?). This could be mitigate by pausing the callee local MediaStream (so no audio/video is sent), but the PeerConnection.onaddstream would fire before the callee accepts the call. This means that the callee may receive audio/video from the caller even before the callee accepts the call, and worse: the caller does NOT know that he is already sending audio/video.

I've another proposal in mind:

  • When the callee receives the call and accepts it (so a 200 OK with SDP answer is being to be generated) JsSIP notifies "connecting" within the session and the 200 is immediately sent to the caller.
  • When the caller receives the 200 with the SDP answer (no 183 was received in my proposal) its JsSIP does not notify "started" event until the ICE is connected (so the caller still sees "connecting/ringing" status).
  • Once ICE is connected (PeerConnection.iceConnectionState == "connected" / "completed") the "started" event is fired in both the caller and callee.

To be clear: this proposal DOES NOT improve the media connection delay, but clearly improves the user experience by not firing the "started" event until there is confirmed media connection.

In order to improve the media connection delay there are two approachs:

  1. The above described by Daniel (which is fully covered by the RFC 5245). The big problem is what I comment at the beginning of this comment, which IMHO is too critical and annoying.
  2. Implement Trickle-ICE in JsSIP by following https://tools.ietf.org/html/draft-ietf-mmusic-trickle-ice-01.

Note that with Trickle-ICE same problems as described above may happen. Perhaps it is responsability of the caller to not send audio/video until it receives the SIP 200.

Opinions? I'd go for option 2 (Trickle ICE) given that it would minimize the ICE gathering process in the caller and callee (something that option 1 does not improve at all).

— Reply to this email directly or view it on GitHub https://github.com/versatica/JsSIP/issues/239#issuecomment-54608097.

ibc commented 10 years ago

The "200 OK" response sent/reception could fire a new event "accepted" instead.

It does not come to my mind any reason for it. Do you consider it really useful for something? Note that internally the SIP stack does know that the SIP dialog is established so a terminate() would mean a BYE rather than CANCEL. But from the user/developer perspective, how is useful the "accepted" event?

Well, it may mean "call accepted by the remote party, now performing the media connection stuff, wait please, your money is important for us". Is that? anything else?

ibc commented 10 years ago

I also vote for option 2. Honestly I do not want to deal with PSTN-style 183 responses.

ibc commented 10 years ago

I will start working on this on next Monday. Sorry but I will put on hold any other open issues until this is implemented.

dpocock commented 10 years ago

Before working on any of the proposed options, it would be good to get some more feedback, maybe from the RTCWeb working group and other people who implement ICE with SIP.

saghul commented 10 years ago

@dpocock As a SIP implementor (Blink), I will never implement ICE on 183, for the reasons I mentioned above. Also, taking into account that Emil is one on the authors of the trickle ICE spec you can pretty much assume that's what Jitsi will do.

The issue at hand is related to SIP's usage of ICE, I don't see how this is related to RTCWeb working group.

dpocock commented 10 years ago

@saghul the issue for RTCWeb is whether or not the browser ICE stack believes media must start immediately after the ICE completion event or whether real media flows can be deferred until the application notifies the user and the user accepts the call.

ibc commented 10 years ago

Before working on any of the proposed options, it would be good to get some more feedback, maybe from the RTCWeb working group and other people who implement ICE with SIP.

Let me make clear that the 183 approach defined by RFC 5245 is pre Trickle-ICE which is the real and optimized way to mitigate, not just the ICE connection duration, but also the ICE gathering process.

Does completion of ICE checks imply an immediate start to media flows?

Yes (unless the streams are set to inactive or recvonly, something I don't want to deal with since it would require re-INVITE and so on after the 200). As said in my comment, we can stop sending audio/video by using the MediaStreamTrack methods/attributes for it (so RTCP and, may be empty RTP packets, are sent and the ICE connection remains open).

Or will the final WebRTC JavaScript API allow the media flow to be kept on hold until the 200 OK?

There is not 200 in the WebRTC world. There are SDP offer and answer that may include full or not-full list of ICE candidates (that can be sent later with ICE-Trickle when implemented).

Even now, can the MediaStreamTrack objects be put in the muted state until the 200 OK?

Yes. That's the way IMHO. Both the caller and the callee are responsible to not send audio/video until the SIP dialog is established.

This is not just an issue for calls from JsSIP to another JsSIP user. How will each of the proposed solutions interoperate with other products like Jitsi @emcho , Lumicall and Asterisk @jcolp?

If those endpoints are servers then there is no problem if JsSIP does not send all the candidates in the initial INVITE given that the servers will be able to provide their local ICE candidates (with public IP) and the browser can reach them.

ibc commented 10 years ago

the issue for RTCWeb is whether or not the browser ICE stack believes media must start immediately after the ICE completion event or whether real media flows can be deferred until the application notifies the user and the user accepts the call.

This is answered in my previous comment. Anyhow, the same issue would happens if we go for the 183 option.

JsSIP will mute outgoing media tracks until the call is established by means of a sent or received SIP 200.

dpocock commented 10 years ago

That sounds like something that the browser API should be more flexible about.

Why? we have the tools (the JS API) to do it. We don't need anything else (unless I miss something, but pausing tracks is feasible right now).

Both the caller and the callee are responsible to not send audio/video until the SIP dialog is established.

Does un-muting mean a new offer/answer exchange is required?

Not at all! mute/unmute a MediaStreamTrack is something you perform on the track itself, you don't need to tell nothing to the PeerConnection. The PeerConnection is just a consumer of the track, and you manage the track as you want.

Or is it actually sending a silent/blank stream when the track is muted?

RTCP is sent regardless RTP is sent or not (of course). I don't know if silent/blank RTP is sent while muted. That's up to the implementation, codec, whatever. I don't care too much, and it changes nothing.

If it is sending silence during that time, then it is probably necessary for the caller system to play simulated ringing sounds and not assume there is early media present.

The caller should not render any received media (audio/video) unless a 200 is received.

Well, it may render it in case a 183 with SDP is received (and JsSIP theorically already does that) but that is for old-fashion-PSTN-hateful scenarios.

ibc commented 10 years ago

Hi guys, all the WebRTC related issues are being closed due to the new approach for JsSIP 0.4 as explained here:

Thanks to all for your contribution to the project.