-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add QUIC transport #600
Add QUIC transport #600
Conversation
Very rough PR to add support for QUIC transport. Not ready to merge. Related to Issue #584
small fixes
More fixes
typedef (RTCDtlsTransport or RTCQuicTransport) RTCTransport;
Add typedef for RTCTransport
@@ -2564,7 +2564,7 @@ | |||
<h3>Operation</h3> | |||
<p>A <code><a>RTCRtpSender</a></code> instance is constructed from an | |||
<a>MediaStreamTrack</a> object and associated to an | |||
<code><a>RTCDtlsTransport</a></code>. If an attempt is made to construct an | |||
<code><a>RTCTransport</a></code>. If an attempt is made to construct an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is for sending SRTP, why not call it an RTCSrtpTransport or RTCSecureTransport (just like we've done for RTCDataTransport for data channels)? Then have both DtlsTransport and QuicTransport inherit from it and you don't need the typedef.
@@ -7724,6 +7731,429 @@ | |||
</pre> | |||
</section> | |||
</section> | |||
<section id="rtcquictransport*"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where will the mapping from RTP/RTCP to QUIC streams be defined?
<pre class="idl"> | ||
[ Constructor (RTCIceTransport transport, sequence<RTCCertificate> certificates)] | ||
interface RTCQuicTransport : RTCStatsProvider { | ||
readonly attribute FrozenArray<RTCCertificate> certificates; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't an RTCQuicTransport also be an RTCDataTransport?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it should be.
interface RTCQuicTransport : RTCStatsProvider { | ||
readonly attribute FrozenArray<RTCCertificate> certificates; | ||
readonly attribute RTCIceTransport transport; | ||
readonly attribute RTCQuicTransportState state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where will the mapping from data messages to QUIC streams be defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this is good, but it looks like it is missing a few things.
Add implementation note
Very early PR to add support for QUIC transport. Not ready to merge.
Related to Issue #584
Missing pieces: