-
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
SLAM: Process images when they're of YCbCr type #1429
Conversation
29c1e15
to
f35bbfa
Compare
f35bbfa
to
40a2841
Compare
if lazyImg.MIMEType() != utils.MimeTypePNG { | ||
return nil, nil, errors.Errorf("expected mime type %v, got %v", utils.MimeTypePNG, lazyImg.MIMEType()) | ||
|
||
if ycbcrImg, ok := img.(*image.YCbCr); ok { |
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.
I suggest adding an integration test that runs mono YCbCr images, but it's fine with me if you want to do that in a follow-up ticket.
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.
I'll create a follow-up ticket for this. Ticket: DATA-535
} | ||
return lazyImg.RawData(), release, nil | ||
|
||
return nil, nil, errors.Errorf("expected lazily encoded image or ycbcrImg, got %T", img) | ||
} |
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.
[nit] You could change the function name to getPNGImage()
, since it will not always be lazy.
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.
Added it to the follow-up ticket (create integration test): DATA-535
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.
LGTM, was curious about the images being BGR vs RGB with these changes+slam changes but we can look more into that later
Implements the RDK side of this ticket: https://viam.atlassian.net/browse/DATA-521
Tested with: