Skip to content

Consumer: Fix sequence number gap#1494

Merged
jmillan merged 10 commits intov3from
fix-sequence-number-gap
Feb 24, 2025
Merged

Consumer: Fix sequence number gap#1494
jmillan merged 10 commits intov3from
fix-sequence-number-gap

Conversation

@jmillan
Copy link
Member

@jmillan jmillan commented Feb 24, 2025

Drop the given sequence number if the RTP packet cannot be forwarded.

Fixes #1069

By not doing so, we are incorrectly generating non legitimate sequence number jumps which involves:

  • The other end requesting NACKs for packets that do not exist, and us processing them.
  • The other end waiting for those packets that will never arrive.
  • Possible implications an jitter buffer and decoding level.

This problem is very apparent when using BWE and the available BW is low enough to discard a lot of packets, or when the outgoing bitrate is manually set low enough too.

Copy link
Member

@ibc ibc left a comment

Choose a reason for hiding this comment

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

I'm amazed that this fix looks too obvious to never have realized of it. I mean, we were literally dropping packets in MANY SCENARIOS without dropping then in SeqManager. How is possible that we never noticed it?

Please add entry in CHANGELOG.

Also minimal cosmetic changes.

And ASAN checks are failing in CI.

@ibc
Copy link
Member

ibc commented Feb 24, 2025

Fixes #1069

Fixes #1069 or fixes #1070 or both?

@ibc
Copy link
Member

ibc commented Feb 24, 2025

I'm updating the branch with pending changes in v3.

@ibc
Copy link
Member

ibc commented Feb 24, 2025

And ASAN checks are failing in CI.

All those are caused by the new TestSimpleConsumer.cpp file.

@jmillan jmillan merged commit 13a7804 into v3 Feb 24, 2025
55 checks passed
@jmillan jmillan deleted the fix-sequence-number-gap branch February 24, 2025 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Large packet loss when estimated bandwidth is close to the lowest SVC layer

2 participants