Attempt to separate webcam and gostream entirely

components/camera/camera.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"image"
 
+	"github.com/pion/mediadevices/pkg/prop"
 	"github.com/pkg/errors"
 	pb "go.viam.com/api/component/camera/v1"
 
@@ -78,12 +79,6 @@ type ImageMetadata struct {
 }
 
 // A Camera is a resource that can capture frames.
-type Camera interface {
-	resource.Resource
-	VideoSource
-}
-
-// VideoSource represents anything that can capture frames.
 // For more information, see the [camera component docs].
 //
 // Image example:
@@ -102,17 +97,6 @@ type Camera interface {
 //
 //	images, metadata, err := myCamera.Images(context.Background())
 //
-// Stream example:
-//
-//	myCamera, err := camera.FromRobot(machine, "my_camera")
-//
-//	// gets the stream from a camera
-//	stream, err := myCamera.Stream(context.Background())
-//
-//	// gets an image from the camera stream
-//	img, release, err := stream.Next(context.Background())
-//	defer release()
-//
 // NextPointCloud example:
 //
 //	myCamera, err := camera.FromRobot(machine, "my_camera")
@@ -127,7 +111,8 @@ type Camera interface {
 //	err = myCamera.Close(context.Background())
 //
 // [camera component docs]: https://docs.viam.com/components/camera/
-type VideoSource interface {
+type Camera interface {
+	resource.Resource
 	// Image returns a byte slice representing an image that tries to adhere to the MIME type hint.
 	// Image also may return metadata about the frame.
 	Image(ctx context.Context, mimeType string, extra map[string]interface{}) ([]byte, ImageMetadata, error)
@@ -136,20 +121,20 @@ type VideoSource interface {
 	// 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)
 
-	// Stream returns a stream that makes a best effort to return consecutive images
-	// that may have a MIME type hint dictated in the context via gostream.WithMIMETypeHint.
-	Stream(ctx context.Context, errHandlers ...gostream.ErrorHandler) (gostream.VideoStream, 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)
 
 	// Properties returns properties that are intrinsic to the particular
 	// implementation of a camera.
 	Properties(ctx context.Context) (Properties, error)
+}
 
-	// Close shuts down the resource and prevents further use.
-	Close(ctx context.Context) error
+// StreamCamera is a camera that has `Stream` embedded to directly integrate with gostream.
+// Note that generally, when writing camera components from scratch, embedding `Stream` is an anti-pattern.
+type StreamCamera interface {
+	Camera
+	Stream(ctx context.Context, errHandlers ...gostream.ErrorHandler) (gostream.VideoStream, error)
 }
 
 // ReadImage reads an image from the given source that is immediately available.
@@ -173,6 +158,25 @@ func DecodeImageFromCamera(ctx context.Context, mimeType string, extra map[strin
 	return img, nil
 }
 
+// VideoSourceFromCamera converts a camera resource into a gostream VideoSource.
+func VideoSourceFromCamera(ctx context.Context, cam Camera) gostream.VideoSource {
+	reader := gostream.VideoReaderFunc(func(ctx context.Context) (image.Image, func(), error) {
+		img, err := DecodeImageFromCamera(ctx, "", nil, cam)
+		if err != nil {
+			return nil, func() {}, err
+		}
+		return img, func() {}, nil
+	})
+	camProps, err := cam.Properties(ctx)
+	if err != nil {
+		camProps = Properties{}
+	}
+	if camProps.IntrinsicParams == nil {
+		return gostream.NewVideoSource(reader, prop.Video{Width: 0, Height: 0})
+	}
+	return gostream.NewVideoSource(reader, prop.Video{Width: camProps.IntrinsicParams.Width, Height: camProps.IntrinsicParams.Height})
+}
+
 // A PointCloudSource is a source that can generate pointclouds.
 type PointCloudSource interface {
 	NextPointCloud(ctx context.Context) (pointcloud.PointCloud, error)

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/logging"
 	"go.viam.com/rdk/pointcloud"
 	"go.viam.com/rdk/resource"
@@ -186,18 +185,6 @@ func TestCameraWithNoProjector(t *testing.T) {
 	_, got := pc.At(0, 0, 0)
 	test.That(t, got, test.ShouldBeTrue)
 
-	// 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)
 
@@ -247,26 +234,6 @@ func TestCameraWithProjector(t *testing.T) {
 	_, got := pc.At(0, 0, 0)
 	test.That(t, got, test.ShouldBeTrue)
 
-	// 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)
 

components/camera/client_test.go

@@ -178,25 +178,6 @@ func TestClient(t *testing.T) {
 		test.That(t, err, test.ShouldBeNil)
 		camera1Client, err := camera.NewClientFromConn(context.Background(), conn, "", camera.Named(testCameraName), logger)
 		test.That(t, err, test.ShouldBeNil)
-		// TODO(hexbabe): remove below test when Stream/ReadImage pattern is refactored
-		t.Run("ReadImage from camera client 1", func(t *testing.T) {
-			// Test Stream and Next
-			ctx := gostream.WithMIMETypeHint(context.Background(), rutils.MimeTypeRawRGBA)
-			stream, err := camera1Client.Stream(ctx)
-			test.That(t, err, test.ShouldBeNil)
-			frame, _, err := stream.Next(ctx)
-			test.That(t, err, test.ShouldBeNil)
-			compVal, _, err := rimage.CompareImages(img, frame)
-			test.That(t, err, test.ShouldBeNil)
-			test.That(t, compVal, test.ShouldEqual, 0) // exact copy, no color conversion
-
-			// Test ReadImage
-			frame, _, err = camera.ReadImage(ctx, camera1Client)
-			test.That(t, err, test.ShouldBeNil)
-			compVal, _, err = rimage.CompareImages(img, frame)
-			test.That(t, err, test.ShouldBeNil)
-			test.That(t, compVal, test.ShouldEqual, 0)
-		})
 		frame, err := camera.DecodeImageFromCamera(context.Background(), rutils.MimeTypeRawRGBA, nil, camera1Client)
 		test.That(t, err, test.ShouldBeNil)
 		compVal, _, err := rimage.CompareImages(img, frame)
@@ -246,17 +227,6 @@ func TestClient(t *testing.T) {
 		client, err := resourceAPI.RPCClient(context.Background(), conn, "", camera.Named(depthCameraName), logger)
 		test.That(t, err, test.ShouldBeNil)
 
-		// TODO(hexbabe): remove below test when Stream/ReadImage pattern is refactored
-		t.Run("ReadImage from camera depth", func(t *testing.T) {
-			ctx := gostream.WithMIMETypeHint(
-				context.Background(), rutils.WithLazyMIMEType(rutils.MimeTypePNG))
-			frame, _, err := camera.ReadImage(ctx, client)
-			test.That(t, err, test.ShouldBeNil)
-			dm, err := rimage.ConvertImageToDepthMap(context.Background(), frame)
-			test.That(t, err, test.ShouldBeNil)
-			test.That(t, dm, test.ShouldResemble, depthImg)
-		})
-
 		ctx := context.Background()
 		frame, err := camera.DecodeImageFromCamera(ctx, rutils.WithLazyMIMEType(rutils.MimeTypePNG), nil, client)
 		test.That(t, err, test.ShouldBeNil)
@@ -274,10 +244,6 @@ func TestClient(t *testing.T) {
 		client2, err := resourceAPI.RPCClient(context.Background(), conn, "", camera.Named(failCameraName), logger)
 		test.That(t, err, test.ShouldBeNil)
 
-		_, _, err = camera.ReadImage(context.Background(), client2)
-		test.That(t, err, test.ShouldNotBeNil)
-		test.That(t, err.Error(), test.ShouldContainSubstring, errGetImageFailed.Error())
-
 		_, _, err = client2.Image(context.Background(), "", nil)
 		test.That(t, err, test.ShouldNotBeNil)
 		test.That(t, err.Error(), test.ShouldContainSubstring, errGetImageFailed.Error())
@@ -305,12 +271,6 @@ func TestClient(t *testing.T) {
 		}
 
 		ctx := context.Background()
-		// TODO(hexbabe): remove below test when Stream/ReadImage pattern is refactored
-		t.Run("ReadImage test failure", func(t *testing.T) {
-			_, _, err = camera.ReadImage(ctx, camClient)
-			test.That(t, err, test.ShouldNotBeNil)
-			test.That(t, err.Error(), test.ShouldContainSubstring, errGetImageFailed.Error())
-		})
 		_, _, err = camClient.Image(ctx, "", nil)
 		test.That(t, err, test.ShouldNotBeNil)
 		test.That(t, err.Error(), test.ShouldContainSubstring, errGetImageFailed.Error())
@@ -509,17 +469,6 @@ func TestClientLazyImage(t *testing.T) {
 	camera1Client, err := camera.NewClientFromConn(context.Background(), conn, "", camera.Named(testCameraName), logger)
 	test.That(t, err, test.ShouldBeNil)
 
-	// TODO(hexbabe): remove below test when Stream/ReadImage pattern is refactored
-	t.Run("lazily decode from ReadImage without lazy suffix", func(t *testing.T) {
-		ctx := gostream.WithMIMETypeHint(context.Background(), rutils.MimeTypePNG)
-		frame, _, err := camera.ReadImage(ctx, camera1Client)
-		test.That(t, err, test.ShouldBeNil)
-		// Should always lazily decode
-		test.That(t, frame, test.ShouldHaveSameTypeAs, &rimage.LazyEncodedImage{})
-		frameLazy := frame.(*rimage.LazyEncodedImage)
-		test.That(t, frameLazy.RawData(), test.ShouldResemble, imgBuf.Bytes())
-	})
-
 	ctx := context.Background()
 	frame, err := camera.DecodeImageFromCamera(ctx, rutils.MimeTypePNG, nil, camera1Client)
 	test.That(t, err, test.ShouldBeNil)
@@ -528,20 +477,6 @@ func TestClientLazyImage(t *testing.T) {
 	frameLazy := frame.(*rimage.LazyEncodedImage)
 	test.That(t, frameLazy.RawData(), test.ShouldResemble, imgBuf.Bytes())
 
-	// TODO(hexbabe): remove below test when Stream/ReadImage pattern is refactored
-	t.Run("lazily decode from ReadImage", func(t *testing.T) {
-		ctx = gostream.WithMIMETypeHint(context.Background(), rutils.WithLazyMIMEType(rutils.MimeTypePNG))
-		frame, _, err = camera.ReadImage(ctx, camera1Client)
-		test.That(t, err, test.ShouldBeNil)
-		test.That(t, frame, test.ShouldHaveSameTypeAs, &rimage.LazyEncodedImage{})
-		frameLazy = frame.(*rimage.LazyEncodedImage)
-		test.That(t, frameLazy.RawData(), test.ShouldResemble, imgBuf.Bytes())
-		test.That(t, frameLazy.MIMEType(), test.ShouldEqual, rutils.MimeTypePNG)
-		compVal, _, err := rimage.CompareImages(img, frame)
-		test.That(t, err, test.ShouldBeNil)
-		test.That(t, compVal, test.ShouldEqual, 0) // exact copy, no color conversion
-	})
-
 	ctx = context.Background()
 	frame, err = camera.DecodeImageFromCamera(ctx, rutils.WithLazyMIMEType(rutils.MimeTypePNG), nil, camera1Client)
 	test.That(t, err, test.ShouldBeNil)
@@ -619,78 +554,6 @@ func TestClientWithInterceptor(t *testing.T) {
 	test.That(t, conn.Close(), test.ShouldBeNil)
 }
 
-func TestClientStreamAfterClose(t *testing.T) {
-	// Set up gRPC server
-	logger := logging.NewTestLogger(t)
-	listener, err := net.Listen("tcp", "localhost:0")
-	test.That(t, err, test.ShouldBeNil)
-	rpcServer, err := rpc.NewServer(logger, rpc.WithUnauthenticated())
-	test.That(t, err, test.ShouldBeNil)
-
-	// Set up camera that can stream images
-	img := image.NewNRGBA(image.Rect(0, 0, 4, 4))
-	injectCamera := &inject.Camera{}
-	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) {
-		imgBytes, err := rimage.EncodeImage(ctx, img, mimeType)
-		test.That(t, err, test.ShouldBeNil)
-		return imgBytes, camera.ImageMetadata{MimeType: mimeType}, nil
-	}
-
-	// Register CameraService API in our gRPC server.
-	resources := map[resource.Name]camera.Camera{
-		camera.Named(testCameraName): injectCamera,
-	}
-	cameraSvc, err := resource.NewAPIResourceCollection(camera.API, resources)
-	test.That(t, err, test.ShouldBeNil)
-	resourceAPI, ok, err := resource.LookupAPIRegistration[camera.Camera](camera.API)
-	test.That(t, err, test.ShouldBeNil)
-	test.That(t, ok, test.ShouldBeTrue)
-	test.That(t, resourceAPI.RegisterRPCService(context.Background(), rpcServer, cameraSvc), test.ShouldBeNil)
-
-	// Start serving requests.
-	go rpcServer.Serve(listener)
-	defer rpcServer.Stop()
-
-	// Make client connection
-	conn, err := viamgrpc.Dial(context.Background(), listener.Addr().String(), logger)
-	test.That(t, err, test.ShouldBeNil)
-	client, err := camera.NewClientFromConn(context.Background(), conn, "", camera.Named(testCameraName), logger)
-	test.That(t, err, test.ShouldBeNil)
-
-	// Get a stream
-	stream, err := client.Stream(context.Background())
-	test.That(t, stream, test.ShouldNotBeNil)
-	test.That(t, err, test.ShouldBeNil)
-
-	// Read from stream
-	media, _, err := stream.Next(context.Background())
-	test.That(t, media, test.ShouldNotBeNil)
-	test.That(t, err, test.ShouldBeNil)
-
-	// Close client and read from stream
-	test.That(t, client.Close(context.Background()), test.ShouldBeNil)
-	media, _, err = stream.Next(context.Background())
-	test.That(t, media, test.ShouldBeNil)
-	test.That(t, err.Error(), test.ShouldContainSubstring, "context canceled")
-
-	// Get a new stream
-	stream, err = client.Stream(context.Background())
-	test.That(t, stream, test.ShouldNotBeNil)
-	test.That(t, err, test.ShouldBeNil)
-
-	// Read from the new stream
-	media, _, err = stream.Next(context.Background())
-	test.That(t, media, test.ShouldNotBeNil)
-	test.That(t, err, test.ShouldBeNil)
-
-	// Close client and connection
-	test.That(t, client.Close(context.Background()), test.ShouldBeNil)
-	test.That(t, conn.Close(), test.ShouldBeNil)
-}
-
 // See modmanager_test.go for the happy path (aka, when the
 // client has a webrtc connection).
 func TestRTPPassthroughWithoutWebRTC(t *testing.T) {

components/camera/fake/camera_test.go

@@ -94,20 +94,6 @@ func TestRTPPassthrough(t *testing.T) {
 		cam, err := NewCamera(context.Background(), nil, cfg, logger)
 		test.That(t, err, test.ShouldBeNil)
 
-		// TODO(hexbabe): remove below test when Stream/ReadImage pattern is refactored
-		t.Run("test Stream and Next", func(t *testing.T) {
-			camera, err := NewCamera(context.Background(), nil, cfg, logger)
-			test.That(t, err, test.ShouldBeNil)
-
-			stream, err := camera.Stream(context.Background())
-			test.That(t, err, test.ShouldBeNil)
-			img, _, err := stream.Next(context.Background())
-			test.That(t, err, test.ShouldBeNil)
-			// GetImage returns the world jpeg
-			test.That(t, img.Bounds(), test.ShouldResemble, image.Rectangle{Max: image.Point{X: 480, Y: 270}})
-			test.That(t, camera, test.ShouldNotBeNil)
-		})
-
 		img, err := camera.DecodeImageFromCamera(context.Background(), utils.MimeTypeRawRGBA, nil, cam)
 		test.That(t, err, test.ShouldBeNil)
 		// GetImage returns the world jpeg