Decouple signaling and WebRTC stuff

[ibc]

A perfect rationale is given in the Janus NoSIP plugin discussion.

This is: make JsSIP WebRTC agnostic and let the app provide JsSIP with externally produced SDP blobs.

[michelepra]

what you think about my solution?

[jmordica]

Is this to support interop with legacy sip servers?

If not, can someone point me to an open issue that contains such?

Thanks!

[ibc]

Define "interop with legacy SIP server". A browser cannot have a "call" with plain RTP audio.

[jmordica]

I’m referring to how Janus gateway (when using sip plugin) works with legacy sip servers. So from browser to Janus gateway it uses normal webrtc (ICE/DTLS), and from Janus gateway to sip server just regular SDP.

[ibc]

JsSIP is just a client side SIP UA library. Said that, once we get this feature done, how the application sends and receives SDP is up to it.

[jmordica]

Ah right on. I think this belongs in oversip. I'll delete my comments :)

[mgoodenUK]

This feature looks really interesting - Is it still on your roadmap?

[ibc]

Yes, but no ETA for now.

[mgoodenUK]

We'd like to implement this ourselves with the goal of ultimately raising a PR - Do you have any pointers on the best place to start? Obviously we'd prefer to implement in a way that you guys would find acceptable.

[ibc]

I'm afraid we have not yet thought on how to do this so cannot provide any pointers. Coming to my mind (but must re-check in the future):

• RTCSession needs new methods to provide it with a SDP offer or re-offer.
• RTCSession needs to emit new events when a remote SDP offer or response has been received.
• When a remote SDP offer has been received, such an event must contain (into its arguments) a callback to provide the RTCSession with a SDP answer.

[jmillan]

Agreed with @ibc, we haven't thought of it yet.

IMHO a good starting point would be, following what @ibc commented:

• Create a MediaConnection specification like we do with Socket
  • Such spec would be something like:
    • receiveSdpOffer(sdp) // SDP offer received from the wire, returns a promise resolving a SDP answer.
    • receiveSdpAnswer(sdp) // SDP answer received from the wire, returns a promise resolving void.
    • getLocalSdp() // Retrieves the local SDP. Ie: used if we want it before all ice candidates are gathered.
    • addTracks([tracks]) // Adds the given audio/video tracks, returns a promise resolving a SDP offer.
    • removeTracks([tracks]) // Removes the given audio/video tracks, returns a promise resolving a SDP offer.
    • Events:
      • 'newtrack' // new remote track
      • 'icecandidate' // new ice candidate.
      • 'close'
• Following the above specification, create a default implementation in its own file which would use RTCPeerConnection, as done now in RTCSession. It could be named MediaConnectionInterface.
• Usage:
  • In UA.call() and RTCSession.answer() the user now must provide a MediaConnection instance (we can later use factories) which is assigned to the RTCSession and following the above specification will be managed by RTCSession regardless its implementation.

[Haardt]

This is a really great feature. Can I use jsSIP with Mediasoup to send an RTP stream to a SIP server (for example to mixing audio)?

[jmillan]

No you can't. Maybe this effort would be a starting point that would let you to it, but you would have to do a module that communicates with mediasoup, deals with SDP etc. In short no, this will not allow you do that.

[Haardt]

Yes, not out of the box - Maybe the wrong place, but we have developed a SFU based on Mediasoup and wanted to use jsSIP to send the RTP streams to another SIP server. We wanted to use the SDP data from Medasoup and then pass it to jsSIP. We need to support "phone ports" :).

[ibc]

mediasoup does not generate SDPs. However there is an ongoing mediasoup-sdp-bridge project. Let's please not use this issue to discuss this.

[Haardt]

I have been working on this ticket. I removed the "RTCPeerConnection" and the "...getUserMedia" from the RTCSession file. My current API for Browser/Node-MediaConnectionInterface:

