Skip to content
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

Drop padding only packets on publisher side. #1990

Merged
merged 8 commits into from
Aug 24, 2023
Merged

Conversation

boks1971
Copy link
Contributor

Probably a good time to bring in the extended range implementation in this PR - #1548.

But, for now, just returning extended sequence number from RTPStats Update and storing offsets in a new struct called RangeMap. RangeMap is used to store a value for certain ranges. It is used as follows

  • When a padding packet is dropped, incoming sequence number is adjusted to ensure continuity of sequence numbers.
  • When there are losses, a retransmission (or out-of-order arriving) packet needs sequence number adjustment. How much to adjust needs to stored so that it can be applied when that packet arrives.
  • It needs to be stored because a later padding only packet might be dropped and the adjustment changes. So, when a gap/drop is detected, the adjustment at that time needs to be cached and re-applied later.

In this implementation (although it is using generics), extended sequence number is calculated (16 bits extended to 32 bits) and a certain number of ranges are maintained (100 in this PR to limit memory). If there is a retransmission/out-of-order arrives, adjustment is retrieved from the cache. For in-order packets, the running value is used for adjustment.

With this change, it is possible to simplify the subscriber side, i. e. can skip checking for padding only packets on subscriber side. But, will do that change in a different PR if this change works fine. Have tested it a bunch using NLC, but it is hard to test all scenarios and needs real world testing.

@@ -404,42 +408,40 @@ func (b *Buffer) SetRTT(rtt uint32) {
}

func (b *Buffer) calc(pkt []byte, arrivalTime time.Time) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this runs per packet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, per packet in the upstream direction.

Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! as far as I understand it :)

@boks1971 boks1971 merged commit 36dadba into master Aug 24, 2023
2 checks passed
@boks1971 boks1971 deleted the raja_squelch_padding branch August 24, 2023 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants