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

Increment rtx rtp packet sequence number only when trasmitted #285

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

oto313
Copy link

@oto313 oto313 commented Oct 29, 2024

Fix for wrong sequence numbers

Currently the nack responder (responder_interceptor.go) generates RTX RTP packet which has not right sequence numbers. It does not increment the sequence number by one according to RFC rfc4588

For either multiplexing scheme, the sequence number has the standard definition; i.e., it MUST be one higher than the sequence number of the preceding packet sent in the retransmission stream.

I am proposing fix for it and appreciate any feedback as this is my first pull request in GO lang.

@Sean-Der
Copy link
Member

Nice find @oto313! How did you find this, I am really impressed! If you run go test -v locally you have some build/test failures.

  • Maybe we make this part of Get? Auto-increments (if needed)
  • We should write some tests that it does the auto increment as expected

@oto313
Copy link
Author

oto313 commented Oct 30, 2024

Hi @Sean-Der. Thanks. I was investigating why retransmission not worked in my use case and I was investigating it with wireshark and I compared retransmission packets with google webrtc implementation.

If you run go test -v locally you have some build/test failures.

Sorry the build/tests are fixed

Maybe we make this part of Get? Auto-increments (if needed)

part of which Get?

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.

2 participants