RTCIceCandidate.tcpType for UDP candidates #608

Closed
lgrahl opened this Issue Oct 18, 2016 · 6 comments

Projects

None yet

3 participants

@lgrahl
lgrahl commented Oct 18, 2016

Which RTCIceTcpCandidateType should a RTCIceCandidate with udp protocol have?

Should the field be optional? Should it be empty string or null for UDP candidates? The spec should describe that.

@taylor-b

In WebRTC it's just optional, I think that makes the most sense.

@lgrahl
lgrahl commented Oct 18, 2016

Making it optional would be inconsistent regarding relatedAddress and relatedPort which are emtpy string and 0 instead of being optional.

@lgrahl
lgrahl commented Oct 18, 2016

But in general, I agree that ORTC should mimic RTCIceCandidate of the WebRTC spec. However, the WebRTC spec is not consistent here either as it makes the fields optional but in the description their values should be null if not available. To me, null is not the same thing as not set (optional).

@taylor-b

I forgot; in the WebRTC spec, RTCIceCandidate is an interface rather than a dictionary, so the only option is to have nullable attributes. So it's hard to draw an exact parallel. But null would be a closer match than an empty string.

@lgrahl
lgrahl commented Oct 19, 2016

Oh, thanks, that explains it. null would be okay for tcpType, but relatedAddress and relatedPort would have to be updated as well to avoid inconsistencies. However, setting relatedPort to null would violate its type. This is somewhat problematic for APIs written in languages that require static types. To me, making all three fields optional seems to be the best solution so far.

@aboba aboba self-assigned this Oct 25, 2016
@aboba
Contributor
aboba commented Oct 26, 2016

@lgrahl The optionality of the three fields can be made clear by adding "required" to the other fields.

@aboba aboba added the PR exists label Oct 26, 2016
@aboba aboba added a commit that referenced this issue Oct 26, 2016
@aboba aboba Optional/required attributes in RTCIceCandidate
Fix for Issue #608

Related to Issue #514
c7517c1
@lgrahl lgrahl closed this Oct 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment