Skip to content

Commit e0bd017

Browse files
authored
[RSDK-10100, RSDK-10111] - Add utils to lazy encoded image to decode and return errors instead of panic-ing always (viamrobotics#4826)
1 parent 978fcce commit e0bd017

File tree

10 files changed

+367
-61
lines changed

10 files changed

+367
-61
lines changed

components/camera/transformpipeline/mods_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,8 @@ func BenchmarkColorRotate(b *testing.B) {
608608
am := utils.AttributeMap{
609609
"angle_degs": 180,
610610
}
611-
vs := videoSourceFromCamera(context.Background(), src)
611+
vs, err := videoSourceFromCamera(context.Background(), src)
612+
test.That(b, err, test.ShouldBeNil)
612613
rs, stream, err := newRotateTransform(context.Background(), vs, camera.ColorStream, am)
613614
test.That(b, err, test.ShouldBeNil)
614615
test.That(b, stream, test.ShouldEqual, camera.ColorStream)
@@ -634,7 +635,8 @@ func BenchmarkDepthRotate(b *testing.B) {
634635
am := utils.AttributeMap{
635636
"angle_degs": 180,
636637
}
637-
vs := videoSourceFromCamera(context.Background(), src)
638+
vs, err := videoSourceFromCamera(context.Background(), src)
639+
test.That(b, err, test.ShouldBeNil)
638640
rs, stream, err := newRotateTransform(context.Background(), vs, camera.DepthStream, am)
639641
test.That(b, err, test.ShouldBeNil)
640642
test.That(b, stream, test.ShouldEqual, camera.DepthStream)

components/camera/transformpipeline/pipeline.go

+23-7
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ import (
2525

2626
var model = resource.DefaultModelFamily.WithModel("transform")
2727

28+
// ErrVideoSourceCreation is returned when creating a video source from a camera fails.
29+
var ErrVideoSourceCreation = errors.New("failed to create video source from camera")
30+
2831
func init() {
2932
resource.RegisterComponent(
3033
camera.API,
@@ -49,7 +52,10 @@ func init() {
4952
if err != nil {
5053
return nil, fmt.Errorf("no source camera for transform pipeline (%s): %w", sourceName, err)
5154
}
52-
vs := videoSourceFromCamera(ctx, source)
55+
vs, err := videoSourceFromCamera(ctx, source)
56+
if err != nil {
57+
return nil, fmt.Errorf("failed to create video source from camera: %w", err)
58+
}
5359
src, err := newTransformPipeline(ctx, vs, conf.ResourceName().AsNamed(), newConf, actualR, logger)
5460
if err != nil {
5561
return nil, err
@@ -103,14 +109,18 @@ func (sc *videoSource) Stream(ctx context.Context, errHandlers ...gostream.Error
103109
// and still implement a camera resource.
104110
// We prefer this methodology over passing Image bytes because each transform desires a image.Image over
105111
// a raw byte slice. To use Image would be to wastefully encode and decode the frame multiple times.
106-
func videoSourceFromCamera(ctx context.Context, cam camera.Camera) camera.VideoSource {
112+
func videoSourceFromCamera(ctx context.Context, cam camera.Camera) (camera.VideoSource, error) {
107113
if streamCam, ok := cam.(camera.VideoSource); ok {
108-
return streamCam
114+
return streamCam, nil
115+
}
116+
vs, err := camerautils.VideoSourceFromCamera(ctx, cam)
117+
if err != nil {
118+
return nil, fmt.Errorf("%w: %w", ErrVideoSourceCreation, err)
109119
}
110120
return &videoSource{
111121
Camera: cam,
112-
vs: camerautils.VideoSourceFromCamera(ctx, cam),
113-
}
122+
vs: vs,
123+
}, nil
114124
}
115125

116126
func newTransformPipeline(
@@ -142,13 +152,19 @@ func newTransformPipeline(
142152
}
143153
// loop through the pipeline and create the image flow
144154
pipeline := make([]camera.VideoSource, 0, len(cfg.Pipeline))
145-
lastSource := videoSourceFromCamera(ctx, source)
155+
lastSource, err := videoSourceFromCamera(ctx, source)
156+
if err != nil {
157+
return nil, err
158+
}
146159
for _, tr := range cfg.Pipeline {
147160
src, newStreamType, err := buildTransform(ctx, r, lastSource, streamType, tr)
148161
if err != nil {
149162
return nil, err
150163
}
151-
streamSrc := videoSourceFromCamera(ctx, src)
164+
streamSrc, err := videoSourceFromCamera(ctx, src)
165+
if err != nil {
166+
return nil, err
167+
}
152168
pipeline = append(pipeline, streamSrc)
153169
lastSource = streamSrc
154170
streamType = newStreamType

components/camera/transformpipeline/pipeline_test.go

+18-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package transformpipeline
22

33
import (
44
"context"
5+
"errors"
56
"testing"
67

78
"github.com/pion/mediadevices/pkg/prop"
@@ -34,7 +35,8 @@ func TestTransformPipelineColor(t *testing.T) {
3435
test.That(t, err, test.ShouldBeNil)
3536
source := gostream.NewVideoSource(&fake.StaticSource{ColorImg: img}, prop.Video{})
3637
src, err := camera.WrapVideoSourceWithProjector(context.Background(), source, nil, camera.ColorStream)
37-
vs := videoSourceFromCamera(context.Background(), src)
38+
test.That(t, err, test.ShouldBeNil)
39+
vs, err := videoSourceFromCamera(context.Background(), src)
3840
test.That(t, err, test.ShouldBeNil)
3941
inImg, err := camera.DecodeImageFromCamera(context.Background(), "", nil, src)
4042
test.That(t, err, test.ShouldBeNil)
@@ -87,7 +89,8 @@ func TestTransformPipelineDepth(t *testing.T) {
8789
test.That(t, inImg.Bounds().Dx(), test.ShouldEqual, 128)
8890
test.That(t, inImg.Bounds().Dy(), test.ShouldEqual, 72)
8991

90-
vs := videoSourceFromCamera(context.Background(), src)
92+
vs, err := videoSourceFromCamera(context.Background(), src)
93+
test.That(t, err, test.ShouldBeNil)
9194
depth, err := newTransformPipeline(context.Background(), vs, nil, transformConf, r, logger)
9295
test.That(t, err, test.ShouldBeNil)
9396

@@ -201,3 +204,16 @@ func TestTransformPipelineValidateFail(t *testing.T) {
201204
test.That(t, resource.GetFieldFromFieldRequiredError(err), test.ShouldEqual, "source")
202205
test.That(t, deps, test.ShouldBeNil)
203206
}
207+
208+
func TestVideoSourceFromCameraError(t *testing.T) {
209+
malformedCam := &inject.Camera{
210+
ImageFunc: func(ctx context.Context, mimeType string, extra map[string]interface{}) ([]byte, camera.ImageMetadata, error) {
211+
return []byte("not a valid image"), camera.ImageMetadata{MimeType: utils.MimeTypePNG}, nil
212+
},
213+
}
214+
215+
vs, err := videoSourceFromCamera(context.Background(), malformedCam)
216+
test.That(t, err, test.ShouldNotBeNil)
217+
test.That(t, errors.Is(err, ErrVideoSourceCreation), test.ShouldBeTrue)
218+
test.That(t, vs, test.ShouldBeNil)
219+
}

rimage/depth_map_raw.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,9 @@ func WriteRawDepthMapTo(img image.Image, out io.Writer) (int64, error) {
312312
// the Viam custom depth type writes 8 bytes of "magic number", 8 bytes of width, 8 bytes of height, and 2 bytes per pixel.
313313
func WriteViamDepthMapTo(img image.Image, out io.Writer) (int64, error) {
314314
if lazy, ok := img.(*LazyEncodedImage); ok {
315-
lazy.decode()
316-
if lazy.decodeErr != nil {
317-
return 0, errors.Errorf("could not decode LazyEncodedImage to a depth image: %v", lazy.decodeErr)
315+
err := lazy.DecodeImage()
316+
if err != nil {
317+
return 0, errors.Errorf("could not decode LazyEncodedImage to a depth image: %v", err)
318318
}
319319
img = lazy.decodedImage
320320
}

rimage/image_file.go

+30-10
Original file line numberDiff line numberDiff line change
@@ -179,19 +179,24 @@ func WriteImageToFile(path string, img image.Image) (err error) {
179179
}
180180
}
181181

182-
// ConvertImage converts a go image into our Image type.
183-
func ConvertImage(img image.Image) *Image {
182+
// ConvertImageSafe converts a go image into our Image type with error handling.
183+
// This is a safer alternative to ConvertImage as it returns errors instead of panicking.
184+
func ConvertImageSafe(img image.Image) (*Image, error) {
184185
if lazyImg, ok := img.(*LazyEncodedImage); ok {
185-
img = lazyImg.DecodedImage()
186+
decodedImg, err := lazyImg.DecodedImage()
187+
if err != nil {
188+
return nil, err
189+
}
190+
img = decodedImg
186191
}
187192
ii, ok := img.(*Image)
188193
if ok {
189-
return ii
194+
return ii, nil
190195
}
191196

192197
iwd, ok := img.(*imageWithDepth)
193198
if ok {
194-
return iwd.Color
199+
return iwd.Color, nil
195200
}
196201

197202
b := img.Bounds()
@@ -211,7 +216,18 @@ func ConvertImage(img image.Image) *Image {
211216
}
212217
}
213218
}
214-
return ii
219+
return ii, nil
220+
}
221+
222+
// ConvertImage converts a go image into our Image type.
223+
// This function panics if there's an error during conversion.
224+
// For a safer alternative that returns errors, use ConvertImageSafe.
225+
func ConvertImage(img image.Image) *Image {
226+
result, err := ConvertImageSafe(img)
227+
if err != nil {
228+
panic(err)
229+
}
230+
return result
215231
}
216232

217233
// CloneImage creates a copy of the input image.
@@ -241,7 +257,11 @@ func SaveImage(pic image.Image, loc string) error {
241257
}
242258
}()
243259
if lazyImg, ok := pic.(*LazyEncodedImage); ok {
244-
pic = lazyImg.DecodedImage()
260+
decodedPic, err := lazyImg.DecodedImage()
261+
if err != nil {
262+
return err
263+
}
264+
pic = decodedPic
245265
}
246266
if err = jpeg.Encode(f, pic, &jpeg.Options{Quality: 75}); err != nil {
247267
return errors.Wrapf(err, "the 'image' will not encode")
@@ -290,9 +310,9 @@ func EncodeImage(ctx context.Context, img image.Image, mimeType string) ([]byte,
290310
return lazy.imgBytes, nil
291311
}
292312
// LazyImage holds bytes different from requested mime type: decode and re-encode
293-
lazy.decode()
294-
if lazy.decodeErr != nil {
295-
return nil, errors.Errorf("could not decode LazyEncodedImage: %v", lazy.decodeErr)
313+
err := lazy.DecodeImage()
314+
if err != nil {
315+
return nil, errors.Errorf("could not decode LazyEncodedImage: %v", err)
296316
}
297317
return EncodeImage(ctx, lazy.decodedImage, actualOutMIME)
298318
}

0 commit comments

Comments
 (0)