Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTCIceCandidate format #28

Closed
dbrgn opened this issue May 17, 2016 · 8 comments
Closed

RTCIceCandidate format #28

dbrgn opened this issue May 17, 2016 · 8 comments

Comments

@dbrgn
Copy link
Member

dbrgn commented May 17, 2016

The spec for the candidate message is as follows:

{  
   "type": "candidates",
   "session": b"bb938a583e67923c6e9f2185a6a03773",
   "sdp": [
       "...",
       "..."
   ]
}

That assumes that the candidates are sdp strings. In the WebRTC RTCIceCandidate structure, that is contained in the candidate string attribute. On the other hand, ORTC does not provide such an attribute: http://ortc.org/wp-content/uploads/2015/06/ortc.html#h-rtcicecandidate

Can we simply send the entire RTCIceCandidate structure instead? Or is that too implementation-specific? Maybe we can define a key mapping?

@lgrahl
Copy link
Member

lgrahl commented May 17, 2016

I agree. The RTCIceCandidate interface should be pretty fixed - I'm not expecting any changes to it for either WebRTC or ORTC. However, they should be treated as individual types.
Also, we want to be independent from the browser's implementations. Thus, the MessagePack serialisation must be crystal clear in the protocol spec.

@dbrgn
Copy link
Member Author

dbrgn commented May 18, 2016

ORTC:

dictionary RTCIceCandidate {
             DOMString              foundation;
             unsigned long          priority;
             DOMString              ip;
             RTCIceProtocol         protocol;
             unsigned short         port;
             RTCIceCandidateType    type;
             RTCIceTcpCandidateType tcpType;
             DOMString              relatedAddress = "";

WebRTC:

dictionary RTCIceCandidateInit {
    required DOMString      candidate;
             DOMString      sdpMid;
             unsigned short sdpMLineIndex;
};

[ Constructor (RTCIceCandidateInit candidateInitDict)]
interface RTCIceCandidate {
    readonly        attribute DOMString               candidate;
    readonly        attribute DOMString?              sdpMid;
    readonly        attribute unsigned short?         sdpMLineIndex;
    readonly        attribute DOMString               foundation;
    readonly        attribute unsigned long           priority;
    readonly        attribute DOMString               ip;
    readonly        attribute RTCIceProtocol          protocol;
    readonly        attribute unsigned short          port;
    readonly        attribute RTCIceCandidateType     type;
    readonly        attribute RTCIceTcpCandidateType? tcpType;
    readonly        attribute DOMString?              relatedAddress;
    readonly        attribute unsigned short?         relatedPort;
    serializer = {candidate, sdpMid, sdpMLineIndex};
};

It looks like WebRTC uses the candidate string together with sdpMid and/or sdpMLineIndex to create an RTCIceCandidate object. In ORTC you construct that object directly.

From the spec I did not really understand what the sdpMid and sdpMLineIndex do. But we could probably choose the same structure as ORTC and then construct the candidate string from that in WebRTC?

@lgrahl
Copy link
Member

lgrahl commented May 18, 2016

Let's just send the whole RTCIceCandidate instance as defined above as a generic JSON object (serialised to a MessagePack map on the wire and unserialised to a generic JSON object before it's being passed to an onCandidate event) and leave it up to the users which fields they want to use to reconstruct the RTCIceCandidate. The SDP-stuff is not really well explained because it's not meant to be modified and... I don't really understand it either. (Personally I think SDP is an unnecessary burden - that's why I like ORTC.)

@dbrgn
Copy link
Member Author

dbrgn commented May 19, 2016

Ok, so this would be the RTCIceCandidate?

interface RTCIceCandidate {
    foundation: string,
    priority: number, // unsigned long
    ip: string,
    protocol: 'tcp' | 'udp', // RTCIceProtocol
    port: number, // unsigned short
    type: 'host', 'srflx', 'prflx', 'relay', // RTCIceCandidateType
    tcpType?: 'active' | 'passive' | 'so', // RTCIceTcpCandidateType
    relatedAddress?: string,
    relatedPort?: number, // unsigned short
}

@lgrahl
Copy link
Member

lgrahl commented May 21, 2016

Nearly. We'll also send the fields candidate, sdpMid and sdpMLineIndex in case WebRTC is being used.

@dbrgn
Copy link
Member Author

dbrgn commented May 22, 2016

Well, but what if both candidate and the other fields are filled out with conflicting information? Which one is authoritative?

If it's easy to deduce the candidate from the fields above, I don't think we should include redundant information.

@lgrahl
Copy link
Member

lgrahl commented May 22, 2016

I'm with you - yes, the information is redundant. But you can't reconstruct an RTCIceCandidate instance without candidate, sdpMid and sdpMLineIndex (see the RTCIceCandidate interface constructor). At least not for WebRTC.

For us, another option would be to just send candidate, sdpMid and sdpMLineIndex for WebRTC and the well defined RTCIceCandidate interface for ORTC (which is also slightly different to the one that WebRTC defines).

@dbrgn
Copy link
Member Author

dbrgn commented May 23, 2016

That would probably be the best approach for now. The implementors know which library they want to use :)

@lgrahl lgrahl added the tasks label Sep 1, 2016
@lgrahl lgrahl added this to the Add 'Tasks' milestone Sep 1, 2016
@lgrahl lgrahl removed this from the Add 'Tasks' milestone Sep 24, 2016
@lgrahl lgrahl mentioned this issue Sep 24, 2016
@lgrahl lgrahl closed this as completed Sep 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants