[RemoteBitrateEstimator] Use the RTP extension abs-send-time to correctly calculate inter-departure time. #239
Conversation
|
Hi, thanks for your contribution! |
|
By the way, we have now signed the Atlassian Corporation CLA for Highfive Technologies, Inc. |
|
Thanks for the contribution! The CLA is probably fine, but jenkins has a separate whitelist...not sure if adding there works right now, so ignore him if he goes on. Jenkins, add to whitelist please. |
|
Yep we need to add contributors per project using the command in comments or directly into jenkins interface, adding usernames. Maybe we can request a feature in the plugin to have the global admins inherited in all pr-testing projects. |
|
I'm not sure how to read what failed in this test. How may I run this test? |
…ctly calculate the inter-departure time. Jitsi uses the RTP timestamp for the calculation of inter-departure time. This is not ideal because the RTP timestamp is recorded before the pacer introduces artificial delay in the inter-arrival times. We fixed this by doing the same as Chrome does and use the RTP extension abs-send-time to calculate deltas in the remote bitrate estimator. With calls on meet.jit.si, the bitrate will unexpectedly drop. This is most noticeable with screenshare content in the following scenario: 1) Artificially high inter-group delay variation (d(i) in draft-ietf-rmcat-gcc-02) because the departure times are recorded before the pacer queue. Overuse is asserted. 2) If overuse occurs during a static screenshare scene, the bitrate drops to 30kbps due to a ratcheting down to the low incoming bitrate. 3) High dynamic scene changes then cause the encoded bitrate to far exceed the target bitrate. 4) The pacer limits the transmit rate to the target bitrate which can cause the packets to be sent out over an interval of 5-10 seconds. Test Plan: Unit test covering the affected logic when we move to the 24-bit abs-send-time. Manual tests of scenarios where the bitrate drop to 30kbps caused high screenshare latency. Reviewers: ed, ethan, hemanth Subscribers: eng-team-list Differential Revision: https://phab.fatline.io/D4188
imedwei
force-pushed
the
abs-send-time
branch
from
fd52114
to
928edcd
Compare
|
Seems like that was a flaky build? When I pushed some minor javadoc changes, the build succeeded. |
|
@bgrozev Anything we need to do to get this this change upstreamed? We noticed a dramatic improvement in our screenshare latency performance as well as a reduction in bitrate drops for camera video. |
|
@imedwei after just a quick glance there's one thing that I'd like to avoid -- defining the abs-send-time extension id as a constant in RawPacket. I'm working on adding some code which handles extensions in a somewhat abstract way right now. What do you think about waiting for a couple of days (next week, realisticly) and then refactoring a little to use that? I'll have abs-send-time reading support in mind. |
|
That sounds fine to me. Thanks for the look. Since these are among my first upstream commits to jitsi, I'm grateful for any feedback so that we can improve our current and future commits. |
|
Hey @imedwei, In order to get the negotiated ID of the abs-send-time extension you can expose MediaStreamImpl#absSendTimeEngine to VideoMediaStreamImpl, and then use its extensionID field. I would also prefer to keep the abs-send-time specific code in AbsSendTimeEngine.java. Some general comments based on our code convention: Thanks again for the contribution. |
| boolean shouldRun() | ||
| { | ||
| int nowMs = (int) (timeProvider.getTimeMs()); | ||
| if (((nowMs - lastUsedInstantMs) & 0xffff_ffffL) > minBackoffMs) |
There was a problem hiding this comment.
"0xffff_ffffL"
This is awesome. I wish I knew about this syntax earlier :)
Jitsi uses the RTP timestamp for the calculation of inter-departure
time. This is not ideal because the RTP timestamp is recorded before
the pacer introduces artificial delay in the inter-arrival times. We
fixed this by doing the same as Chrome does and use the RTP extension
abs-send-time to calculate deltas in the remote bitrate estimator.
With calls on jitsi, the bitrate sometimes will unexpectedly drop. This is
most noticeable with screenshare content in the following scenario:
because the departure times are recorded before the pacer queue. Overuse is asserted.
drops to 30kbps due to a ratcheting down to the low incoming bitrate.
exceed the target bitrate.
cause the packets to be sent out over an interval of 5-10 seconds.
Test Plan:
Unit test covering the affected logic when we move to the 24-bit abs-send-time.
Manual tests of scenarios where the bitrate drop to 30kbps caused high
screenshare latency.