RSDK-12758 — Support webcam unplug replug recovery on darwin #5618
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| //go:build darwin | ||
|
|
||
| package videosource | ||
|
|
||
| import ( | ||
| mediadevicescamera "github.com/pion/mediadevices/pkg/driver/camera" | ||
|
|
||
| "go.viam.com/rdk/logging" | ||
| ) | ||
|
|
||
| // startCameraObserver starts the Darwin camera device observer for hot-plug support. | ||
| // This should be called after SetupObserver has been called from the main thread. | ||
| // startCameraObserver is idempotent and can safely be called multiple times. | ||
| func startCameraObserver(logger logging.Logger) { | ||
| if err := mediadevicescamera.StartObserver(); err != nil { | ||
|
Member
There was a problem hiding this comment. does this need to be cleaned up ever?
Member
Author
There was a problem hiding this comment. it should be cleaned up by |
||
| logger.Errorw("failed to start darwin mediadevices camera observer; webcams will not handle hot unplug/replug", "error", err) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| //go:build !darwin | ||
|
|
||
| package videosource | ||
|
|
||
| import "go.viam.com/rdk/logging" | ||
|
|
||
| // startCameraObserver is a no-op on non-Darwin platforms. | ||
| // Camera hot-plug detection via AVFoundation observers is only supported on macOS. | ||
| func startCameraObserver(_ logging.Logger) {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| //go:build darwin | ||
|
|
||
| package main | ||
|
|
||
| import ( | ||
| mediadevicescamera "github.com/pion/mediadevices/pkg/driver/camera" | ||
|
|
||
| "go.viam.com/rdk/logging" | ||
| ) | ||
|
|
||
| // setupCameraObserver initializes the Darwin camera device observer for hot-plug support. | ||
| // | ||
| // On Darwin/macOS, SetupObserver must be called from the main.go because AVFoundation requires | ||
| // that camera device notification events and Key-Value Observation (KVO) updates occur on the main thread. | ||
| // The mediadevices library uses runtime.LockOSThread() to pin the background goroutine to whatever thread | ||
| // calls SetupObserver, so calling it in main() ensures that we run on the correct thread. | ||
| // | ||
| // This is why SetupObserver is called here rather than in the webcam component constructor: | ||
| // component constructors can be invoked from arbitrary goroutines, which would violate | ||
| // AVFoundation's threading requirements. StartObserver (called in webcam.go) can safely run from | ||
| // any thread after SetupObserver has been called. | ||
| // | ||
| // See: https://github.com/pion/mediadevices/pull/670 | ||
| func setupCameraObserver(logger logging.Logger) func() { | ||
| if err := mediadevicescamera.SetupObserver(); err != nil { | ||
| logger.Errorw("failed to set up darwin mediadevices camera observer; webcams will not handle hot unplug/replug", "error", err) | ||
| } | ||
| return func() { | ||
| if err := mediadevicescamera.DestroyObserver(); err != nil { | ||
| logger.Errorw("failed to destroy darwin mediadevices camera observer", "error", err) | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| //go:build !darwin | ||
abe-winter marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| package main | ||
|
|
||
| import "go.viam.com/rdk/logging" | ||
|
|
||
| // setupCameraObserver is a no-op on non-Darwin platforms. | ||
| // Camera hot-plug detection via AVFoundation observers is only supported on macOS. | ||
| // On Linux, poll-based device enumeration is used instead (checking /dev/video* files). | ||
| // On Windows, Media Foundation APIs handle device enumeration differently. | ||
| func setupCameraObserver(_ logging.Logger) func() { | ||
| return func() {} | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if you get the order wrong? crash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StartObserveractually callsSetupObserverunder the hood, but we callSetupObserverfirst in the entrypointmain.goto allow the objective-c NSRunLoop to set up on the main thread. It would be the "wrong" order if we only calledStartObserverin the webcam code and didn't callSetupObserverin main.go