Fixes bug in group accumulator#291
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #291 +/- ##
==========================================
- Coverage 71.34% 71.31% -0.03%
==========================================
Files 80 80
Lines 4477 4483 +6
==========================================
+ Hits 3194 3197 +3
- Misses 1151 1153 +2
- Partials 132 133 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| func (g *arrivalGroup) add(a cc.Acknowledgment) { | ||
| g.packets = append(g.packets, a) | ||
| g.arrival = a.Arrival | ||
| g.departure = a.Departure |
There was a problem hiding this comment.
This is one of the main changes, the departure time of the group is set to the departure time of the first packet in the group. This makes sense because the point of the packet group is to reduce a group of packets down as if it was all 1 packet.
| return 0 | ||
| } | ||
| return b.Departure.Sub(a.packets[len(a.packets)-1].Departure) | ||
| return ack.Departure.Sub(group.departure) |
There was a problem hiding this comment.
Second change is here. The inter departure time is the difference between the ack departure time and the departure time of the group.
This is equivalent to the code here in libwebrtc
|
@sterlingdeng thank you for going so deep on this code. I really appreciate it! Mind squashing your commit into one and doing a commit message with Subject+Body (just describing what you did). Your comments on the PR itself work great. In the future having a nice thanks again! |
06a4ff4 to
7469b9c
Compare
|
@mengelbart FYI since you were the original author.. |
|
@Sean-Der No problem, glad to help. Regarding CI: Not sure why it's failing now.. perhaps flakiness? |
This fixes issue pion#274.
7469b9c to
bad09be
Compare
Description
See referenced issue for background
Reference issue
Fixes #274