RSDK-9132-with-performance-tests-with-nick

components/camera/camera.go

@@ -6,6 +6,7 @@ package camera
 
 import (
 	"context"
+	"fmt"
 	"image"
 
 	"github.com/pkg/errors"
@@ -15,6 +16,7 @@ import (
 	"go.viam.com/rdk/gostream"
 	"go.viam.com/rdk/pointcloud"
 	"go.viam.com/rdk/resource"
+	"go.viam.com/rdk/rimage"
 	"go.viam.com/rdk/rimage/transform"
 	"go.viam.com/rdk/robot"
 )
@@ -70,15 +72,32 @@ type NamedImage struct {
 	SourceName string
 }
 
+// ImageMetadata contains useful information about returned image bytes such as its mimetype.
+type ImageMetadata struct {
+	MimeType string
+}
+
 // A Camera is a resource that can capture frames.
 type Camera interface {
 	resource.Resource
 	VideoSource
 }
 
-// A VideoSource represents anything that can capture frames.
+// VideoSource represents anything that can capture frames.
 // For more information, see the [camera component docs].
 //
+// Image example:
+//
+//	myCamera, err := camera.FromRobot(machine, "my_camera")
+//
+//	// gets an image from the camera
+//	imageBytes, mimeType, err := myCamera.Image(context.Background(), utils.MimeTypeJPEG, nil)
+//
+// Or try to directly decode into an image.Image:
+//
+//	myCamera, err := camera.FromRobot(machine, "my_camera")
+//	img, err = camera.DecodeImageFromCamera(context.Background(), utils.MimeTypeJPEG, nil, myCamera)
+//
 // Images example:
 //
 //	myCamera, err := camera.FromRobot(machine, "my_camera")
@@ -111,6 +130,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, ImageMetadata, 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)
@@ -136,6 +159,22 @@ func ReadImage(ctx context.Context, src gostream.VideoSource) (image.Image, func
 	return gostream.ReadImage(ctx, src)
 }
 
