-
Notifications
You must be signed in to change notification settings - Fork 46
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
Optimize Read with buffering #15
Conversation
f19847f
to
54d1a41
Compare
stream_srtp.go
Outdated
|
||
// Create a buffer with a 1MB limit | ||
r.buffer = packetio.NewBuffer() | ||
r.buffer.SetLimitSize(1000 * 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make this a constant and pull it out of the function (just for visibility)
stream_srtcp.go
Outdated
|
||
// Create a buffer and limit it to 100KB | ||
r.buffer = packetio.NewBuffer() | ||
r.buffer.SetLimitSize(100 * 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
constant
This LGTM! Lets wait to merge until |
This avoids channel/lock coordination in favor of performing a copy and always adding the packet to a queue. Obviously I hate performing copies, but I think this is the fastest approach with the current API. This fixes much of the packet loss on the reader side.
This avoids channel/lock coordination in favor of performing a copy and
always adding the packet to a queue. Obviously I hate performing copies,
but I think this is the fastest approach with the current API.
Along with switching webrtc to using this package, this reduces packet loss
on the reader size from roughly 8% to 0.5%.
I wrote an implementation similar to the old code for comparison sake.