Fixes bug in group accumulator #291
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #291 +/- ##
==========================================
+ Coverage 71.27% 71.38% +0.10%
==========================================
Files 80 80
Lines 4477 4483 +6
==========================================
+ Hits 3191 3200 +9
+ Misses 1153 1151 -2
+ Partials 133 132 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
| 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! |
This fixes issue pion#274.
sterlingdeng
force-pushed
the
sdeng/fix_departure_time_group
branch
from
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? |
Description
See referenced issue for background
Reference issue
Fixes #274