[RSDK-9660] - fix-camera-image-returning-no-mime-type #4674

nicksanford · 2025-01-03T19:00:23Z

https://viam.atlassian.net/browse/RSDK-9660

This fixes a bug in which the vision.CaptureAllFromCamera data collector receives an *rimage.LazyEncodedImage with a mime_type of empty string. This condition happens when a builtin vision service uses a remote or modular camera to get it's image. This causes those images to show up under the files tab in app.viam.com rather than the data tab when they are synced to the cloud.

Change camera.DecodeImageFromCamera to always return a lazy image (for lower resource usage)
Change camera.client.Image to return the mime_type of the camera.GetImageResponse when the camera.Image caller doesn't specify a mime_type. This is needed so that vision services (which frequently don't specify a mime_type in their call to camera.client.Image) to still be able to detect the mime_type of the image.
Change camera.client.Image to no longer wrap the mime_type in the +lazy suffix, as the lazy suffix is a directive to the rimage package to wrap the image bytes in a lazy image, which is something which camera.client.Image should be deciding for all callers, especially given that the +lazy suffix will not be understood by mime_type libraries outside of viam. Point 1 makes it so that this isn't a breaking change for all RDK code.
Change rimage.NewLazyEncodedImage to log a warning when the caller doesn't specify a mime_type and detect the mime type by sniffing the image's bytes.

github-actions · 2025-01-03T19:00:36Z

Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!

component	function
base	IsMoving
board	GPIOPinByName
camera	Properties
encoder	Properties
motor	IsMoving
sensor	Readings
servo	Position
arm	EndPosition
audio	MediaProperties
gantry	Lengths
gripper	IsMoving
input_controller	Controls
movement_sensor	LinearAcceleration
power_sensor	Power
pose_tracker	Poses
motion	GetPose
vision	GetProperties

nicksanford · 2025-01-03T19:35:01Z

components/camera/client.go

 	}

-	return resp.Image, ImageMetadata{MimeType: utils.WithLazyMIMEType(resp.MimeType)}, nil
+	return resp.Image, ImageMetadata{MimeType: resp.MimeType}, nil


I'm moving this utils.WithLazyMIMEType call here: https://github.com/viamrobotics/rdk/pull/4674/files#diff-4eab9b93978c09650defc23c4274a4c77d87cfdea7775d86dcf8c838ec207a01R170 as that is the only caller of Image in viam-server and the reason why we were adding the lazy mime type.

nicksanford · 2025-01-03T19:36:25Z

components/camera/client.go

+		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


this makes it so that we only override the camera.GetImageResponse.MimeType if it is empty string and the caller specified an expected mime type.

nicksanford · 2025-01-03T22:11:12Z

rimage/lazy_encoded.go

+	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)


This will attempt to detect the mime type of the image bytes if mimeType is empty.

This is a cool function from that library - nice to know it's in there.

It seems this method can return 3 mime types that would be interesting to us:

image/*

application/octet-stream

something else

It might be worth logging if this code path is getting application/octet-stream
Definitely worth logging if it's "something else"

great suggestion, done

nicksanford · 2025-01-03T22:24:28Z

components/camera/client_test.go

+		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


change to non lazy

randhid

LGTM!

randhid · 2025-01-06T15:55:34Z

rimage/lazy_encoded.go

+	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)


This is a cool function from that library - nice to know it's in there.

dgottlieb

I can't check the changes too closely for bugs -- I'm not well versed enough in this to spot something subtle.

Left a suggestion regarding the automatic detection. Doesn't change how applications would behave in the PR, just a diagnostic improvement.

dgottlieb · 2025-01-06T16:00:02Z

rimage/lazy_encoded.go

+	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)


It seems this method can return 3 mime types that would be interesting to us:

image/*

application/octet-stream

something else

It might be worth logging if this code path is getting application/octet-stream
Definitely worth logging if it's "something else"

viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jan 3, 2025

viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jan 3, 2025

nicksanford commented Jan 3, 2025

View reviewed changes

viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jan 3, 2025

nicksanford commented Jan 3, 2025

View reviewed changes

nicksanford added 4 commits January 6, 2025 10:28

fix-camera-image-returning-no-mime-type

be4f369

lint

e850ece

wip

b2a7095

wip

ed37538

nicksanford force-pushed the fix-camera-image-returning-no-mime-type branch from 79e5393 to ed37538 Compare January 6, 2025 15:28

viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jan 6, 2025

nicksanford changed the title ~~fix-camera-image-returning-no-mime-type~~ [RSDK-9660] - fix-camera-image-returning-no-mime-type Jan 6, 2025

nicksanford marked this pull request as ready for review January 6, 2025 15:42

nicksanford requested review from randhid and dgottlieb January 6, 2025 15:46

randhid approved these changes Jan 6, 2025

View reviewed changes

dgottlieb approved these changes Jan 6, 2025

View reviewed changes

wip

05d8b52

viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jan 6, 2025

nicksanford merged commit 7297934 into viamrobotics:main Jan 6, 2025
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RSDK-9660] - fix-camera-image-returning-no-mime-type #4674

[RSDK-9660] - fix-camera-image-returning-no-mime-type #4674

nicksanford commented Jan 3, 2025 •

edited

Loading

github-actions bot commented Jan 3, 2025

nicksanford Jan 3, 2025

nicksanford Jan 3, 2025

nicksanford Jan 3, 2025

randhid Jan 6, 2025

dgottlieb Jan 6, 2025

nicksanford Jan 6, 2025

nicksanford Jan 3, 2025

randhid left a comment

randhid Jan 6, 2025

dgottlieb left a comment

dgottlieb Jan 6, 2025

[RSDK-9660] - fix-camera-image-returning-no-mime-type #4674

[RSDK-9660] - fix-camera-image-returning-no-mime-type #4674

Conversation

nicksanford commented Jan 3, 2025 • edited Loading

github-actions bot commented Jan 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

randhid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgottlieb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicksanford commented Jan 3, 2025 •

edited

Loading