-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RSDK-9458] Remove 'Stream' from Camera interface #4691
base: main
Are you sure you want to change the base?
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]>
@@ -128,7 +111,8 @@ type Camera interface { | |||
// err = myCamera.Close(context.Background()) | |||
// | |||
// [camera component docs]: https://docs.viam.com/components/camera/ | |||
type VideoSource interface { | |||
type Camera interface { |
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.
There's a lot of files changed here. Am I correct that these changes to VideoSource/Camera/StreamCamera are the only real changes and everything else is the refactor to appease the compiler?
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.
Kind of - in the process of deprecating Stream, different methods are being used in vision, and the stream server now takes context to refresh the streams - I can break up this pr into independent pieces and remove some of sean yu's renames to make it smaller, I do think this was the diff that made the most sense for his manual testing.
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.
The camera driver changes + the new StreamCamera
interface (ffmpeg, transformpipeline, webcam) are there to appease the compiler without refactoring those drivers intensely.
@bhaney I like Rand's idea of testing with a vision config. Could/did you send that over? |
The merge conflicts on Sean's PR will likely be terrible when he gets back in 2.5 weeks, so I'm using this branch to keep it recent.
I also think we're at a point where we can merge. I'm adding the original testing Sean did to the end of this comment.
Requests: @bhaney, could I have a config to test one of the vision services with to ensure
DecodeImageFromCamera
does not break them?Original PR + comment
Re-testing after long time (my, Sean, vacation)