unblock video path after webcam unplug #15
Conversation
query.go
Outdated
| @@ -80,6 +80,18 @@ func GetNamedVideoSource( | |||
| return newVideoSourceFromDriver(d, selectedMedia, logger) | |||
| } | |||
|
|
|||
| func GetStatusVideoSource( | |||
There was a problem hiding this comment.
How do you intend on using this? I thought we wanted to just always requery?
There was a problem hiding this comment.
I'm using it in a background thread as a low-lift way to check when a USB camera is unplugged in this commit viamrobotics/rdk#1595. I just call this function every half second and if we get back an error we close the camera. Some of the other functions in this file repeatedly close and the open the underlying file which causes issues for other things (mainly the app) trying to stream video.
There was a problem hiding this comment.
Got it. It seems better to expose a way to access the driver that you have from the media source instead. The underlying unexported type has the driver so you can maybe make a method to get the driver/status from it instead of doing constraint evaluation again which can be a little expensive
There was a problem hiding this comment.
So we'd have to query for a new driver each time we check the status. This is because the status on any given driver is static. So if we saved the driver in a variable and kept checking its status we'd always get "running" back even after it's unplugged. To avoid running the SelectSettings algorithm over and over I added a method SelectDriverByDeviceId to filter based solely on the randomly generated UUID of the device driver grabbed from GetStatusVideoSource (renamed GetDriverVideoSource). We'd use the two methods like so
if attrs.Path != "" {
cam, err := tryWebcamOpen(ctx, attrs, attrs.Path, false, constraints, logger)
if err != nil {
return cam, err
}
goutils.PanicCapturingGo(func() {
ticker := time.NewTicker(500 * time.Millisecond)
defer ticker.Stop()
defer goutils.TryClose(ctx, cam)
name := resolveSourceName(attrs.Path, false)
d, err := gostream.GetDriverVideoSource(name, constraints, logger)
if err != nil {
return
}
uuid := d.ID()
for {
select {
case <-ticker.C:
if d := gostream.SelectDriverByDeviceId(uuid); d == nil {
return
}
case <-ctx.Done():
return
}
}
})
return cam, err
}
There was a problem hiding this comment.
I did not add any documentation yet just in case we need more changes to function signatures.
query.go
Outdated
| @@ -80,6 +80,18 @@ func GetNamedVideoSource( | |||
| return newVideoSourceFromDriver(d, selectedMedia, logger) | |||
| } | |||
|
|
|||
| func GetStatusVideoSource( | |||
There was a problem hiding this comment.
Got it. It seems better to expose a way to access the driver that you have from the media source instead. The underlying unexported type has the driver so you can maybe make a method to get the driver/status from it instead of doing constraint evaluation again which can be a little expensive
query.go
Outdated
| func SelectDriverByDeviceId(uuid string) driver.Driver { | ||
| filter := func(driver driver.Driver) bool { | ||
| for _, prop := range driver.Properties() { | ||
| if prop.DeviceID == uuid { |
There was a problem hiding this comment.
So two things: this UUID isn't very stable and can change if discovery changes, so I'm not sure this is the best way to get a device. The other (unless I'm missing something) is that the state of the driver has nothing to do with the OS state of the driver. How is the state being updated. It seems like overall what this code in conjunction with the RDK is doing is just checking if the file still exists (status is unused). In that case, is there any gostream change necessary?
media.go
Outdated
| return asMedia.driver.Properties() | ||
| } | ||
| } | ||
| return make([]prop.Media, 0) |
There was a problem hiding this comment.
You won't be able to take the len of nil though right? So the caller would have to check for nil and the len as opposed to just getting the length.
query.go
Outdated
| @@ -80,6 +80,15 @@ func GetNamedVideoSource( | |||
| return newVideoSourceFromDriver(d, selectedMedia, logger) | |||
| } | |||
|
|
|||
| func GetDriverVideoSource( | |||
bazile-clyde
force-pushed
the
DATA-764
branch
from
e9c354d
to
5213b23
Compare
bazile-clyde
force-pushed
the
DATA-764
branch
from
5213b23
to
b6b1d87
Compare
This commit is to help resolve DATA-764. The PR for that is viamrobotics/rdk#1595. We need to monitor the status of the video driver to ensure we free resources after a webcam is unplugged. See the Jira ticket for more info and steps to reproduce.