mirror of
https://github.com/mollyim/webrtc.git
synced 2025-05-12 21:30:45 +01:00
2c1cfd047f
This CL removes the send buffers (but not the receive buffer) from
SctpDataChannel and increases the send buffer in DcSctpSocket instead.
The reasons are:
1) Simplify the code. This additional buffering was strictly needed
before we migrated away from usrsctp, as that send buffer was very
limited in size (by design). But with the migration to dcSCTP, it's
no longer needed, so it just adds complexity.
2) Make `RTCDataChannel::bufferedAmount` correct. Before this CL, it
represented just the data buffered in SctpDataChannel, and not the
data accepted by the SCTP socket, but not yet put on the wire. This
makes it hard for clients to know when a message has ever been sent.
3) Better handle draining data on data channel close. While this is not
implemented in dcSCTP, having a single buffer makes this easier to
add.
While most of this CL is straightforward, the handling of bufferedAmount
in the signaling thread (in RTCDataChannel in Blink), is a bit special.
The number returned by `RTCDataChannel::bufferedAmount` is not what the
true value is inside the SCTP socket, but an eventual consistent view
of that value. When a message is sent, the value is incremented and:
- Before this change: When a message was put on the SCTP socket, the
view's value was decremented. Which made the view reflect what was
buffered outside the SCTP socket, and that buffering is now gone.
- After this change: SctpDataChannel will track what RTCDataChannel
will think it is, and provide updates to that number as we are
notified that it's reduced - by setting a "low threshold" callback
trigger.
A bonus with the new behavior is that it will be eventually consistent
and auto-heal also in error conditions - when messages are dropped due
to errors (bad input, bad state, etc). Previously, the bufferedAmount
value could drift away from the correct value on errors.
Note that a big chunk of unit tests were removed with this CL, as those
tested how the buffering behaved. Now, there is no buffering, so the
removed test cases represent a simpler interface.
This CL has been extensively tested with data channel benchmarks that
use the bufferedAmount thresholds (in Javascript).
Bug: chromium:40072842
Change-Id: I1a6a4af6b6e1116832f5028f989ce9f44683d229
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343361
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41945}
|
||
|---|---|---|
| .. | ||
| dcsctp_transport.cc | ||
| dcsctp_transport.h | ||
| dcsctp_transport_unittest.cc | ||
| OWNERS | ||
| sctp_transport_factory.cc | ||
| sctp_transport_factory.h | ||
| sctp_transport_internal.h | ||