Allow empty array for "stun_servers" and "turn_servers"

[ibc]

Proposal 1

• An empty array should be passed verbatim (so no STUN/TURN servers are set, which is good in some cases).
• When "null" or "undefined" apply default value.

Proposal 2

• An empty array, null or undefined should apply the default value, but...
• The default value is an empty array (so no STUN/TURN servers are set, which is good in some cases).

I vote for proposal 2, but I'm fine with proposal 1.

[mEkblom]

Overall only null and undefined should be counted as a request for default value for any parameter in my opinion. So, I'd go for solution 1.

[ibc]

Right, my point is "what should be the default value for 'stun_servers' passed to RTCPeerConnection?

• If you pass null or undefined then the browser (really) sets its default STUN servers.
• If you pass an empty array the browser does not set STUN servers.

So, what should be the default value in JsSIP's "stun_servers" and "turn_servers" parameter? null? or an empty array? That's the point :)

[jmillan]

The default should be null or undefined in my opinion, since an empty array says explicitly that no STUN or TURN should be used.

[ibc]

Fine with me. But it must be clearly documented in the API that not setting "stun_servers" means "use whichever STUN settings the browser uses by default, if any".

[mEkblom]

I don't actually mind having the google server as default - and in my opinion otherwise it should default to an empty array (null or undefined will probably fail and cause an error, not verified). If STUN server defaults to null it would make more sense to make it a required parameter (but then again, it makes more sense to leave it optional and default to [] or google server not to cause an error).

So, default to empty array (preferred) or the google server (may work in most circumstances) in my opinion.

[mEkblom]

Read the above comments more carefully now. If null or undefined will not cause an error, just passing on whatever value would be fine.

[jmillan]

@ibc, I've tested Chrome 35.0 and FF and Chrome does not use any STUN server when passing a null or undefined parameter to the RTCPeerConnection constructor. FF does use a default STUN server on the other hand. Did you try Chrome?

[ibc]

@jmillan: Honestly, no idea, I thought so but may be I was wrong or things have changed or whatever.

@mEkblom:

I don't actually mind having the google server as default - and in my opinion otherwise it should default to an empty array (null or undefined will probably fail and cause an error, not verified).

Yes, passing anything but an array as "iceServers" value causes an error.

If STUN server defaults to null it would make more sense to make it a required parameter (but then again, it makes more sense to leave it optional and default to [] or google server not to cause an error).

Please let's just talk about JsSIP "stun_servers" parameters (to avoid confusion). Which should be its default value if not set (regardless what each browser does if "iceServers" is not set in a PeerConnection)?

I vote for empty array (so no STUN servers as the empty array will be passed to the PeerConnection as "iceServers" value).

[mEkblom]

vote++ for empty array

[jmillan]

I come back to this after some time.

If stun_servers configuration parameter is not set, it should take a default STUN server (google)
If stun_servers configuration parameter is set to an empty array, it explicitly says not to use STUN server

Unless explicitly said, it should always use a STUN server. That's my rationale.

[ibc]

I don't like the idea of relying on any server given that the domain may change or whatever, but I'm ok with that. It's harmless.

[jmillan]

IMHO it's a plus if we don't force the developer to set a stun server in order to be able to actually use one, transparently.

[ibc]

Fine. Can we close this issue? (not sure if it's currently implemented).

[mEkblom]

As far as I can see this is still not fixed. Default should be no STUN servers in my opinion.

Because I need to use no STUN servers I currently use this patch (only add servers if available):
mEkblom@26c05ba (mEkblom@418eb30)
stun_servers is set to empty array, using the master branch. Possibly strict type checking might prevent this from working in the devel branch, I didn't try.

[ibc]

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

• JsSIP New Design

Thanks to all for your contribution to the project.