• signalingState() (ice state?, getter)
• setup(pcConfig, rtcConstraints) - create the RTCPeerConnection or something else
• receiveSdpOffer(options)
• receiveSdpAnswer(options)
• registerIceUpdates(constraints, type, callback) - used in '_createLocalDescription'
• prepareStreams(constraints) - calls 'getUserMedia' and adds the tracks
• onIceConnectionEvent(callback) - inform about ice failures ('global')
• getSenders() - used for dtmf... bad name?
• close() - Close the RtcPeerConnection and you can stop your streams
• setRemoteDescription(sdp) - rename to setRemoteSdp?
• getLocalSdp()

I can use the 'tryit-jssip' application with the modifications. However, the 'RTCPeerConnection' is also used outside (as 'peerconnection') - maybe 'beaking changes'.

@jmillan I can't find the events? How to handle 'newtrack, close' in the RTCSession?

[jmillan]

How to handle 'newtrack, close' in the RTCSession?

In the MediaConnection light spec done here it is said that such events are triggered by the MediaConnection instance.

RTCSession.connection will now be a MediaConnection instance instead of a RTCPeerConnection one.

Could you please detail the question having that in mind?

[Haardt]

Exactly, I have replaced the 'connection'. Now the 'Interface / MediaConnection' is used everywhere. I have created a feature branch: https://github.com/Haardt/JsSIP/tree/feature/427-Decouple_signaling_and_WebRTC_stuff Maybe you can look into it - i'm on the right way?

[jmillan]

Sure! Let us few days and we'll take a look.

[jmillan]

I've made some comments @Haardt, it's starting to look nice! I've more comments to do but I prefer that you handle these comments and then we'll go for a second round.

[jmillan]

few points:

• Please keep the name _connection in RTCSession. It's way easier to see the real changes this way.
• Please keep the bracket style, new line before return, new line after the last const declaration in the block.
• We are not going to do getUserMedia anymore. Let the user provide the tracks or add them explicitly.
• UA does not need an instance of MediaConnection
• MediaConnection must be generic, and not mandate any WebRTC specific thing like getSenders.
• Lets provide a sendDTMFs() method and let the implementations do it their way.

[jmillan]

• No need for onIceConnectionEvent() or alike.
  • they are WebRTC specific.
  • the interface already defines [add|remove]EventListener. Let's provide a single way to do things.

[Haardt]

@jmillan i made some progress:

• [x] Please keep the name _connection
• [x] We are not going to do getUserMedia anymore
• [x] UA does not need an instance of MediaConnection
• [x] Lets provide a sendDTMFs()
• [ ] getSenders (Used in mute/unmute)
• [x] onIceConnectionEvent() removed
• [ ] Some problem with "this.rtcReady" flag (depends on Ice?)
• [ ] Please keep the bracket style (hope i get all)
• [x] Moved RTCSessionDescription to 'MediaConnection' Thank you for your comments! I need now the next suggestions :).

[jmillan]

Hi Haardt,

It's very difficult to comment on a branch if there is no PR. Could you make a PR from your new branch to a JsSIP master branch in your origin?

This way it'll be much more agile making comments and handling feedback.

[Haardt]

@jmillan i have create a draft pull request from my branch to my master.

[Haardt]

ping - some progress in the branch.

[jmillan]

We will comment on it.

[jmillan]

Another review done.

[Haardt]

I have implemented your comments. What do we do with 'getSender'? This is needed for mute.

I have also implemented two test projects:

• TrySip with the new version
• Mediasoup with FreeSwitch (Web->Mediasoup->jsSIP->FreeSwitch->SipPhone)

I think I can publish both projects at the end of next week.

[jmillan]

What do we do with 'getSender'? This is needed for mute?

I commented here:

https://github.com/Haardt/JsSIP/pull/1/files#r586408690

I think I can publish both projects at the end of next week.

We need to review it further. Please make sure you have implemented all the comments I made.