Add interceptor to aggregate CCFB reports
Description
This interceptor keeps track of outgoing RTP packets and reads incoming CCFB (RFC8888) RTCP packets. For each incoming CCFB packet, it creates a report of the included acknowledgments and stores it in the interceptor attributes. Applications reading RTCP packets from it can read the report, e.g., to perform bandwidth estimation.
This is a PoC to improve bandwidth estimation in Pion. The current GCC implementation is hard to use and test. Giving applications access to the feedback reports could simplify and decouple the actual bandwidth estimation from the interceptors.
TODO
- [x] Add unit tests
- [x] Implement TWCC converter (currently, only RFC 8888 is supported)
- [x] Remove old entries from history
- [x] Document the semantics of the generated reports (currently, it generates a report for every packet included in the feedback packet, including the sequence number, size, departure timestamp, status (received or not), arrival timestamp, and ECN bits.
Codecov Report
:x: Patch coverage is 82.83133% with 57 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 78.94%. Comparing base (ba697c7) to head (6e00a00).
Additional details and impacted files
@@ Coverage Diff @@
## master #300 +/- ##
==========================================
+ Coverage 78.68% 78.94% +0.25%
==========================================
Files 82 86 +4
Lines 5109 5437 +328
==========================================
+ Hits 4020 4292 +272
- Misses 919 966 +47
- Partials 170 179 +9
| Flag | Coverage Δ | |
|---|---|---|
| go | 78.94% <82.83%> (+0.25%) |
:arrow_up: |
| wasm | 76.51% <82.53%> (+0.41%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
This is so exciting! I am glad to see you back @mengelbart :)
@jech and @joeturki are getting deep into this world if you are looking for people to work with/quick reviews
Could you please give some background? Why not simply provide a function that parses the reports?
To my untrained eyes, it looks like your interceptor aims to allow a client to do:
_, attr, _ := track.ReadRTCP()
data := attr.Get(CCFBAttributesKey)
Without an interceptor, the same code would look like this:
p, _, _ := track.ReadRTCP()
var data CCFBReportPacket
data.Unmarshal(p.Data)
Now, perhaps I'm biased against interceptors (which are causing me a fair amount of suffering), but the second code looks clearer to me. What am I missing?
The interceptor also keeps track of outgoing RTP packets, timestamps them, and matches RTCP reports to the history of sent packets. The report you get from the attributes then contains departure and arrival timestamps for each packet that was reported on in the RTCP. Without the interceptor, you have to keep track of the departure timestamps yourself.
I tried to make all of the new linters happy, but I added nolint in a few places...
Hi @mengelbart what a cool PR! I'm curious to learn what's the relationship between this interceptor, the pacer, and bandwidth estimator GCC will be like by your design. For users using this interceptor, they need to make sure it goes after the pacer right? Otherwise the captured send time will be incorrect. I also take a brief look at the GCC refactor, and it seems like pacing is not included in it.
@3DRX right, pacing is not included in the GCC refactoring PR, because the new design only works on received feedback and everything it does is independent of the packet transmission part. Pacing can still be done in a new interceptor or even before writing packets to the track.