[RSDK-9864] Remove gostream middle-man dependency from webcam #4604
Conversation
…here we convert to go image; Change default mimetypes for classifier
…ng default mimetypes for vision since we are failing unit tests with 'do not know how to encode' errors
…urcewrappers Image func
Co-authored-by: nicksanford <[email protected]>
viambot
added
safe to test
Merge Rand's discovery removal PR into my refactor
viambot
added
safe to test
viambot
added
safe to test
hexbabe
changed the title
hexbabe
changed the title
hexbabe
changed the title
viambot
added
safe to test
viambot
added
safe to test
hexbabe
changed the title
|
#4720 (comment) more context surrounding this ticket |
|
Did it find the first driver it could connect to always? Try disabling and re-nable the webcam resource and a viam-server restart. I'm good to go otherwise. We'll handle any new bugs in this new world. |
Yeah it non-determinstically finds my iPhone or my builtin Mac webcam each disable+enable |
| return driver.Info{}, errors.Wrap(err, "cannot get driver from media source") | ||
| } | ||
| return d.Info(), nil | ||
| return nil, errors.New("NextPointCloud unimplemented") |
There was a problem hiding this comment.
Can we find a way to maintain PointCloud capabilities and the map of known intrinsics/extrinsics?
It's okay if it's dumb.
There was a problem hiding this comment.
Is it ok if we don't 😖 I really don't like noopCloser
There was a problem hiding this comment.
the map of known intrinsics/extrinsics
Does the new GetProperties handle this? Or do you mean something else
There was a problem hiding this comment.
I don't want a noop closer at all
here's what I'm thinking in pseudo code:
props, err := webcam.Properties(ctx, blah) , errcheck
switch
case props.Intrinsics != nil :
// call the rimage logic that makes pointclouds from images with a default Distortion Parameters, it can be found within the bowels on the videsource PointCloud method
case props.Intrinsics != nil && props.Distrotion != nil {
// call the rimage logic that makes pointclouds from images with the camera's know Distortion Parameters, it can be found within the bowels on the videsource PointCloud method
default:
error("NoPointCloud For YOU"
}
| } | ||
| } | ||
|
|
||
| for _, d := range needToClose { |
There was a problem hiding this comment.
Should we just close the driver after getting Properties in the main loop?
Does not seem like that big of a deal if we need to open everything then close out afterwards as is.
There was a problem hiding this comment.
This is a reasonable suggestion... but I'm a bit hesitant touching this code since it's stuff copy and pasted from gostream. Feels a bit out of scope and could cause confusion since there's no git history of it being here. WDYT @randhid
|
|
||
| // select implements SelectSettings algorithm. | ||
| // Reference: https://w3c.github.io/mediacapture-main/#dfn-selectsettings | ||
| func selectBestDriver( |
There was a problem hiding this comment.
Does the behavior of the driver matcher here change at all from previous code? Does it accept width, height, and framerate props that are not an exact fit?
There was a problem hiding this comment.
It should be the same since I copy pasted it from where it used to be referenced in gostream
| cam, err := tryWebcamOpen(ctx, conf, path, false, constraints, logger) | ||
| if err != nil { | ||
| return nil, "", errors.Wrap(err, "cannot open webcam") | ||
| if conf.Width != 0 && conf.Height != 0 { |
There was a problem hiding this comment.
Should we be checking conf width and height here? Could the conf specify 0s but the driver that is matched has a valid size?
There was a problem hiding this comment.
you're so right let me fix it to pull a frame regardless of config
viambot
added
safe to test
| return c.Images(ctx) | ||
| } | ||
| img, release, err := camera.ReadImage(ctx, c.underlyingSource) | ||
| img, release, err := c.reader.Read() |
RSDK-9864
Remove gostream middle-man dependency from webcam
Tested webcam on Mac and RPI4. Live and GetImage views on app. Unplug + replug on RPI4 (Logitech C90 Pro). Driver selection on app doesn't work anymore (as expected)