saltyrtc / saltyrtc-meta

Protocol description and organisational information for SaltyRTC implementations.
MIT License
74 stars 8 forks source link

RTCIceCandidate format #28

Closed dbrgn closed 8 years ago

dbrgn commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

dbrgn commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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