Use agreed upon Image signature

viamrobotics · Nov 6, 2024 · d6439dd · d6439dd
1 parent f6e3d69
commit d6439dd
Show file tree

Hide file tree

Showing 42 changed files with 449 additions and 453 deletions.
diff --git a/components/camera/camera.go b/components/camera/camera.go
@@ -111,6 +111,10 @@ type Camera interface {
 //
 // [camera component docs]: https://docs.viam.com/components/camera/
 type VideoSource interface {
+	// Image returns a byte slice representing an image that tries to adhere to the MIME type hint.
+	// Image also may return a string representing the mime type hint or empty string if not.
+	Image(ctx context.Context, mimeType string, extra map[string]interface{}) ([]byte, string, error)
+
 	// Images is used for getting simultaneous images from different imagers,
 	// along with associated metadata (just timestamp for now). It's not for getting a time series of images from the same imager.
 	Images(ctx context.Context) ([]NamedImage, resource.ResponseMetadata, error)
@@ -119,9 +123,6 @@ type VideoSource interface {
 	// that may have a MIME type hint dictated in the context via gostream.WithMIMETypeHint.
 	Stream(ctx context.Context, errHandlers ...gostream.ErrorHandler) (gostream.VideoStream, error)
 
-	// GetImage returns a single image that may have a MIME type hint dictated in the context via gostream.WithMIMETypeHint.
-	GetImage(ctx context.Context) (image.Image, func(), error)
-
 	// NextPointCloud returns the next immediately available point cloud, not necessarily one
 	// a part of a sequence. In the future, there could be streaming of point clouds.
 	NextPointCloud(ctx context.Context) (pointcloud.PointCloud, error)

diff --git a/components/camera/camera_test.go b/components/camera/camera_test.go
@@ -10,7 +10,6 @@ import (
 	"go.viam.com/utils/artifact"
 
 	"go.viam.com/rdk/components/camera"
-	"go.viam.com/rdk/gostream"
 	"go.viam.com/rdk/pointcloud"
 	"go.viam.com/rdk/resource"
 	"go.viam.com/rdk/rimage"
@@ -183,13 +182,13 @@ func TestCameraWithNoProjector(t *testing.T) {
 	_, got := pc.At(0, 0, 0)
 	test.That(t, got, test.ShouldBeTrue)
 
-	img, _, err := noProj2.GetImage(gostream.WithMIMETypeHint(context.Background(), rutils.WithLazyMIMEType(rutils.MimeTypePNG)))
+	imgBytes, mimeType, err := noProj2.Image(context.Background(), rutils.WithLazyMIMEType(rutils.MimeTypePNG), nil)
 	test.That(t, err, test.ShouldBeNil)
-
-	depthImg := img.(*rimage.DepthMap)
+	img, err := rimage.DecodeImage(context.Background(), imgBytes, mimeType)
 	test.That(t, err, test.ShouldBeNil)
-	test.That(t, depthImg.Bounds().Dx(), test.ShouldEqual, 1280)
-	test.That(t, depthImg.Bounds().Dy(), test.ShouldEqual, 720)
+
+	test.That(t, img.Bounds().Dx(), test.ShouldEqual, 1280)
+	test.That(t, img.Bounds().Dy(), test.ShouldEqual, 720)
 
 	test.That(t, noProj2.Close(context.Background()), test.ShouldBeNil)
 }
@@ -232,13 +231,14 @@ func TestCameraWithProjector(t *testing.T) {
 	_, got := pc.At(0, 0, 0)
 	test.That(t, got, test.ShouldBeTrue)
 
-	img, _, err := cam2.GetImage(gostream.WithMIMETypeHint(context.Background(), rutils.MimeTypePNG))
+	imgBytes, mimeType, err := cam2.Image(context.Background(), rutils.MimeTypePNG, nil)
+	test.That(t, err, test.ShouldBeNil)
+	img, err := rimage.DecodeImage(context.Background(), imgBytes, mimeType)
 	test.That(t, err, test.ShouldBeNil)
 
-	depthImg := img.(*rimage.DepthMap)
 	test.That(t, err, test.ShouldBeNil)
-	test.That(t, depthImg.Bounds().Dx(), test.ShouldEqual, 1280)
-	test.That(t, depthImg.Bounds().Dy(), test.ShouldEqual, 720)
+	test.That(t, img.Bounds().Dx(), test.ShouldEqual, 1280)
+	test.That(t, img.Bounds().Dy(), test.ShouldEqual, 720)
 	// cam2 should implement a default GetImages, that just returns the one image
 	images, _, err := cam2.Images(context.Background())
 	test.That(t, err, test.ShouldBeNil)

diff --git a/components/camera/client.go b/components/camera/client.go
@@ -21,8 +21,6 @@ import (
 	goprotoutils "go.viam.com/utils/protoutils"
 	"go.viam.com/utils/rpc"
 	"golang.org/x/exp/slices"
-	"google.golang.org/protobuf/proto"
-	"google.golang.org/protobuf/types/known/structpb"
 
 	"go.viam.com/rdk/components/camera/rtppassthrough"
 	"go.viam.com/rdk/data"
@@ -100,60 +98,6 @@ func NewClientFromConn(
 	}, nil
 }
 
-func getExtra(ctx context.Context) (*structpb.Struct, error) {
-	ext := &structpb.Struct{}
-	if extra, ok := FromContext(ctx); ok {
-		var err error
-		if ext, err = goprotoutils.StructToStructPb(extra); err != nil {
-			return nil, err
-		}
-	}
-
-	dataExt, err := data.GetExtraFromContext(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	proto.Merge(ext, dataExt)
-	return ext, nil
-}
-
-// RSDK-8663: This method signature is depended on by the `camera.serviceServer` optimization that
-// avoids using an image stream just to get a single image.
-func (c *client) Read(ctx context.Context) (image.Image, func(), error) {
-	ctx, span := trace.StartSpan(ctx, "camera::client::Read")
-	defer span.End()
-	mimeType := gostream.MIMETypeHint(ctx, "")
-	expectedType, _ := utils.CheckLazyMIMEType(mimeType)
-
-	ext, err := getExtra(ctx)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	resp, err := c.client.GetImage(ctx, &pb.GetImageRequest{
-		Name:     c.name,
-		MimeType: expectedType,
-		Extra:    ext,
-	})
-	if err != nil {
-		return nil, nil, err
-	}
-
-	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
-	}
-
-	resp.MimeType = utils.WithLazyMIMEType(resp.MimeType)
-	img, err := rimage.DecodeImage(ctx, resp.Image, resp.MimeType)
-	if err != nil {
-		return nil, nil, err
-	}
-	return img, func() {}, nil
-}
-
 func (c *client) Stream(
 	ctx context.Context,
 	errHandlers ...gostream.ErrorHandler,
@@ -201,13 +145,19 @@ func (c *client) Stream(
 				return
 			}
 
-			frame, release, err := c.Read(streamCtx)
+			frame, mimeType, err := c.Image(streamCtx, gostream.MIMETypeHint(ctx, ""), nil)
 			if err != nil {
 				for _, handler := range errHandlers {
 					handler(streamCtx, err)
 				}
 			}
 
+			img, err := rimage.DecodeImage(ctx, frame, mimeType)
+			if err != nil {
+				c.logger.CWarnw(ctx, "error decoding image", "err", err)
+				return
+			}
+
 			select {
 			case <-streamCtx.Done():
 				return
@@ -217,8 +167,8 @@ func (c *client) Stream(
 				}
 				return
 			case frameCh <- gostream.MediaReleasePairWithError[image.Image]{
-				Media:   frame,
-				Release: release,
+				Media:   img,
+				Release: func() {},
 				Err:     err,
 			}:
 			}
@@ -228,8 +178,32 @@ func (c *client) Stream(
 	return stream, nil
 }
 
-func (c *client) GetImage(ctx context.Context) (image.Image, func(), error) {
-	return c.Read(ctx)
+func (c *client) Image(ctx context.Context, mimeType string, extra map[string]interface{}) ([]byte, string, error) {
+	ctx, span := trace.StartSpan(ctx, "camera::client::GetImage")
+	defer span.End()
+	expectedType, _ := utils.CheckLazyMIMEType(mimeType)
+
+	structExtra, err := goprotoutils.StructToStructPb(extra)
+	if err != nil {
+		return nil, "", err
+	}
+	resp, err := c.client.GetImage(ctx, &pb.GetImageRequest{
+		Name:     c.name,
+		MimeType: expectedType,
+		Extra:    structExtra,
+	})
+	if err != nil {
+		return nil, "", err
+	}
+
+	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
+	}
+
+	mimeType = utils.WithLazyMIMEType(resp.MimeType)
+	return resp.Image, mimeType, nil
 }
 
 func (c *client) Images(ctx context.Context) ([]NamedImage, resource.ResponseMetadata, error) {