Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

change buffer size to 2 #482

Merged
merged 2 commits into from
Mar 29, 2023
Merged

change buffer size to 2 #482

merged 2 commits into from
Mar 29, 2023

Conversation

bazile-clyde
Copy link
Collaborator

@bazile-clyde bazile-clyde commented Mar 17, 2023

Description

From the Linux kernel documentation here

As a practical rule, a minimum of two buffers are needed for proper streaming, and there is usually a maximum (which cannot exceed 32) which makes sense for each device.

This code updates the buffer size from 1 to 2 along with the frame skip logic. The side effect is much higher fps on all 3 of the cameras I've tested, presumably because the buffer is no longer a bottleneck, and there shouldn't be a risk of slowing down any streams.

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Patch coverage: 57.14% and project coverage change: +0.49 🎉

Comparison is base (11bf55f) 58.33% compared to head (2882872) 58.83%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #482      +/-   ##
==========================================
+ Coverage   58.33%   58.83%   +0.49%     
==========================================
  Files          62       62              
  Lines        3737     3741       +4     
==========================================
+ Hits         2180     2201      +21     
+ Misses       1430     1413      -17     
  Partials      127      127              
Impacted Files Coverage Δ
pkg/driver/camera/camera_linux.go 28.93% <0.00%> (ø)
pkg/driver/manager.go 60.86% <100.00%> (+34.67%) ⬆️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -72,6 +72,7 @@ type camera struct {
mutex sync.Mutex
cancel func()
prevFrameTime time.Time
bufCount int
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason it's not

const bufCount = 2

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, that's better. Done!

@bazile-clyde bazile-clyde requested a review from at-wat March 27, 2023 15:13
@bazile-clyde bazile-clyde requested a review from at-wat March 28, 2023 17:07
Copy link
Member

@at-wat at-wat left a comment

Choose a reason for hiding this comment

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

LGTM:+1:

@edaniels edaniels merged commit b9ce5bb into pion:master Mar 29, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants