RSDK-7469: Add CaptureAllFromCamera() #3906
Conversation
viambot
added
the
safe to test
|
Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!
|
viambot
added
safe to test
rimage/image_file.go
Outdated
| return nil, err | ||
| if mimeType == "" { | ||
| img, _, err = image.Decode(bytes.NewReader(imgBytes)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } else { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
I know we wrote this together, but actually I think what you can do is just put this in its own case statement, so that you don't have to check again if the mimeType is ""
so it would be
case "":
img, err := DecodeJPEG(bytes.NewReader(imgBytes))
if err != nil {
img, _, err = image.Decode(bytes.NewReader(imgBytes))
if err != nil {
return nil, err
}
}
return img, nil
services/vision/client.go
Outdated
| returnImage bool, | ||
| returnDetections bool, | ||
| returnClassifications bool, | ||
| returnObject bool, |
There was a problem hiding this comment.
I would prefer that you create a struct that contains all of these bools together, rather than having to feed them in individually. It would also help with backwards compatability in case anything is ever added to the list
services/vision/vision.go
Outdated
| returnImage bool, | ||
| returnClass bool, | ||
| returnDet bool, | ||
| returnObjPCD bool, |
There was a problem hiding this comment.
gather all of these bools into a struct, and pass the struct in as the argument. You can define this input within the viscapture package too
services/vision/vision.go
Outdated
| returnImage bool, | ||
| returnClass bool, | ||
| returnDet bool, | ||
| returnObjPCD bool, |
There was a problem hiding this comment.
use a new struct for these bool inputs
vision/viscapture/viscapture.go
Outdated
| type VisCapture interface { | ||
| Image() image.Image | ||
| Detections() []objectdetection.Detection | ||
| Classifications() classification.Classifications | ||
| PointCloudObject() []*vision.Object | ||
| } | ||
|
|
||
| // NewVisCapture returns a VisCapture. | ||
| func NewVisCapture(img image.Image, | ||
| dets []objectdetection.Detection, | ||
| class classification.Classifications, | ||
| obj []*vision.Object, | ||
| ) VisCapture { | ||
| return &capture{ | ||
| image: img, | ||
| detections: dets, | ||
| classifications: class, | ||
| pcd: obj, | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm curious why you chose to make an interface for VisCapture, rather than just making the struct Public. Was it in order so that the output could not be modified by a user? In that case, what you can do is make the struct public, but the member variables private, and then have the methods on it to retrieve the values from the member variables (as you have already done)
services/vision/server.go
Outdated
| func imageToProto(ctx context.Context, img image.Image, mimeType string) (*v11.Image, error) { | ||
| imgBytes, err := rimage.EncodeImage(ctx, img, mimeType) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &v11.Image{Image: imgBytes}, nil | ||
| } |
There was a problem hiding this comment.
Add checks for if a "" mimeType is passed in.
- Check if its DepthMap or Gray16 (then encode as a ViamRawDepth Image)
- Check if it is a LazyEncodedImage (then use the mimeType from the lazy encoded Image)
- If it's none of those, you can default to a JPEG encoding
services/vision/server.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &v11.Image{Image: imgBytes}, nil |
There was a problem hiding this comment.
Also, Image has more fields than "Image", you should also store the"Format" that you ended up encoding it as. You also have the original camera name -- put that in the "SourceName" field
services/vision/client.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| img, err := rimage.DecodeImage(ctx, resp.Image.Image, "") |
There was a problem hiding this comment.
The response Image gives you the "Format" as well. So you don't actually have to leave this blank -- you can know what the image is encoded as and decode it appropriately
services/vision/client.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| capt := viscapture.NewVisCapture(img, dets, protoToClas(resp.Classifications), objPCD) |
There was a problem hiding this comment.
While protoToClas doesn't return an error, it was confusing at first because it seemed like you didn't return classifications. Go ahead and put this conversion on a line after protosToDets rather than doing the conversion in the New function.
viambot
added
safe to test
viambot
added
safe to test
viambot
removed
the
safe to test
viambot
added
the
safe to test
viambot
added
safe to test
| defer release() | ||
| var detections []objectdetection.Detection | ||
| if opt.ReturnDetections { | ||
| if !vm.properties.DetectionSupported { |
There was a problem hiding this comment.
@bhaney I changed those tests to use the new vm.properites. Is that ok?
| sort.Slice(cc, func(i, j int) bool { return cc[i].Score() > cc[j].Score() }) | ||
| return cc[0:n], nil |
There was a problem hiding this comment.
the change to return all classifications when n=0
| return &camerapb.Image{ | ||
| Image: imgBytes, | ||
| Format: format, | ||
| SourceName: cameraName, |
There was a problem hiding this comment.
it seems that SourceName gets lost when the proto is decoded in the image.Image....
There was a problem hiding this comment.
luckily, SourceName is only really important for GetImages -- so I think this is fine
viambot
added
safe to test
| }, nil | ||
| } | ||
|
|
||
| func encodeUnknownType(ctx context.Context, img image.Image, defaultMime string) ([]byte, string, error) { |
that way it seems that it just works for all built-in vision service in RDK.
EDIT:
Open question on the MimeType for the "ok nowresp.Image.Image"