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

ReplayDetector is shared among multiple SSRCs #60

Closed
at-wat opened this issue Mar 24, 2020 · 5 comments · Fixed by #62
Closed

ReplayDetector is shared among multiple SSRCs #60

at-wat opened this issue Mar 24, 2020 · 5 comments · Fixed by #62

Comments

@at-wat
Copy link
Member

at-wat commented Mar 24, 2020

srtp.Context is shared among multiple SSRCs, but roll over counter and replay detector must not be shared.
Shared ROC causes decryption failure when the difference of actual ROCs is more than 1.
(Packet rate of each stream can be different.)
And, replay detector causes false positive detection.

ref: pion/webrtc#1095 (comment)

@Sean-Der
Copy link
Member

It has been a while since I have worked on this package, but rolloverCounter is a member of ssrcState. So maybe we just need to push the reploy detector into this as well?

@Sean-Der
Copy link
Member

@at-wat
Copy link
Member Author

at-wat commented Mar 25, 2020

That's true. Thanks!

@Sean-Der
Copy link
Member

One thing I do think we should do is get rid of all the stream stuff in this package, it was a bad design on my part :(

I would rather see demuxing in pion/rtp and a Session then we can do Congestion Control and JitterBuffer in there. I just have hit the second systems problem, just anxious to make mistakes when refactoring.

@at-wat at-wat changed the title srtp.Context is shared among multiple SSRCs ReplayDetector is shared among multiple SSRCs Mar 25, 2020
@at-wat
Copy link
Member Author

at-wat commented Mar 25, 2020

I would rather see demuxing in pion/rtp and a Session then we can do Congestion Control and JitterBuffer in there.

That sounds better.
I think all of congestion controller, JitterBuffer, and also SRTP decoder can be represented as a kind of interceptor interface that input and output RTP packet.
https://github.com/pion/webrtc/wiki/PlanningV3/_compare/ff1af1c4ae536549a0355c0eb64236ada391c234

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 a pull request may close this issue.

2 participants