search

w3c / ortc

ORTC Community Group specification repository (see W3C WebRTC for official standards track)
http://www.w3.org/community/ortc/
122 stars 42 forks source link

QUIC: Add buffering support to write() #793

Closed aboba closed 7 years ago

aboba commented 7 years ago

This patch adds support for buffering to the RTCQuicStream.write method. The bufferedAmount, bufferedAmountLowThreshold and onbufferedamountlow attributes are utilized to enable building an RTCDataChannel interface on top of the RTCQuicStream abstraction.

@lgrahl @pthatcher Can you review/comment?

pthatcherg commented 7 years ago

I'm not a fan of this model of supporting buffering. Two reasons:

  1. It only provides control for buffering on the send/write side, not on the read/receive side.
  2. It's clunky.

On the other hand, it is easy to specify and implement, and I think that it's possible to implement the API I would prefer (waitForWritable) on top of it.

If we came up with a matching buffering solution for the read/receive side, I could overlook the clunkyness and look at its good characteristics. But I feel strongly that we should figure out a solution for both sides of buffering before proceeding with either.

lgrahl commented 7 years ago

I agree with @pthatcherg. The API model is clunky for data channels and it's clunky for QUIC streams, too.

Like I said in #785, the idealist in me wants to see an inherited WHATWG stream. But if that's too much to ask, then I'd go for write and drain (instead of the onbufferedamountlow event) as suggested in this comment before. That proposal may also cover aspects of read buffering (I'm not exactly sure what @pthatcherg has in mind there). Feedback to that proposal would be much appreciated.