[RSDK-9458] Remove Stream from Go camera interface
#4580
Conversation
This file contains 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
…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
the
safe to test
viambot
added
safe to test
hexbabe
commented
Dec 16, 2024
| if err != nil { | ||
| return nil, ImageMetadata{}, err | ||
| } | ||
| defer release() | ||
| if mimeType == "" { | ||
| mimeType = utils.MimeTypePNG // default to lossless mimetype such as PNG |
There was a problem hiding this comment.
Does it make more sense to default to jpeg?
viambot
added
safe to test
hexbabe
changed the title
Stream from Go camera interface
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
hexbabe
commented
Dec 18, 2024
|
|
||
| // Close shuts down the resource and prevents further use. | ||
| Close(ctx context.Context) error | ||
| // StreamCamera is a camera that has `Stream` embedded to directly integrate with gostream. |
There was a problem hiding this comment.
I want to delete this asap, but it's necessary for preserving some of the builtins e.g. transform
Could make follow-up tickets to refactor all the builtins to use Image and yeet transform pipeline and modularize all the nodes.
hexbabe
commented
Dec 18, 2024
| // streamCameraFromCamera is a hack to allow us to use Stream to pipe frames through the pipeline | ||
| // and still implement a camera resource. | ||
| // We prefer this methodology over passing Image bytes because each transform desires a image.Image over | ||
| // a raw byte slice. To use Image would be to wastefully encode and decode the frame multiple times. |
There was a problem hiding this comment.
It would also make this PR so hard because every transform would be need to be re-written with Image
hexbabe
commented
Dec 18, 2024
| "go.viam.com/rdk/logging" | ||
| "go.viam.com/rdk/pointcloud" | ||
| "go.viam.com/rdk/resource" | ||
| "go.viam.com/rdk/rimage" | ||
| "go.viam.com/rdk/rimage/transform" | ||
| "go.viam.com/rdk/robot" | ||
| camerautils "go.viam.com/rdk/robot/web/stream/camera" |
There was a problem hiding this comment.
This is terrible and why I think we should clean up transform after this
But in its defense... we are already muxing the stream and camera stuff. This import is just more honest about it.
viambot
added
safe to test
|
We can wait til Rand (OOO) takes a look before merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Tests