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

When finding cameras on linux systems, look at camera ID before path or video* #476

Merged
merged 8 commits into from
Mar 7, 2023

Conversation

martha-johnston
Copy link
Contributor

@martha-johnston martha-johnston commented Feb 27, 2023

Problem Description

Currently, it is not possible to detect a camera that has moved from one USB port to another on a linux system. This is the case because the camera_linux.go file implementation first looks at the paths in /dev/v4l/by-path/* and then in /dev/video* when looking for a camera object. The problem is that when USB cameras are moved around between USB ports on the linux system, the by-path and video* addresses may change. If there is a reference to the camera path, and then the camera is physically moved to a different USB port, the old path no longer exists and the camera cannot be opened.

For example, observe the naming schema in /dev/v4l/by-path/* for a camera mapped to /dev/video0

lrwxrwxrwx 1 root root 12 Feb 27 11:45 platform-fd500000.pcie-pci-0000:01:00.0-usb-0:1.3:1.0-video-index0 -> ../../video0
lrwxrwxrwx 1 root root 12 Feb 27 11:45 platform-fd500000.pcie-pci-0000:01:00.0-usb-0:1.3:1.0-video-index1 -> ../../video1 

The USB port is part of the name. For comparison, here is the same camera moved to a different USB port

lrwxrwxrwx 1 root root 12 Feb 27 11:45 platform-fd500000.pcie-pci-0000:01:00.0-usb-0:1.1:1.0-video-index0 -> ../../video0
lrwxrwxrwx 1 root root 12 Feb 27 11:45 platform-fd500000.pcie-pci-0000:01:00.0-usb-0:1.1:1.0-video-index1 -> ../../video1 

Note that the path has changed. When the camera is first connected to video0, the associated path is saved, and when the camera is unplugged and plugged back in to a different port, it may still be identified as video0, however the associated path is no longer the same. If the path is identical, i.e., it has been plugged back into the same USB port, then there’s no issue. However, if it moves USB ports, the old path no longer exists so it cannot be opened.

Solution

In order to allow cameras to be switched between USB ports and still be accessible, the cameras must be identified by their ID, as that will never change regardless of where the camera is plugged in. This is a very simple change of looking in dev/v4l/by-id/* before by-path or video*.

Here is an example of a camera ID found in the newly created dev/v4l/by-id folder.

lrwxrwxrwx 1 root root 12 Feb 27 10:43 usb-EMEET_HD_Webcam_eMeet_C950_A220914000210705-video-index0 -> ../../video0
lrwxrwxrwx 1 root root 12 Feb 27 10:43 usb-EMEET_HD_Webcam_eMeet_C950_A220914000210705-video-index1 -> ../../video1

And after the camera is moved to a different USB port, the path remains exactly the same.

lrwxrwxrwx 1 root root 12 Feb 27 11:44 usb-EMEET_HD_Webcam_eMeet_C950_A220914000210705-video-index0 -> ../../video0
lrwxrwxrwx 1 root root 12 Feb 27 11:44 usb-EMEET_HD_Webcam_eMeet_C950_A220914000210705-video-index1 -> ../../video1

This change will allow for more flexibility for users who want to adjust their physical set up, as well as hopefully decrease troubleshooting and reinitializing when users do change their physical set ups and run in to issues.

Testing

For manually testing, I attached two webcams to a Raspberry Pi and switched them around to different USB ports to observe their behavior. I tried every combination of the two webcams, and with this new implementation, the cameras will consistently reconnect, and the video stream will resume normally. Previously, the cameras would only reconnect and resume the video stream if they were plugged in to the same USB port. Additionally, I added a new test to the camera_linux_test.go file that tests the camera discoveries by-id in addition to the currently existing test that tests by-path

@martha-johnston martha-johnston marked this pull request as ready for review February 27, 2023 17:30
@@ -79,6 +79,7 @@ func init() {
// Initialize finds and registers camera devices. This is part of an experimental API.
func Initialize() {
discovered := make(map[string]struct{})
discover(discovered, "/dev/v4l/by-id/*")
Copy link
Member

Choose a reason for hiding this comment

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

Can you write a test for this and also describe how you've manually tested this?

@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (62009a8) 58.32% compared to head (fd3ca4b) 58.33%.

❗ Current head fd3ca4b differs from pull request most recent head 607ca82. Consider uploading reports for the commit 607ca82 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
+ Coverage   58.32%   58.33%   +0.01%     
==========================================
  Files          62       62              
  Lines        3736     3737       +1     
==========================================
+ Hits         2179     2180       +1     
  Misses       1430     1430              
  Partials      127      127              
Impacted Files Coverage Δ
pkg/driver/camera/camera_linux.go 28.93% <100.00%> (+0.36%) ⬆️

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@edaniels
Copy link
Member

edaniels commented Mar 6, 2023

@martha-johnston it looks like CI is failing. Can you take a look?

@martha-johnston
Copy link
Contributor Author

@martha-johnston it looks like CI is failing. Can you take a look?

It's passing locally so not 100% sure but I think this change should fix it

@edaniels
Copy link
Member

edaniels commented Mar 6, 2023

That's still failing unfortunately. You may want to try using https://github.com/nektos/act to run that linux action to see if you can reproduce it.

@martha-johnston
Copy link
Contributor Author

That's still failing unfortunately. You may want to try using https://github.com/nektos/act to run that linux action to see if you can reproduce it.

Should be solved now. Was able to reproduce and fix locally

@edaniels
Copy link
Member

edaniels commented Mar 7, 2023

HOORAY

@edaniels edaniels merged commit 55881dd into pion:master Mar 7, 2023
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