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

Add manager clearing mechanism for linux cams #532

Merged
merged 2 commits into from
Oct 17, 2023

Conversation

seanavery
Copy link
Contributor

@seanavery seanavery commented Oct 16, 2023

Description

Currently, there is a bug in the Initialize() fn in camera_linux.go where the full driver list is appended causing the manager to fill up with duplicates. This is due to generating a new uuid on the fly for each driver record.

!!! The problem only occurs if calling Initialize after initial init run.

This issue does not occur id camera_darwin.go because the id is taken from avfoundation and is deterministic/unique for each driver.

Solution

Clear all video drivers during the Initialize call to prevent duplicates.

@seanavery
Copy link
Contributor Author

seanavery commented Oct 16, 2023

Looks like I am having CI issues with pion/ice dependency. Not sure if this is related to the recent golang and dependency updates over the last couple of weeks.

Run make test
go: github.com/pion/ice/v2@v2.3.10: missing go.sum entry for go.mod file; to add it:
	go mod download github.com/pion/ice/v2
go: github.com/pion/ice/v2@v2.3.10: missing go.sum entry for go.mod file; to add it:
	go mod download github.com/pion/ice/v2
go vet 
go: github.com/pion/ice/v2@v2.3.10: missing go.sum entry for go.mod file; to add it:
	go mod download github.com/pion/ice/v2
make: *** [Makefile:7[6](https://github.com/pion/mediadevices/actions/runs/6537347218/job/17750984775?pr=532#step:5:7): test] Error 1
Error: Process completed with exit code 2.

It looks like CI failed for the last commit Bump minimum supported Go version to 1.19 .

@bazile-clyde
Copy link
Collaborator

Looks like I am having CI issues with pion/ice dependency. Not sure if this is related to the recent golang and dependency updates over the last couple of weeks.

Run make test
go: github.com/pion/ice/v2@v2.3.10: missing go.sum entry for go.mod file; to add it:
	go mod download github.com/pion/ice/v2
go: github.com/pion/ice/v2@v2.3.10: missing go.sum entry for go.mod file; to add it:
	go mod download github.com/pion/ice/v2
go vet 
go: github.com/pion/ice/v2@v2.3.10: missing go.sum entry for go.mod file; to add it:
	go mod download github.com/pion/ice/v2
make: *** [Makefile:7[6](https://github.com/pion/mediadevices/actions/runs/6537347218/job/17750984775?pr=532#step:5:7): test] Error 1
Error: Process completed with exit code 2.

It looks like CI failed for the last commit Bump minimum supported Go version to 1.19 .

Ah, I see. I added a fix for this issue #533. You can rebase this on top of that commit after it's pushed. Looks good so far.

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Files Coverage Δ
pkg/driver/camera/camera_linux.go 25.00% <50.00%> (+0.52%) ⬆️
pkg/driver/manager.go 56.00% <0.00%> (-4.87%) ⬇️

📢 Thoughts on this report? Let us know!.

@bazile-clyde
Copy link
Collaborator

@seanavery the master branch is passing, so feel free to rebase and force push.

@bazile-clyde bazile-clyde self-requested a review October 17, 2023 20:40
Copy link
Collaborator

@bazile-clyde bazile-clyde left a comment

Choose a reason for hiding this comment

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

LGTM!

@bazile-clyde bazile-clyde merged commit 3c9fee9 into pion:master Oct 17, 2023
8 checks passed
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 this pull request may close these issues.

3 participants