pion / mediadevices

Go implementation of the MediaDevices API.
https://pion.ly/
MIT License
521 stars 120 forks source link

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

Closed martha-johnston closed 1 year ago

martha-johnston commented 1 year ago

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

codecov[bot] commented 1 year ago

Codecov Report

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

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

:exclamation: 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 ```diff @@ 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](https://codecov.io/gh/pion/mediadevices/pull/476?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | | |---|---|---| | [pkg/driver/camera/camera\_linux.go](https://codecov.io/gh/pion/mediadevices/pull/476?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL2RyaXZlci9jYW1lcmEvY2FtZXJhX2xpbnV4Lmdv) | `28.93% <100.00%> (+0.36%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

edaniels commented 1 year ago

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

martha-johnston commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

HOORAY