Skip to content

Worker: RtpStreamSend, do not send duplicated RTP packets #1683

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.

Sign up for GitHub

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

Jump to bottom
Merged
jmillan merged 5 commits into from
Jan 7, 2026
Merged

Worker: RtpStreamSend, do not send duplicated RTP packets #1683

jmillan merged 5 commits into from
Jan 7, 2026

Conversation

@jmillan
Copy link
Member

@jmillan jmillan commented Jan 5, 2026 •
edited
Loading

Fixes #1675

If the packet is already present in the retransmission buffer then discard it.

jmillan added 2 commits January 5, 2026 12:04
@jmillan
Worker: RtpStreamSend, do not send duplicated RTP packets
dc037ec 
Fixes #1675

If the packet is already present in the retransmission buffer then discard it.

Also fix a bug where we were sending packets higher than the MTU size.
@jmillan
CHANGELOG
a5c0259
ibc
ibc reviewed Jan 5, 2026
View reviewed changes
ibc
ibc reviewed Jan 5, 2026
View reviewed changes
ibc
ibc reviewed Jan 7, 2026
View reviewed changes
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.

Without looking at the code, should we also do this check in RtxStream::ReceivePacket()? In other words: are we checking if packets received via RTX are duplicated?

@jmillan
Copy link
Member Author

jmillan commented Jan 7, 2026

Without looking at the code, should we also do this check in RtxStream::ReceivePacket()? In other words: are we checking if packets received via RTX are duplicated?

It does not apply. When receiving a RTX packet and calling RtxStream::ReceivePacket() this method does decode the packet into the original one. Original packet duplication is fixed in this PR.

@ibc
Copy link
Member

ibc commented Jan 7, 2026

It does not apply. When receiving a RTX packet and calling RtxStream::ReceivePacket() this method does decode the packet into the original one. Original packet duplication is fixed in this PR.

What do you mean? Producer::ReceiveRtpPacket() may call to rtpStream->ReceiveRtxPacket() if this is a RTX packet and there we are not checking if the decoded RTP packet is duplicated.

@jmillan
Copy link
Member Author

jmillan commented Jan 7, 2026 •
edited
Loading

What do you mean? Producer::ReceiveRtpPacket() may call to rtpStream->ReceiveRtxPacket() if this is a RTX packet and there we are not checking if the decoded RTP packet is duplicated.

The duplicated packet will reach RtpStreamSend::ReceivePacket() and we'll discard it for being already buffered for retransmissions. This is what this PR does.

@jmillan jmillan mentioned this pull request Jan 7, 2026
Closed
@ibc
Copy link
Member

ibc commented Jan 7, 2026

The duplicated packet will reach RtpStreamSend::ReceivePacket()

True, now I got it

@jmillan jmillan merged commit d986d5d into v3 Jan 7, 2026
48 checks passed
@jmillan jmillan deleted the issue_1675 branch January 7, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ibc ibc ibc approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Side-effect of sending duplicate packets to clients

3 participants

@jmillan @ibc