From be4f36996a4690de3c5f89d9289da65cdc353265 Mon Sep 17 00:00:00 2001 From: Nick Sanford Date: Fri, 3 Jan 2025 14:00:06 -0500 Subject: [PATCH 1/5] fix-camera-image-returning-no-mime-type --- components/camera/camera.go | 3 ++- components/camera/client.go | 9 ++++++--- rimage/lazy_encoded.go | 7 +++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/components/camera/camera.go b/components/camera/camera.go index 71f9b4e25fc..e6288ba9850 100644 --- a/components/camera/camera.go +++ b/components/camera/camera.go @@ -19,6 +19,7 @@ import ( "go.viam.com/rdk/rimage" "go.viam.com/rdk/rimage/transform" "go.viam.com/rdk/robot" + "go.viam.com/rdk/utils" ) func init() { @@ -166,7 +167,7 @@ func DecodeImageFromCamera(ctx context.Context, mimeType string, extra map[strin if len(resBytes) == 0 { return nil, errors.New("received empty bytes from camera") } - img, err := rimage.DecodeImage(ctx, resBytes, resMetadata.MimeType) + img, err := rimage.DecodeImage(ctx, resBytes, utils.WithLazyMIMEType(resMetadata.MimeType)) if err != nil { return nil, fmt.Errorf("could not decode into image.Image: %w", err) } diff --git a/components/camera/client.go b/components/camera/client.go index 6dd8ed72b6c..682e82dd732 100644 --- a/components/camera/client.go +++ b/components/camera/client.go @@ -196,11 +196,14 @@ func (c *client) Image(ctx context.Context, mimeType string, extra map[string]in if expectedType != "" && resp.MimeType != expectedType { c.logger.CDebugw(ctx, "got different MIME type than what was asked for", "sent", expectedType, "received", resp.MimeType) - } else { - resp.MimeType = mimeType + if resp.MimeType == "" { + // if the user expected a mime_type and the successful response didn't have a mime type, assume the + // response's mime_type was what the user requested + resp.MimeType = mimeType + } } - return resp.Image, ImageMetadata{MimeType: utils.WithLazyMIMEType(resp.MimeType)}, nil + return resp.Image, ImageMetadata{MimeType: resp.MimeType}, nil } func (c *client) Images(ctx context.Context) ([]NamedImage, resource.ResponseMetadata, error) { diff --git a/rimage/lazy_encoded.go b/rimage/lazy_encoded.go index 8676ca55a08..23c956efa7b 100644 --- a/rimage/lazy_encoded.go +++ b/rimage/lazy_encoded.go @@ -5,7 +5,10 @@ import ( "context" "image" "image/color" + "net/http" "sync" + + "go.viam.com/rdk/logging" ) // LazyEncodedImage defers the decoding of an image until necessary. @@ -29,6 +32,10 @@ type LazyEncodedImage struct { // away with reading all metadata from the header of the image bytes. // NOTE: Usage of an image that would fail to decode causes a lazy panic. func NewLazyEncodedImage(imgBytes []byte, mimeType string) image.Image { + if mimeType == "" { + logging.Global().Warn("NewLazyEncodedImage called without a mime_type. Sniffing bytes to detect mime_type. Specify mime_type to reduce CPU utilization") + mimeType = http.DetectContentType(imgBytes) + } return &LazyEncodedImage{ imgBytes: imgBytes, mimeType: mimeType, From e850ece29ff41d57be07681bbdc30ac08619a5d8 Mon Sep 17 00:00:00 2001 From: Nick Sanford Date: Fri, 3 Jan 2025 14:33:51 -0500 Subject: [PATCH 2/5] lint --- rimage/lazy_encoded.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rimage/lazy_encoded.go b/rimage/lazy_encoded.go index 23c956efa7b..39252632326 100644 --- a/rimage/lazy_encoded.go +++ b/rimage/lazy_encoded.go @@ -33,7 +33,8 @@ type LazyEncodedImage struct { // NOTE: Usage of an image that would fail to decode causes a lazy panic. func NewLazyEncodedImage(imgBytes []byte, mimeType string) image.Image { if mimeType == "" { - logging.Global().Warn("NewLazyEncodedImage called without a mime_type. Sniffing bytes to detect mime_type. Specify mime_type to reduce CPU utilization") + logging.Global().Warn("NewLazyEncodedImage called without a mime_type. " + + "Sniffing bytes to detect mime_type. Specify mime_type to reduce CPU utilization") mimeType = http.DetectContentType(imgBytes) } return &LazyEncodedImage{ From b2a7095a2c59fbb7f2a37d724cad2f680f62c725 Mon Sep 17 00:00:00 2001 From: Nick Sanford Date: Fri, 3 Jan 2025 14:49:20 -0500 Subject: [PATCH 3/5] wip --- rimage/lazy_encoded_test.go | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/rimage/lazy_encoded_test.go b/rimage/lazy_encoded_test.go index 0727e0e2a00..2e28cafa41f 100644 --- a/rimage/lazy_encoded_test.go +++ b/rimage/lazy_encoded_test.go @@ -3,6 +3,7 @@ package rimage import ( "bytes" "image" + "image/jpeg" "image/png" "testing" @@ -15,10 +16,14 @@ func TestLazyEncodedImage(t *testing.T) { img := image.NewNRGBA(image.Rect(0, 0, 4, 8)) img.Set(3, 3, Red) - var buf bytes.Buffer - test.That(t, png.Encode(&buf, img), test.ShouldBeNil) + var pngBuf bytes.Buffer + test.That(t, png.Encode(&pngBuf, img), test.ShouldBeNil) + var jpegBuf bytes.Buffer + test.That(t, jpeg.Encode(&jpegBuf, img, &jpeg.Options{Quality: 100}), test.ShouldBeNil) + jpegImg, err := jpeg.Decode(bytes.NewBuffer(jpegBuf.Bytes())) + test.That(t, err, test.ShouldBeNil) - imgLazy := NewLazyEncodedImage(buf.Bytes(), utils.MimeTypePNG) + imgLazy := NewLazyEncodedImage(pngBuf.Bytes(), utils.MimeTypePNG) test.That(t, imgLazy.(*LazyEncodedImage).MIMEType(), test.ShouldEqual, utils.MimeTypePNG) test.That(t, NewColorFromColor(imgLazy.At(0, 0)), test.ShouldEqual, Black) @@ -46,4 +51,26 @@ func TestLazyEncodedImage(t *testing.T) { test.That(t, func() { imgLazy.ColorModel() }, test.ShouldPanic) test.That(t, func() { NewColorFromColor(imgLazy.At(0, 0)) }, test.ShouldPanic) test.That(t, func() { NewColorFromColor(imgLazy.At(4, 4)) }, test.ShouldPanic) + + // png without a mime type + imgLazy = NewLazyEncodedImage(pngBuf.Bytes(), "") + test.That(t, imgLazy.(*LazyEncodedImage).MIMEType(), test.ShouldEqual, utils.MimeTypePNG) + test.That(t, NewColorFromColor(imgLazy.At(0, 0)), test.ShouldEqual, Black) + test.That(t, NewColorFromColor(imgLazy.At(3, 3)), test.ShouldEqual, Red) + test.That(t, imgLazy.Bounds(), test.ShouldResemble, img.Bounds()) + test.That(t, imgLazy.ColorModel(), test.ShouldResemble, img.ColorModel()) + + img2, err = png.Decode(bytes.NewBuffer(imgLazy.(*LazyEncodedImage).RawData())) + test.That(t, err, test.ShouldBeNil) + test.That(t, img2, test.ShouldResemble, img) + + // jpeg without a mime type + imgLazy = NewLazyEncodedImage(jpegBuf.Bytes(), "") + test.That(t, imgLazy.(*LazyEncodedImage).MIMEType(), test.ShouldEqual, utils.MimeTypeJPEG) + test.That(t, imgLazy.Bounds(), test.ShouldResemble, jpegImg.Bounds()) + test.That(t, imgLazy.ColorModel(), test.ShouldResemble, jpegImg.ColorModel()) + + img2, err = jpeg.Decode(bytes.NewBuffer(imgLazy.(*LazyEncodedImage).RawData())) + test.That(t, err, test.ShouldBeNil) + test.That(t, img2, test.ShouldResemble, jpegImg) } From ed375381e1d6d6a4c378f944cc607603753be9dc Mon Sep 17 00:00:00 2001 From: Nick Sanford Date: Fri, 3 Jan 2025 16:16:10 -0500 Subject: [PATCH 4/5] wip --- components/camera/client_test.go | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/components/camera/client_test.go b/components/camera/client_test.go index e1a827d5339..291c8d2d8e1 100644 --- a/components/camera/client_test.go +++ b/components/camera/client_test.go @@ -5,6 +5,7 @@ import ( "context" "image" "image/color" + "image/jpeg" "image/png" "net" "testing" @@ -428,7 +429,14 @@ func TestClientLazyImage(t *testing.T) { test.That(t, png.Encode(&imgBuf, img), test.ShouldBeNil) imgPng, err := png.Decode(bytes.NewReader(imgBuf.Bytes())) test.That(t, err, test.ShouldBeNil) + var jpegBuf bytes.Buffer + test.That(t, jpeg.Encode(&jpegBuf, img, &jpeg.Options{Quality: 100}), test.ShouldBeNil) + imgJpeg, err := jpeg.Decode(bytes.NewBuffer(jpegBuf.Bytes())) + test.That(t, err, test.ShouldBeNil) + injectCamera.PropertiesFunc = func(ctx context.Context) (camera.Properties, error) { + return camera.Properties{}, nil + } injectCamera.ImageFunc = func(ctx context.Context, mimeType string, extra map[string]interface{}) ([]byte, camera.ImageMetadata, error) { if mimeType == "" { mimeType = rutils.MimeTypeRawRGBA @@ -470,7 +478,6 @@ func TestClientLazyImage(t *testing.T) { frameLazy := frame.(*rimage.LazyEncodedImage) test.That(t, frameLazy.RawData(), test.ShouldResemble, imgBuf.Bytes()) - ctx = context.Background() frame, err = camera.DecodeImageFromCamera(ctx, rutils.WithLazyMIMEType(rutils.MimeTypePNG), nil, camera1Client) test.That(t, err, test.ShouldBeNil) test.That(t, frame, test.ShouldHaveSameTypeAs, &rimage.LazyEncodedImage{}) @@ -482,6 +489,24 @@ func TestClientLazyImage(t *testing.T) { test.That(t, err, test.ShouldBeNil) test.That(t, compVal, test.ShouldEqual, 0) // exact copy, no color conversion + // when DecodeImageFromCamera is called without a mime type, defaults to JPEG + var called bool + injectCamera.ImageFunc = func(ctx context.Context, mimeType string, extra map[string]interface{}) ([]byte, camera.ImageMetadata, error) { + called = true + test.That(t, mimeType, test.ShouldResemble, rutils.WithLazyMIMEType(rutils.MimeTypeJPEG)) + resBytes, err := rimage.EncodeImage(ctx, imgPng, mimeType) + test.That(t, err, test.ShouldBeNil) + return resBytes, camera.ImageMetadata{MimeType: mimeType}, nil + } + frame, err = camera.DecodeImageFromCamera(ctx, "", nil, camera1Client) + test.That(t, err, test.ShouldBeNil) + test.That(t, frame, test.ShouldHaveSameTypeAs, &rimage.LazyEncodedImage{}) + frameLazy = frame.(*rimage.LazyEncodedImage) + test.That(t, frameLazy.MIMEType(), test.ShouldEqual, rutils.MimeTypeJPEG) + compVal, _, err = rimage.CompareImages(imgJpeg, frame) + test.That(t, err, test.ShouldBeNil) + test.That(t, compVal, test.ShouldEqual, 0) // exact copy, no color conversion + test.That(t, called, test.ShouldBeTrue) test.That(t, conn.Close(), test.ShouldBeNil) } From 05d8b526520f2350bcf18d3c8f8ee12d3b75903d Mon Sep 17 00:00:00 2001 From: Nick Sanford Date: Mon, 6 Jan 2025 12:18:40 -0500 Subject: [PATCH 5/5] wip --- rimage/lazy_encoded.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rimage/lazy_encoded.go b/rimage/lazy_encoded.go index 39252632326..4314f2f67bb 100644 --- a/rimage/lazy_encoded.go +++ b/rimage/lazy_encoded.go @@ -6,6 +6,7 @@ import ( "image" "image/color" "net/http" + "strings" "sync" "go.viam.com/rdk/logging" @@ -37,6 +38,11 @@ func NewLazyEncodedImage(imgBytes []byte, mimeType string) image.Image { "Sniffing bytes to detect mime_type. Specify mime_type to reduce CPU utilization") mimeType = http.DetectContentType(imgBytes) } + + if !strings.HasPrefix(mimeType, "image/") { + logging.Global().Warnf("NewLazyEncodedImage resolving to non image mime_type: %s", mimeType) + } + return &LazyEncodedImage{ imgBytes: imgBytes, mimeType: mimeType,