Buffer RT(C)P packets that arrive pre-DTLS handshake completion #160
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #160 +/- ##
==========================================
- Coverage 88.51% 88.44% -0.07%
==========================================
Files 38 38
Lines 1916 1922 +6
==========================================
+ Hits 1696 1700 +4
- Misses 220 222 +2
Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
For now it's probably fine, but we should also consider buffering DTLS packets that came out of order during the handshake (I assume that ExDTLS does not take care of that, so we would need to do something in {:error, reason} clause in the big case ExDTLS.handle_data(state.dtls, data) do statement.
Another thing I was thinking of is buffering packets send by us, because as of now we just throw a log and discard the packets.
If you don't want to take care of these and split attention between this and jitter buffer, just add a comment, especially about the DTLS packets, and I'll take care of that next week.
| @@ -108,6 +108,7 @@ defmodule ExWebRTC.DTLSTransport do | |||
| ice_connected: false, | |||
| buffered_local_packets: nil, | |||
| buffered_remote_packets: nil, | |||
| buffered_remote_rtp_packets: [], | |||
There was a problem hiding this comment.
We aren't sure if these are RTP packets, it might as well be RTCP (or even SCTP data if we account for it, see other comment), so the name is a bit misleading.
I was thinking of reusing the buffered_remote_packets, as it's used for packets before negotiation, then is flushed (affair), so it could be used for that, although this might make the code a bit more cryptic.
There was a problem hiding this comment.
Yeah, I was wary of reusing the same buffer because of that. If you have suggestions for alternative names, I'm all ears
There was a problem hiding this comment.
For now leave it, I'll think about it when taking care of buffering DTLS data.
|
@LVala added a TODO comment in the place you mentioned |
This is exceedingly rare but can happen in extremely poor network conditions due to jitter -- and when it happens, it results in dropping the initial keyframe