+// DecodeImageFromCamera retrieves image bytes from a camera resource and serializes it as an image.Image.
+func DecodeImageFromCamera(ctx context.Context, mimeType string, extra map[string]interface{}, cam Camera) (image.Image, error) {
+	resBytes, resMetadata, err := cam.Image(ctx, mimeType, extra)
+	if err != nil {
+		return nil, fmt.Errorf("could not get image bytes from camera: %w", err)
+	}
+	if len(resBytes) == 0 {
+		return nil, errors.New("received empty bytes from camera")
+	}
+	img, err := rimage.DecodeImage(ctx, resBytes, resMetadata.MimeType)
+	if err != nil {
+		return nil, fmt.Errorf("could not decode into image.Image: %w", err)
+	}
+	return img, nil
+}
+
 // A PointCloudSource is a source that can generate pointclouds.
 type PointCloudSource interface {
 	NextPointCloud(ctx context.Context) (pointcloud.PointCloud, error)

components/camera/camera_test.go

@@ -11,6 +11,7 @@ import (
 
 	"go.viam.com/rdk/components/camera"
 	"go.viam.com/rdk/gostream"
+	"go.viam.com/rdk/logging"
 	"go.viam.com/rdk/pointcloud"
 	"go.viam.com/rdk/resource"
 	"go.viam.com/rdk/rimage"
@@ -168,35 +169,46 @@ func (cs *cloudSource) NextPointCloud(ctx context.Context) (pointcloud.PointClou
 }
 
 func TestCameraWithNoProjector(t *testing.T) {
+	logger := logging.NewTestLogger(t)
 	videoSrc := &simpleSource{"rimage/board1"}
 	noProj, err := camera.NewVideoSourceFromReader(context.Background(), videoSrc, nil, camera.DepthStream)
 	test.That(t, err, test.ShouldBeNil)
 	_, err = noProj.NextPointCloud(context.Background())
 	test.That(t, errors.Is(err, transform.ErrNoIntrinsics), test.ShouldBeTrue)
 
 	// make a camera with a NextPointCloudFunction
-	videoSrc2 := &cloudSource{Named: camera.Named("foo").AsNamed(), simpleSource: videoSrc}
-	noProj2, err := camera.NewVideoSourceFromReader(context.Background(), videoSrc2, nil, camera.DepthStream)
+	cloudSrc2 := &cloudSource{Named: camera.Named("foo").AsNamed(), simpleSource: videoSrc}
+	videoSrc2, err := camera.NewVideoSourceFromReader(context.Background(), cloudSrc2, nil, camera.DepthStream)
+	noProj2 := camera.FromVideoSource(resource.NewName(camera.API, "bar"), videoSrc2, logger)
 	test.That(t, err, test.ShouldBeNil)
 	pc, err := noProj2.NextPointCloud(context.Background())
 	test.That(t, err, test.ShouldBeNil)
 	_, got := pc.At(0, 0, 0)
 	test.That(t, got, test.ShouldBeTrue)
 
-	img, _, err := camera.ReadImage(
-		gostream.WithMIMETypeHint(context.Background(), rutils.WithLazyMIMEType(rutils.MimeTypePNG)),
-		noProj2)
+	// TODO(hexbabe): remove below test when Stream is refactored
+	t.Run("ReadImage depth map without projector", func(t *testing.T) {
+		img, _, err := camera.ReadImage(
+			gostream.WithMIMETypeHint(context.Background(), rutils.WithLazyMIMEType(rutils.MimeTypePNG)),
+			noProj2)
+		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)
+	})
+
+	img, err := camera.DecodeImageFromCamera(context.Background(), rutils.WithLazyMIMEType(rutils.MimeTypePNG), nil, noProj2)
 	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)
 
 	test.That(t, noProj2.Close(context.Background()), test.ShouldBeNil)
 }
 
 func TestCameraWithProjector(t *testing.T) {
+	logger := logging.NewTestLogger(t)
 	videoSrc := &simpleSource{"rimage/board1"}
 	params1 := &transform.PinholeCameraIntrinsics{ // not the real camera parameters -- fake for test
 		Width:  1280,
@@ -219,32 +231,50 @@ func TestCameraWithProjector(t *testing.T) {
 	test.That(t, src.Close(context.Background()), test.ShouldBeNil)
 
 	// camera with a point cloud function
-	videoSrc2 := &cloudSource{Named: camera.Named("foo").AsNamed(), simpleSource: videoSrc}
+	cloudSrc2 := &cloudSource{Named: camera.Named("foo").AsNamed(), simpleSource: videoSrc}
 	props, err := src.Properties(context.Background())
 	test.That(t, err, test.ShouldBeNil)
-	cam2, err := camera.NewVideoSourceFromReader(
+	videoSrc2, err := camera.NewVideoSourceFromReader(
 		context.Background(),
-		videoSrc2,
+		cloudSrc2,
 		&transform.PinholeCameraModel{PinholeCameraIntrinsics: props.IntrinsicParams},
 		camera.DepthStream,
 	)
+	cam2 := camera.FromVideoSource(resource.NewName(camera.API, "bar"), videoSrc2, logger)
 	test.That(t, err, test.ShouldBeNil)
-	pc, err = cam2.NextPointCloud(context.Background())
+	pc, err = videoSrc2.NextPointCloud(context.Background())
 	test.That(t, err, test.ShouldBeNil)
 	_, got := pc.At(0, 0, 0)
 	test.That(t, got, test.ShouldBeTrue)
 
-	img, _, err := camera.ReadImage(
-		gostream.WithMIMETypeHint(context.Background(), rutils.MimeTypePNG),
-		cam2)
+	// TODO(hexbabe): remove below test when Stream/ReadImage pattern is refactored
+	t.Run("ReadImage depth map with projector", func(t *testing.T) {
+		img, _, err := camera.ReadImage(
+			gostream.WithMIMETypeHint(context.Background(), rutils.MimeTypePNG),
+			cam2)
+		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)
+		// cam2 should implement a default GetImages, that just returns the one image
+		images, _, err := cam2.Images(context.Background())
+		test.That(t, err, test.ShouldBeNil)
+		test.That(t, len(images), test.ShouldEqual, 1)
+		test.That(t, images[0].Image, test.ShouldHaveSameTypeAs, &rimage.DepthMap{})
+		test.That(t, images[0].Image.Bounds().Dx(), test.ShouldEqual, 1280)
+		test.That(t, images[0].Image.Bounds().Dy(), test.ShouldEqual, 720)
+	})
+
+	img, err := camera.DecodeImageFromCamera(context.Background(), rutils.MimeTypePNG, nil, cam2)
 	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())
+	images, _, err := videoSrc2.Images(context.Background())
 	test.That(t, err, test.ShouldBeNil)
 	test.That(t, len(images), test.ShouldEqual, 1)
 	test.That(t, images[0].Image, test.ShouldHaveSameTypeAs, &rimage.DepthMap{})

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,
@@ -184,7 +128,8 @@ func (c *client) Stream(
 	// with those from the new "generation".
 	healthyClientCh := c.maybeResetHealthyClientCh()
 
-	ctxWithMIME := gostream.WithMIMETypeHint(context.Background(), gostream.MIMETypeHint(ctx, ""))
+	mimeTypeFromCtx := gostream.MIMETypeHint(ctx, "")
+	ctxWithMIME := gostream.WithMIMETypeHint(context.Background(), mimeTypeFromCtx)
 	streamCtx, stream, frameCh := gostream.NewMediaStreamForChannel[image.Image](ctxWithMIME)
 
 	c.activeBackgroundWorkers.Add(1)
@@ -201,7 +146,7 @@ func (c *client) Stream(
 				return
 			}
 
-			frame, release, err := c.Read(streamCtx)
+			img, err := DecodeImageFromCamera(streamCtx, mimeTypeFromCtx, nil, c)
 			if err != nil {
 				for _, handler := range errHandlers {
 					handler(streamCtx, err)
@@ -217,8 +162,8 @@ func (c *client) Stream(
 				}
 				return
 			case frameCh <- gostream.MediaReleasePairWithError[image.Image]{
-				Media:   frame,
-				Release: release,
+				Media:   img,
+				Release: func() {},
 				Err:     err,
 			}:
 			}
@@ -228,6 +173,36 @@ func (c *client) Stream(
 	return stream, nil
 }
 
+func (c *client) Image(ctx context.Context, mimeType string, extra map[string]interface{}) ([]byte, ImageMetadata, error) {
+	ctx, span := trace.StartSpan(ctx, "camera::client::Image")
+	defer span.End()
+	expectedType, _ := utils.CheckLazyMIMEType(mimeType)
+
+	convertedExtra, err := goprotoutils.StructToStructPb(extra)
+	if err != nil {
+		return nil, ImageMetadata{}, err
+	}
+	resp, err := c.client.GetImage(ctx, &pb.GetImageRequest{
+		Name:     c.name,
+		MimeType: expectedType,
+		Extra:    convertedExtra,
+	})
+	if err != nil {
+		return nil, ImageMetadata{}, err
+	}
+	if len(resp.Image) == 0 {
+		return nil, ImageMetadata{}, errors.New("received empty bytes from client GetImage")
+	}
+
+	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
+	}
+
+	return resp.Image, ImageMetadata{MimeType: utils.WithLazyMIMEType(resp.MimeType)}, nil
+}
+
 func (c *client) Images(ctx context.Context) ([]NamedImage, resource.ResponseMetadata, error) {
 	ctx, span := trace.StartSpan(ctx, "camera::client::Images")
 	defer span.End()