[BUG]: end of candidates indicator

[FabienDanieau]

Package version

3.0.0-pre.6

Environment

* OS: Windows 10
* Unity version: 2022.3.5f1

Steps To Reproduce

I'm able to connect to gstreamer signalling server, and connect to audio/video producers. Local tests work well.

In real conditions, with two different computers, I receive from the gstreamer producer that makes the SDP offer a null ice candidate {"type":"peer","sessionId":"c375043d-1145-4867-99de-97ed0240b898","ice":{"candidate":"","sdpMLineIndex":0}}. I believe this is supposed to mean the end of the sending of ice candidates (end of candidates indicator)

Current Behavior

Unity fails to make an empty ice candidate

ArgumentException: create candidate is failed. error type:InvalidParameter, candidate:
sdpMid:
sdpMLineIndex:0
  at Unity.WebRTC.RTCIceCandidate..ctor (Unity.WebRTC.RTCIceCandidateInit candidateInfo) [0x000c1] in .\Library\PackageCache\[email protected]\Runtime\Scripts\RTCIceCandidate.cs:264 

I am supposed to simply ignore this ice candidate? (if I do so the connection if ended by gstreamer but I don't know exactly why yet)

Expected Behavior

This case should be managed by Unity. For now this function is designed to refuse any empty candidate

        public RTCIceCandidate(RTCIceCandidateInit candidateInfo = null)
        {
            candidateInfo = candidateInfo ?? new RTCIceCandidateInit();
            if(candidateInfo.sdpMLineIndex == null && candidateInfo.sdpMid == null)
                throw new ArgumentException("sdpMid and sdpMLineIndex are both null");

            RTCIceCandidateInitInternal option = (RTCIceCandidateInitInternal)candidateInfo;
            RTCErrorType error = NativeMethods.CreateIceCandidate(ref option, out self);
            if (error != RTCErrorType.None)
                throw new ArgumentException(
                        $"create candidate is failed. error type:{error}, " +
                        $"candidate:{candidateInfo.candidate}\n" +
                        $"sdpMid:{candidateInfo.sdpMid}\n" +
                        $"sdpMLineIndex:{candidateInfo.sdpMLineIndex}\n");

            NativeMethods.IceCandidateGetCandidate(self, out _candidate);
        }
    }

Anything else?

No response

[karasusan]

@FabienDanieau Can we solve your issue if I remove lines in the code which throws an ArgumentException ? This is because I write the code to make protective from unexpected parameters.

[FabienDanieau]

Can we solve your issue if I remove lines in the code which throws an ArgumentException ?

Somehow. If I comment these lines in the plugin's code it get reverted automatically when Unity compiles. But for some reason it stayed on another computer. So the error disappears but not my problem. I had a similar behavior just by ignoring this null ice candidate. It seems that gstreamer is ending the connection so the problem might be not on Unity side.

Anyway, according to gstreamer people this null candidate can happen, so I believe this case should be managed.

[karasusan]

@FabienDanieau Unity checks package folder and if files are changed, redownload them automatically. You need to download the package on your machine, and install the package to your Unity project.

There is a document explains how to install the package from local disk. https://docs.unity3d.com/Manual/upm-ui-local.html

The package is here. You need to extract tgz file and install following above url. https://github.com/Unity-Technologies/com.unity.webrtc/releases/tag/3.0.0-pre.6

[karasusan]

@FabienDanieau I can make a package which I comment these lines out and provide it to you. I am not familier with gstreamer, can you help me to fix it?

[FabienDanieau]

Thanks I was able to make the changes, it seems to work but I'd like to test it more.

This is not specific to gstreamer. An empty ice candidate can happen (https://developer.mozilla.org/en-US/docs/Web/API/RTCIceCandidate). Shall we just remove the argument exception and pass this empty candidate to NativeMethods.IceCandidateGetCandidate(self, out _candidate); or is there more processing to do? Like closing the connection if there are no valid ice candidate pairs?

[karasusan]

@FabienDanieau OK, I'll remove the argument exception and check it.

[karasusan]

memo: WRS-498

[gtk2k]

Finally, this allows us to negotiate normally with Vanilla ICE. . .

[karasusan]

@FabienDanieau

Let me clarify the specification.

Previously, RTCIceCandidate would return an exception when an empty candidate was passed in the constructor.

In the proposed design, it ignores when candidate is empty; the CreateIceCandidate function defined in C++ returns an error, so the instantiation fails internally. Many properties in RTCIceCandidate refer to the instance, so if the instance does not exist, null or an invalid value is returned.

Is this OK?

[FabienDanieau]

Ignoring the message seemed to work for me. If you create a branch with this fix I can test it.

But aren't you supposed to do something when an end of ice candidates is received? The spec (https://www.rfc-editor.org/rfc/rfc8838#name-receiving-an-end-of-candida) mentions

When an agent receives an end-of-candidates indication for a specific data stream, it will update the state of the relevant checklist or

After an agent has received an end-of-candidates indication, it MUST ignore any newly received candidates for that data stream or data session.

Is it relevant here? BTW, does Unity sends a end of ice candidates?

[karasusan]

@FabienDanieau The CreateIceCandidate function returns an error Expect line: candidate:<candidate-str> when the candidate is empty.

It looks Google Chrome doesn't support end-of-candidate . So we need to have a special process for empty candidate. https://bugs.chromium.org/p/chromium/issues/detail?id=935898

[FabienDanieau]

Ok I see. So just ignoring this ice candidate works in my situation. We can either leave it this way and I use a try catch to catch this throw new ArgumentException("sdpMid and sdpMLineIndex are both null"); , or the function returns a null candidate. Then in both case I don't add this candidate in the list of candidates. What do you think?

[gtk2k]

Generating an event when candidate is empty is a behavior that is also mentioned in the WebRTC API specifications, so I think it is wrong to implement this behavior by replacing it with an exception. I think it's better to set candidate to null and dispatch the onIceCandidate event.

Due to the implementation of WebRTC for Unity, if there is no instance of iceCandidate, an exception will occur, so I think it would be nice to be able to generate an iceCandidate instance with the candidate property set to an empty string or null.

[karasusan]

Please review it. https://github.com/Unity-Technologies/com.unity.webrtc/pull/988

[FabienDanieau]

Please review it. #988

Thanks. I'll test it next week