pion / mediadevices

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

allow adapters to handle their own state #495

Closed bazile-clyde closed 1 year ago

bazile-clyde commented 1 year ago

Description

This PR allows for adapters (video and audio drivers) to handle their own state. This allows us to fix a few issues

Ideally, all drivers would return the actual state of the driver on queries. This PR returns real states for camera_linux.go fixing the above issues. All other drivers keep their current behavior.

Testing

This was manually tested on a Linux device and works as expected. It should not break any tests or change the current behavior for any drivers other than those on Linux.

Advice for Reviewers

The easiest way to review this code is to start with the changes in driver.go and wrapper.go. Afterwards, ensure none of the logic in any of the files besides camera_linux.go has changed. Lastly, review/test the changes in camera_linux.go and the new states in state.go. Simply running a background goroutine that continuously calls and prints the Status() of a Linux camera and observing what happens when the camera is disconnected and reconnected should be enough.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 36.36% and project coverage change: -0.29 :warning:

Comparison is base (f0f6be7) 59.11% compared to head (d82c93b) 58.83%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #495 +/- ## ========================================== - Coverage 59.11% 58.83% -0.29% ========================================== Files 62 63 +1 Lines 3784 3826 +42 ========================================== + Hits 2237 2251 +14 - Misses 1416 1444 +28 Partials 131 131 ``` | [Impacted Files](https://codecov.io/gh/pion/mediadevices/pull/495?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/495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL2RyaXZlci9jYW1lcmEvY2FtZXJhX2xpbnV4Lmdv) | `27.54% <0.00%> (-3.56%)` | :arrow_down: | | [pkg/driver/driver.go](https://codecov.io/gh/pion/mediadevices/pull/495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL2RyaXZlci9kcml2ZXIuZ28=) | `80.00% <80.00%> (ø)` | | | [pkg/driver/wrapper.go](https://codecov.io/gh/pion/mediadevices/pull/495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL2RyaXZlci93cmFwcGVyLmdv) | `88.31% <100.00%> (+1.74%)` | :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 in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

edaniels commented 1 year ago

@at-wat could you take a second set of eyes on this?

bazile-clyde commented 1 year ago

@edaniels @at-wat Thanks for the reviews. After seeing all of the comments and speaking with @edaniels I updated this PR to make this functionality optional. It's far less changes now and will not break the API.