-
Notifications
You must be signed in to change notification settings - Fork 635
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
Do not include packet in RED if timestamp is too far back. #1478
Conversation
} | ||
r.pktBuff[i] = pkt | ||
break | ||
} |
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.
Re-doing this a bit to catch scenarios like following
- Packet arrival sequence is 5, 7, 6
- When 6 is received, the history would have 5, 7
- RED encoding would have been 5, 6 (only one redundancy)
- Previously, the history would not have been changed as 6 is out-of-order with the last element. So, when 8 was received, it would have been encoded as 7, 8 (only one redundancy as 5 is too old)
Changing it to walk back and find a location for the new packet. So, after 6, the history would be 6, 7 and when 8 comes around it can be encoded with 6, 7, 8 (two redundancies)
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.
Looks good, thanks for fixing the timestamp overflow! It would be better to have a test case for this
Will add a test case @cnderrauber |
@cnderrauber Added a test case in this commit (ac5721d). Can you please review when you get a chance? |
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.
Looks good!
No description provided.