janus.js uses an incorrect method call to mark end-of-candidates

[mykola-mokhnach]

We are experiencing the following error in Chrome when sending the watch request to the streaming plugin:

chrome_shim.js:688 Uncaught (in promise) TypeError: Failed to execute 'addIceCandidate' on 'RTCPeerConnection': Candidate missing values for both sdpMid and sdpMLineIndex
    at RTCPeerConnection.window.RTCPeerConnection.addIceCandidate (chrome_shim.js:688)
    at janus.js:1803
window.RTCPeerConnection.addIceCandidate @ chrome_shim.js:688
(anonymous) @ janus.js:1803

Quick search on the interned shows the problem might be in this part of janus.js codebase:

					// Any trickle candidate we cached?
					if(config.candidates && config.candidates.length > 0) {
						for(var i in config.candidates) {
							var candidate = config.candidates[i];
							Janus.debug("Adding remote candidate:", candidate);
							if(!candidate || candidate.completed === true) {
								// end-of-candidates
								config.pc.addIceCandidate();
							} else {
								// New candidate
								config.pc.addIceCandidate(candidate);
							}
						}
						config.candidates = [];
					}

The spec at https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/addIceCandidate#Example says end-of-candidates should be sent as

pc.addIceCandidate({candidate: ''});

References:
w3c/webrtc-pc#1952 (comment)
w3c/webrtc-pc#2039

[lminiero]

I guess you're using full-trickle in Janus, and a more recent version of the adapter that I think our demos are still using? That might explain why the old "let's just pass null" thing that always worked now maybe doesn't. I'll make the fix, thanks for the heads up 👍

[lminiero]

Fixed in 345cc5e (wrong commit message so this didn't close the issue automatically).

[lminiero]

What version of the adapter are you using @mykola-mokhnach? I've got reports that this fix actually broke things for them, and I can confirm that when doing full-trickle I'm getting an exception as well:

Uncaught (in promise) TypeError: Failed to construct 'RTCIceCandidate': sdpMid and sdpMLineIndex are both null.
    at RTCPeerConnection.r.RTCPeerConnection.<computed> (adapter.min.js:1)
    at RTCPeerConnection.r.RTCPeerConnection.addIceCandidate (adapter.min.js:1)
    at janus.js:2414

This is with adapter 6.4.0. Unless you give me a good reason, I'll have to change that again to null.

[mykola-mokhnach]

It looks like different browsers handle it differently. In Firefox, for example, everything works smoothly, but Chrome and Safari are complaining about this line. It's such a mess %/

[lminiero]

No, Firefox complains as well:

TypeError: Either sdpMid or sdpMLineIndex must be specified

[lminiero]

Reverted for now, using a new variable that we can redifine later on to change behaviour if needed.