pion / mediadevices

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

Expose device list update func #463

Closed bazile-clyde closed 1 year ago

bazile-clyde commented 1 year ago

Description

This PR exports the logic executed at startup to find and register cameras. This will allow us to discover cameras connected after our application has started.

at-wat commented 1 year ago

Do you think we can expose setup function in other drivers as well? It would be better to have unified interface to update device list.

codecov[bot] commented 1 year ago

Codecov Report

Base: 58.16% // Head: 58.21% // Increases project coverage by +0.04% :tada:

Coverage data is based on head (b4118d2) compared to base (6ac1424). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #463 +/- ## ========================================== + Coverage 58.16% 58.21% +0.04% ========================================== Files 62 62 Lines 3679 3683 +4 ========================================== + Hits 2140 2144 +4 Misses 1412 1412 Partials 127 127 ``` | [Impacted Files](https://codecov.io/gh/pion/mediadevices/pull/463?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/463?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL2RyaXZlci9jYW1lcmEvY2FtZXJhX2xpbnV4Lmdv) | `30.43% <100.00%> (+0.76%)` | :arrow_up: | | [pkg/driver/screen/x11\_linux.go](https://codecov.io/gh/pion/mediadevices/pull/463?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL2RyaXZlci9zY3JlZW4veDExX2xpbnV4Lmdv) | `12.69% <100.00%> (+2.86%)` | :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.

bazile-clyde commented 1 year ago

Do you think we can expose setup function in other drivers as well? It would be better to have unified interface to update device list.

@at-wat Done! I skipped the dummy.go files in audiotest and videotest. I also skipped vncdriver.go since there's no func init. Let me know if I should not have skipped any of those.

at-wat commented 1 year ago

@bazile-clyde thanks!

I think it would be better in future to have another API that mediadevices.MediaDevices can automatically call them to update the device list. For this, Initialize functions would be better to have godoc comments to mention this API is experimental. What do you think?

bazile-clyde commented 1 year ago

@bazile-clyde thanks!

I think it would be better in future to have another API that mediadevices.MediaDevices can automatically call them to update the device list. For this, Initialize functions would be better to have godoc comments to mention this API is experimental. What do you think?

That makes sense. I don't have strong opinions either way so I added it. Thanks for the quick turn-around btw!

bazile-clyde commented 1 year ago

Could you revert import block orders to keep them like

import (
  "stdlib1"
  "stdlib2"

  "nonstdlib1"
  "nonstdlib2"
)

Done!