-
Notifications
You must be signed in to change notification settings - Fork 856
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] Improved RTT estimation #1957
Merged
maxsharabayko
merged 12 commits into
Haivision:master
from
mbakholdina:mbakholdina/reset-rtt-after-initialization
Apr 27, 2021
Merged
[core] Improved RTT estimation #1957
maxsharabayko
merged 12 commits into
Haivision:master
from
mbakholdina:mbakholdina/reset-rtt-after-initialization
Apr 27, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was
linked to
issues
Apr 19, 2021
mbakholdina
force-pushed
the
mbakholdina/reset-rtt-after-initialization
branch
from
April 23, 2021 09:48
eaaaec0
to
0807374
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor corrections.
maxsharabayko
added
the
Type: Maintenance
Work required to maintain or clean up the code
label
Apr 27, 2021
maxsharabayko
approved these changes
Apr 27, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
[core]
Area: Changes in SRT library core
Type: Maintenance
Work required to maintain or clean up the code
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improvements Done
Improved RTT estimation in the following way:
INITIAL_RTT = 100 ms
, or is taken from cache. It remains so until the very first ACKACK packet is received from the sender and triggers RTT sample calculation. At this moment, the smoothed RTT is reset to the very first RTT sample and the EWMA is applied further on the subsequent RTT samples.The initial values of RTT and RTT variance are either equal to
INITIAL_RTT = 100 ms
andINITIAL_RTTVAR = 50 ms
, or taken from sender's cache. Upon ACK packet reception, they can be reset to the receiver's RTT and RTTVar extracted from the ACK if those values are taken from the receiver's cache. Initial values of 100 ms and 50 ms will be ignored. Receiver's cache is of a higher priority meaning that in case of both peers have cache, receiver's cache wins.-DENFORCE_SRT_DEBUG_RTT=1
withcmake
to activate it.Fixes #782, #1769, #1818.
Test Cases
The tests were performed with the help of
srt-xtransmit
application with the following test parameters:(see test case 6 from #1937).
The SRT library was built with the advanced RTT log plus the usage of monotonic clocks:
Test Case 1. Unidirectional transmission. Initial values for smoothed RTT and RTTVar are applied on both sides.
Test Case 2. Unidirectional transmission, resumed connection. Initial value for smoothed RTT is taken from cache.
2.1. Sender-caller, receiver-listener, reconnection on the receiver side. The cache is available on the receiver side only.
The logic of applying the values from cache works correct, however, during this test, I've faced the following bug #1958.
The sender application has been restarted 4 times. As we can see from the first picture, for the first restart (see row 1859), the values of smoothed RTT and RTTVar are equal to the initial values of 100 ms and 50 ms, because the values from cache are not yet available. The socket is in process of closing, however it's not yet closed when the reconnection happens.
As can be seen from the following pictures, when the second reconnection happens (see row 3717), the value from cache is available (500288), however this value isn't correct and actually taken from the very first session.
2.2. Sender-listener, receiver-caller, reconnection on the sender side. The cache is available on the sender side only.
Haven't faced the bug #1958 as in test case 2.1.
2.3. Reconnection on both sides, the cache is available on both sides.
srt-xtransmit
application.Test Case 3. Bidirectional transmission.
develop/generate-null-receiver branch of
srt-xtransmit
was used to test the case of bidirectional transmission.The very first implementation showed that there was a variation in smoothed RTT and RTTVar at the beginning of a transmission. The screenshot below was taken on the caller side. In case of bidirectional transmission, at each peer we receive both ACK and ACKACK packets. The very first packet triggers the reset of smoothed RTT, however later there is still a chance to receive ACK packets with initial RTT value of 100 ms which will be taken into account while calculating smoothed average.
In the improved version, these values are simply ignored.