From d9e193d482701987298dde233bd1039da3c84674 Mon Sep 17 00:00:00 2001 From: hexbabe Date: Mon, 17 Nov 2025 17:12:44 -0500 Subject: [PATCH 1/8] Fix hang --- components/camera/videosource/webcam.go | 77 +++++++++++++++++++------ 1 file changed, 60 insertions(+), 17 deletions(-) diff --git a/components/camera/videosource/webcam.go b/components/camera/videosource/webcam.go index 34f495c9dd3..aece8557745 100644 --- a/components/camera/videosource/webcam.go +++ b/components/camera/videosource/webcam.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "image" + "io" "path/filepath" "strings" "sync" @@ -248,10 +249,12 @@ func (c *webcam) Reconfigure( if err != nil { return err } - c.buffer.worker.Stop() // Calling this before locking shuts down the goroutines, and allows stopBuffer() to handle rest of the shutdown. + c.stopWorkers() c.mu.Lock() defer c.mu.Unlock() c.buffer.stopBuffer() + c.reader = nil + c.driver = nil c.cameraModel = camera.NewPinholeModelWithBrownConradyDistortion(newConf.CameraParameters, newConf.DistortionParameters) driverReinitNotNeeded := c.conf.Format == newConf.Format && @@ -533,29 +536,69 @@ func (c *webcam) startBuffer() { case <-closedCtx.Done(): return case <-ticker.C: - // We must unlock the mutex even if the release() or read() functions panic. - func() { - c.mu.Lock() - defer c.mu.Unlock() - if c.buffer.release != nil { - c.buffer.release() - c.buffer.release = nil - c.buffer.frame = nil + // Make a private copy of the previously published frame so consumers can continue to read it, + // then return the Reader-managed buffer before we kick off the next read. This avoids holding + // on to driver memory while still serving the last frame. + var prevRelease func() + c.mu.Lock() + if c.buffer.release != nil && c.buffer.frame != nil { + c.buffer.frame = rimage.CloneImage(c.buffer.frame) + } + prevRelease = c.buffer.release + c.buffer.release = nil + c.mu.Unlock() + + if prevRelease != nil { + prevRelease() + } + + img, release, err := c.reader.Read() + + c.mu.Lock() + c.buffer.err = err + if err != nil { + c.buffer.release = nil + c.buffer.frame = nil + c.logger.Errorf("error reading frame: %v", err) + isEOF := err == io.EOF + if isEOF { + c.logger.Warnf("camera disconnected (EOF), stopping buffer. Error: %v", err) + c.disconnected = true } - img, release, err := c.reader.Read() - c.buffer.err = err - if err != nil { - c.logger.Errorf("error reading frame: %v", err) - return // next iteration of for loop + c.mu.Unlock() + if isEOF { + return } - c.buffer.frame = img - c.buffer.release = release - }() + continue + } + c.buffer.frame = img + c.buffer.release = release + c.mu.Unlock() + } } }) } +// stopWorkers closes the current driver (if any) to unblock any pending reads +// and stops the buffer worker so reconfigure or shutdown operations don't hang. +func (c *webcam) stopWorkers() { + c.mu.Lock() + driver := c.driver + bufferWorker := c.buffer.worker + c.mu.Unlock() + + if driver != nil { + if err := driver.Close(); err != nil { + c.logger.Errorw("failed to close current camera before stopping buffer worker", "error", err) + } + } + + if bufferWorker != nil { + bufferWorker.Stop() + } +} + // Must lock the mutex before using this function. func (buffer *WebcamBuffer) stopBuffer() { if buffer == nil { From a6da87b6c07a621540addfda056b8aba42867e98 Mon Sep 17 00:00:00 2001 From: hexbabe Date: Mon, 17 Nov 2025 18:12:16 -0500 Subject: [PATCH 2/8] Make code better --- components/camera/videosource/webcam.go | 67 +++++++++++-------------- 1 file changed, 28 insertions(+), 39 deletions(-) diff --git a/components/camera/videosource/webcam.go b/components/camera/videosource/webcam.go index aece8557745..24c2330592e 100644 --- a/components/camera/videosource/webcam.go +++ b/components/camera/videosource/webcam.go @@ -249,10 +249,32 @@ func (c *webcam) Reconfigure( if err != nil { return err } - c.stopWorkers() + + // Stop the driver and frame buffer worker + c.mu.Lock() + driver := c.driver + frameBufferWorker := c.buffer.worker + c.mu.Unlock() + + if driver != nil { + if err := driver.Close(); err != nil { + c.logger.Errorw("failed to close current camera before stopping buffer worker", "error", err) + } + } + + if frameBufferWorker != nil { + frameBufferWorker.Stop() + } + + // Release buffer c.mu.Lock() defer c.mu.Unlock() - c.buffer.stopBuffer() + if c.buffer != nil { + if c.buffer.release != nil { + c.buffer.release() + c.buffer.release = nil + } + } c.reader = nil c.driver = nil @@ -282,7 +304,7 @@ func (c *webcam) Reconfigure( c.conf.FrameRate = defaultFrameRate } c.buffer = NewWebcamBuffer(c.workers.Context()) - c.startBuffer() + c.startFrameBufferWorker() return nil } @@ -522,9 +544,9 @@ func (c *webcam) getLatestFrame() (image.Image, error) { return c.buffer.frame, nil } -func (c *webcam) startBuffer() { +func (c *webcam) startFrameBufferWorker() { if c.buffer.frame != nil { - return // webcam buffer already started + return // frame buffer worker already started } interFrameDuration := time.Duration(float32(time.Second) / c.conf.FrameRate) @@ -560,7 +582,7 @@ func (c *webcam) startBuffer() { c.buffer.release = nil c.buffer.frame = nil c.logger.Errorf("error reading frame: %v", err) - isEOF := err == io.EOF + isEOF := errors.Is(err, io.EOF) if isEOF { c.logger.Warnf("camera disconnected (EOF), stopping buffer. Error: %v", err) c.disconnected = true @@ -574,44 +596,11 @@ func (c *webcam) startBuffer() { c.buffer.frame = img c.buffer.release = release c.mu.Unlock() - } } }) } -// stopWorkers closes the current driver (if any) to unblock any pending reads -// and stops the buffer worker so reconfigure or shutdown operations don't hang. -func (c *webcam) stopWorkers() { - c.mu.Lock() - driver := c.driver - bufferWorker := c.buffer.worker - c.mu.Unlock() - - if driver != nil { - if err := driver.Close(); err != nil { - c.logger.Errorw("failed to close current camera before stopping buffer worker", "error", err) - } - } - - if bufferWorker != nil { - bufferWorker.Stop() - } -} - -// Must lock the mutex before using this function. -func (buffer *WebcamBuffer) stopBuffer() { - if buffer == nil { - return - } - - // Release the remaining frame. - if buffer.release != nil { - buffer.release() - buffer.release = nil - } -} - func (c *webcam) Close(ctx context.Context) error { c.workers.Stop() c.buffer.worker.Stop() From 0b48406c1f085755886f10680e5afb8fe204fe16 Mon Sep 17 00:00:00 2001 From: hexbabe Date: Mon, 1 Dec 2025 13:05:37 -0500 Subject: [PATCH 3/8] Remove unnecessary initialize call; Add buffer worker safety code --- components/camera/videosource/webcam.go | 29 ++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/components/camera/videosource/webcam.go b/components/camera/videosource/webcam.go index 24c2330592e..b39cc6a445e 100644 --- a/components/camera/videosource/webcam.go +++ b/components/camera/videosource/webcam.go @@ -147,7 +147,6 @@ func findReaderAndDriver( path string, logger logging.Logger, ) (video.Reader, driverutils.Driver, string, error) { - mediadevicescamera.Initialize() constraints := makeConstraints(conf, logger) // Handle specific path @@ -393,13 +392,27 @@ func (c *webcam) Monitor() { case <-ticker.C: cont := func() bool { c.mu.Lock() - defer c.mu.Unlock() - if err := c.reconnectCamera(&c.conf); err != nil { + c.mu.Unlock() c.logger.Debugw("failed to reconnect camera", "error", err) return true } c.logger.Infow("camera reconnected") + + // Stop the buffer worker outside of the mutex to avoid deadlock + var oldWorker *goutils.StoppableWorkers + if c.buffer != nil && c.buffer.worker != nil { + oldWorker = c.buffer.worker + } + + c.buffer = NewWebcamBuffer(c.workers.Context()) + c.startFrameBufferWorker() + c.mu.Unlock() + + if oldWorker != nil { + oldWorker.Stop() + } + return false }() if cont { @@ -603,7 +616,6 @@ func (c *webcam) startFrameBufferWorker() { func (c *webcam) Close(ctx context.Context) error { c.workers.Stop() - c.buffer.worker.Stop() c.mu.Lock() defer c.mu.Unlock() if c.closed { @@ -611,5 +623,12 @@ func (c *webcam) Close(ctx context.Context) error { } c.closed = true - return c.driver.Close() + if c.buffer != nil && c.buffer.worker != nil { + c.buffer.worker.Stop() + } + + if c.driver != nil { + return c.driver.Close() + } + return nil } From cffd5e8ba6f30af466927bafb86e98521c160d2e Mon Sep 17 00:00:00 2001 From: hexbabe Date: Mon, 1 Dec 2025 14:32:07 -0500 Subject: [PATCH 4/8] Fix log and guard Initialize call to fix macos --- components/camera/videosource/webcam.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/components/camera/videosource/webcam.go b/components/camera/videosource/webcam.go index b39cc6a445e..a6f4d0a8f44 100644 --- a/components/camera/videosource/webcam.go +++ b/components/camera/videosource/webcam.go @@ -7,6 +7,7 @@ import ( "image" "io" "path/filepath" + "runtime" "strings" "sync" "time" @@ -147,6 +148,12 @@ func findReaderAndDriver( path string, logger logging.Logger, ) (video.Reader, driverutils.Driver, string, error) { + if runtime.GOOS == "linux" { + // TODO(RSDK-12789): Separate discover() calls from Initialize() calls. + // So we can call discover() without calling this overridden Initialize(), + // which behaves differently on darwin and other platforms. + mediadevicescamera.Initialize() + } constraints := makeConstraints(conf, logger) // Handle specific path @@ -597,7 +604,7 @@ func (c *webcam) startFrameBufferWorker() { c.logger.Errorf("error reading frame: %v", err) isEOF := errors.Is(err, io.EOF) if isEOF { - c.logger.Warnf("camera disconnected (EOF), stopping buffer. Error: %v", err) + c.logger.Warnf("camera disconnected (EOF), stopping buffer worker. Error: %v", err) c.disconnected = true } c.mu.Unlock() From d61d40c8faf81a4beddba646dc230e75720fc3c9 Mon Sep 17 00:00:00 2001 From: hexbabe Date: Tue, 2 Dec 2025 17:27:31 -0500 Subject: [PATCH 5/8] Move some mediadevices helpers to query.go --- components/camera/videosource/query.go | 104 +++++++++++++++++++++- components/camera/videosource/webcam.go | 110 ------------------------ 2 files changed, 102 insertions(+), 112 deletions(-) diff --git a/components/camera/videosource/query.go b/components/camera/videosource/query.go index dec6898a42a..6ac2e15fdae 100644 --- a/components/camera/videosource/query.go +++ b/components/camera/videosource/query.go @@ -3,13 +3,15 @@ package videosource import ( "fmt" "math" + "path/filepath" "strings" "time" "github.com/pion/mediadevices" "github.com/pion/mediadevices/pkg/driver" "github.com/pion/mediadevices/pkg/driver/availability" - "github.com/pion/mediadevices/pkg/driver/camera" + mediadevicescamera "github.com/pion/mediadevices/pkg/driver/camera" + "github.com/pion/mediadevices/pkg/frame" "github.com/pion/mediadevices/pkg/io/video" "github.com/pion/mediadevices/pkg/prop" "github.com/pkg/errors" @@ -22,6 +24,104 @@ import ( // However, this is the minimum code needed for webcam to work, placed in this directory. // This vastly improves the debugging and feature development experience, by not over-DRY-ing. +// makeConstraints is a helper that returns constraints to mediadevices in order to find and make a video source. +// Constraints are specifications for the video stream such as frame format, resolution etc. +func makeConstraints(conf *WebcamConfig, logger logging.Logger) mediadevices.MediaStreamConstraints { + return mediadevices.MediaStreamConstraints{ + Video: func(constraint *mediadevices.MediaTrackConstraints) { + if conf.Width > 0 { + constraint.Width = prop.IntExact(conf.Width) + } else { + constraint.Width = prop.IntRanged{Min: minResolutionDimension, Ideal: 640, Max: 4096} + } + + if conf.Height > 0 { + constraint.Height = prop.IntExact(conf.Height) + } else { + constraint.Height = prop.IntRanged{Min: minResolutionDimension, Ideal: 480, Max: 2160} + } + + if conf.FrameRate > 0.0 { + constraint.FrameRate = prop.FloatExact(conf.FrameRate) + } else { + constraint.FrameRate = prop.FloatRanged{Min: 0.0, Ideal: 30.0, Max: 140.0} + } + + if conf.Format == "" { + constraint.FrameFormat = prop.FrameFormatOneOf{ + frame.FormatI420, + frame.FormatI444, + frame.FormatYUY2, + frame.FormatUYVY, + frame.FormatRGBA, + frame.FormatMJPEG, + frame.FormatNV12, + frame.FormatNV21, + frame.FormatZ16, + } + } else { + constraint.FrameFormat = prop.FrameFormatExact(conf.Format) + } + + logger.Debugf("constraints: %v", constraint) + }, + } +} + +// findReaderAndDriver finds a video device and returns an image reader and the driver instance, +// as well as the path to the driver. +func findReaderAndDriver( + conf *WebcamConfig, + path string, + logger logging.Logger, +) (video.Reader, driver.Driver, string, error) { + mediadevicescamera.Initialize() + constraints := makeConstraints(conf, logger) + + // Handle specific path + if path != "" { + resolvedPath, err := filepath.EvalSymlinks(path) + if err == nil { + path = resolvedPath + } + reader, driver, err := getReaderAndDriver(filepath.Base(path), constraints, logger) + if err != nil { + return nil, nil, "", err + } + + img, release, err := reader.Read() + if release != nil { + defer release() + } + if err != nil { + return nil, nil, "", err + } + + if conf.Width != 0 && conf.Height != 0 { + if img.Bounds().Dx() != conf.Width || img.Bounds().Dy() != conf.Height { + return nil, nil, "", errors.Errorf("requested width and height (%dx%d) are not available for this webcam"+ + " (closest driver found supports resolution %dx%d)", + conf.Width, conf.Height, img.Bounds().Dx(), img.Bounds().Dy()) + } + } + return reader, driver, path, nil + } + + // Handle "any" path + reader, driver, err := getReaderAndDriver("", constraints, logger) + if err != nil { + return nil, nil, "", errors.Wrap(err, "found no webcams") + } + labels := strings.Split(driver.Info().Label, mediadevicescamera.LabelSeparator) + if len(labels) == 0 { + logger.Error("no labels parsed from driver") + return nil, nil, "", nil + } + path = labels[0] // path is always the first element + + return reader, driver, path, nil +} + // GetNamedVideoSource attempts to find a device (not a screen) by the given name. // If name is empty, it finds any device. func getReaderAndDriver( @@ -88,7 +188,7 @@ func labelFilter(target string, useSep bool) driver.FilterFn { if !useSep { return d.Info().Label == target } - labels := strings.Split(d.Info().Label, camera.LabelSeparator) + labels := strings.Split(d.Info().Label, mediadevicescamera.LabelSeparator) for _, label := range labels { if label == target { return true diff --git a/components/camera/videosource/webcam.go b/components/camera/videosource/webcam.go index a6f4d0a8f44..1b26b58544a 100644 --- a/components/camera/videosource/webcam.go +++ b/components/camera/videosource/webcam.go @@ -6,19 +6,12 @@ import ( "fmt" "image" "io" - "path/filepath" - "runtime" - "strings" "sync" "time" - "github.com/pion/mediadevices" driverutils "github.com/pion/mediadevices/pkg/driver" "github.com/pion/mediadevices/pkg/driver/availability" - mediadevicescamera "github.com/pion/mediadevices/pkg/driver/camera" - "github.com/pion/mediadevices/pkg/frame" "github.com/pion/mediadevices/pkg/io/video" - "github.com/pion/mediadevices/pkg/prop" "github.com/pkg/errors" goutils "go.viam.com/utils" @@ -97,109 +90,6 @@ func (c WebcamConfig) Validate(path string) ([]string, []string, error) { return []string{}, nil, nil } -// makeConstraints is a helper that returns constraints to mediadevices in order to find and make a video source. -// Constraints are specifications for the video stream such as frame format, resolution etc. -func makeConstraints(conf *WebcamConfig, logger logging.Logger) mediadevices.MediaStreamConstraints { - return mediadevices.MediaStreamConstraints{ - Video: func(constraint *mediadevices.MediaTrackConstraints) { - if conf.Width > 0 { - constraint.Width = prop.IntExact(conf.Width) - } else { - constraint.Width = prop.IntRanged{Min: minResolutionDimension, Ideal: 640, Max: 4096} - } - - if conf.Height > 0 { - constraint.Height = prop.IntExact(conf.Height) - } else { - constraint.Height = prop.IntRanged{Min: minResolutionDimension, Ideal: 480, Max: 2160} - } - - if conf.FrameRate > 0.0 { - constraint.FrameRate = prop.FloatExact(conf.FrameRate) - } else { - constraint.FrameRate = prop.FloatRanged{Min: 0.0, Ideal: 30.0, Max: 140.0} - } - - if conf.Format == "" { - constraint.FrameFormat = prop.FrameFormatOneOf{ - frame.FormatI420, - frame.FormatI444, - frame.FormatYUY2, - frame.FormatUYVY, - frame.FormatRGBA, - frame.FormatMJPEG, - frame.FormatNV12, - frame.FormatNV21, - frame.FormatZ16, - } - } else { - constraint.FrameFormat = prop.FrameFormatExact(conf.Format) - } - - logger.Debugf("constraints: %v", constraint) - }, - } -} - -// findReaderAndDriver finds a video device and returns an image reader and the driver instance, -// as well as the path to the driver. -func findReaderAndDriver( - conf *WebcamConfig, - path string, - logger logging.Logger, -) (video.Reader, driverutils.Driver, string, error) { - if runtime.GOOS == "linux" { - // TODO(RSDK-12789): Separate discover() calls from Initialize() calls. - // So we can call discover() without calling this overridden Initialize(), - // which behaves differently on darwin and other platforms. - mediadevicescamera.Initialize() - } - constraints := makeConstraints(conf, logger) - - // Handle specific path - if path != "" { - resolvedPath, err := filepath.EvalSymlinks(path) - if err == nil { - path = resolvedPath - } - reader, driver, err := getReaderAndDriver(filepath.Base(path), constraints, logger) - if err != nil { - return nil, nil, "", err - } - - img, release, err := reader.Read() - if release != nil { - defer release() - } - if err != nil { - return nil, nil, "", err - } - - if conf.Width != 0 && conf.Height != 0 { - if img.Bounds().Dx() != conf.Width || img.Bounds().Dy() != conf.Height { - return nil, nil, "", errors.Errorf("requested width and height (%dx%d) are not available for this webcam"+ - " (closest driver found supports resolution %dx%d)", - conf.Width, conf.Height, img.Bounds().Dx(), img.Bounds().Dy()) - } - } - return reader, driver, path, nil - } - - // Handle "any" path - reader, driver, err := getReaderAndDriver("", constraints, logger) - if err != nil { - return nil, nil, "", errors.Wrap(err, "found no webcams") - } - labels := strings.Split(driver.Info().Label, mediadevicescamera.LabelSeparator) - if len(labels) == 0 { - logger.Error("no labels parsed from driver") - return nil, nil, "", nil - } - path = labels[0] // path is always the first element - - return reader, driver, path, nil -} - // webcam is a video driver wrapper camera that ensures its underlying driver stays connected. type webcam struct { resource.Named From 96f6cdc10da93ee7c361005dab75395422f7baf2 Mon Sep 17 00:00:00 2001 From: hexbabe Date: Mon, 8 Dec 2025 10:46:41 -0500 Subject: [PATCH 6/8] Refactor --- components/camera/videosource/query.go | 15 +- components/camera/videosource/webcam.go | 494 +++++++++--------------- 2 files changed, 207 insertions(+), 302 deletions(-) diff --git a/components/camera/videosource/query.go b/components/camera/videosource/query.go index 6ac2e15fdae..7ad96b22f4f 100644 --- a/components/camera/videosource/query.go +++ b/components/camera/videosource/query.go @@ -4,6 +4,7 @@ import ( "fmt" "math" "path/filepath" + "runtime" "strings" "time" @@ -19,6 +20,13 @@ import ( "go.viam.com/rdk/logging" ) +// minResolutionDimension is set to 2 to ensure proper fitness distance calculation for resolution selection. +// Setting this to 0 would cause mediadevices' IntRanged.Compare() method to treat all values smaller than ideal +// as equally acceptable. See https://github.com/pion/mediadevices/blob/c10fb000dbbb28597e068468f3175dc68a281bfd/pkg/prop/int.go#L104 +// Setting it to 1 could theoretically allow 1x1 resolutions. 2 is small enough and even, +// allowing all real camera resolutions while ensuring proper distance calculations. +const minResolutionDimension = 2 + // Below is adapted from github.com/pion/mediadevices. // It is further adapted from gostream's query.go // However, this is the minimum code needed for webcam to work, placed in this directory. @@ -75,7 +83,12 @@ func findReaderAndDriver( path string, logger logging.Logger, ) (video.Reader, driver.Driver, string, error) { - mediadevicescamera.Initialize() + if runtime.GOOS == "linux" { + // TODO(RSDK-12789): Separate discover() calls from Initialize() calls. + // So we can call Initialize() only once, and call discover() as many times as we need. + mediadevicescamera.Initialize() + } + constraints := makeConstraints(conf, logger) // Handle specific path diff --git a/components/camera/videosource/webcam.go b/components/camera/videosource/webcam.go index 1b26b58544a..b9630ff1c30 100644 --- a/components/camera/videosource/webcam.go +++ b/components/camera/videosource/webcam.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "image" - "io" "sync" "time" @@ -20,8 +19,6 @@ import ( "go.viam.com/rdk/logging" "go.viam.com/rdk/pointcloud" "go.viam.com/rdk/resource" - "go.viam.com/rdk/rimage" - "go.viam.com/rdk/rimage/depthadapter" "go.viam.com/rdk/rimage/transform" "go.viam.com/rdk/spatialmath" "go.viam.com/rdk/utils" @@ -31,18 +28,14 @@ import ( var ModelWebcam = resource.DefaultModelFamily.WithModel("webcam") var ( - errClosed = errors.New("camera has been closed") - errDisconnected = errors.New("camera is disconnected; please try again in a few moments") + errClosed = errors.New("webcam has been closed") + errDisconnected = errors.New("webcam is disconnected; please try again in a few moments") + errNoFrames = errors.New("no frames available to read") + errNoDriver = errors.New("no camera driver set") ) -// minResolutionDimension is set to 2 to ensure proper fitness distance calculation for resolution selection. -// Setting this to 0 would cause mediadevices' IntRanged.Compare() method to treat all values smaller than ideal -// as equally acceptable. See https://github.com/pion/mediadevices/blob/c10fb000dbbb28597e068468f3175dc68a281bfd/pkg/prop/int.go#L104 -// Setting it to 1 could theoretically allow 1x1 resolutions. 2 is small enough and even, -// allowing all real camera resolutions while ensuring proper distance calculations. const ( - minResolutionDimension = 2 - defaultFrameRate = float32(30.0) + defaultFrameRate = float32(30.0) ) func init() { @@ -54,15 +47,6 @@ func init() { }) } -// WebcamBuffer is a buffer for webcam frames. -// WARNING: This struct is NOT thread safe. It must be protected by the mutex in the webcam struct. -type WebcamBuffer struct { - frame image.Image // Holds the frames and their release functions in the buffer - release func() - err error - worker *goutils.StoppableWorkers // A separate worker for the webcam buffer that allows stronger concurrency control. -} - // WebcamConfig is the native config attribute struct for webcams. type WebcamConfig struct { CameraParameters *transform.PinholeCameraIntrinsics `json:"intrinsic_parameters,omitempty"` @@ -90,18 +74,24 @@ func (c WebcamConfig) Validate(path string) ([]string, []string, error) { return []string{}, nil, nil } -// webcam is a video driver wrapper camera that ensures its underlying driver stays connected. +// webcam is a video driver wrapper camera that ensures its underlying driver stays connected, +// handling hot unplugs/replugs, and provides a buffer to read frames from. type webcam struct { resource.Named - mu sync.RWMutex - hasLoggedIntrinsicsInfo bool + resource.AlwaysRebuild + // mu protects all struct fields except workers. + // + // Lock ordering rules: + // - Never hold mu when calling workers.Stop() or workers.Add() + // - Workers may acquire mu, so stopping them while holding mu causes deadlock + mu sync.Mutex cameraModel transform.PinholeCameraModel reader video.Reader driver driverutils.Driver - // this is returned to us as a label in mediadevices but our config + // This is returned to us as a label in mediadevices but our config // treats it as a video path. targetPath string conf WebcamConfig @@ -109,9 +99,22 @@ type webcam struct { closed bool disconnected bool logger logging.Logger - workers *goutils.StoppableWorkers + // workers take the mutex when starting and stopping, so do not call them while holding mu + workers *goutils.StoppableWorkers + buffer *webcamBuffer +} + +// webcamBuffer is a buffer for webcam frames. +// It must be protected by the mutex in the webcam struct. +type webcamBuffer struct { + frame image.Image + release func() + err error +} - buffer *WebcamBuffer +// newWebcamBuffer creates a new WebcamBuffer struct. +func newWebcamBuffer() *webcamBuffer { + return &webcamBuffer{} } // NewWebcam returns the webcam discovered based on the given config as the Camera interface type. @@ -121,96 +124,65 @@ func NewWebcam( conf resource.Config, logger logging.Logger, ) (camera.Camera, error) { - cam := &webcam{ + c := &webcam{ Named: conf.ResourceName().AsNamed(), logger: logger.WithFields("camera_name", conf.ResourceName().ShortName()), workers: goutils.NewBackgroundStoppableWorkers(), + buffer: newWebcamBuffer(), } - cam.buffer = NewWebcamBuffer(cam.workers.Context()) - if err := cam.Reconfigure(ctx, deps, conf); err != nil { + nativeConf, err := resource.NativeConfig[*WebcamConfig](conf) + if err != nil { return nil, err } - cam.Monitor() - return cam, nil -} + c.cameraModel = camera.NewPinholeModelWithBrownConradyDistortion(nativeConf.CameraParameters, nativeConf.DistortionParameters) -func (c *webcam) Reconfigure( - ctx context.Context, - _ resource.Dependencies, - conf resource.Config, -) error { - newConf, err := resource.NativeConfig[*WebcamConfig](conf) + c.targetPath = nativeConf.Path + reader, driver, label, err := findReaderAndDriver(nativeConf, c.targetPath, c.logger) if err != nil { - return err + return nil, fmt.Errorf("failed to find camera: %w", err) } - // Stop the driver and frame buffer worker - c.mu.Lock() - driver := c.driver - frameBufferWorker := c.buffer.worker - c.mu.Unlock() - - if driver != nil { - if err := driver.Close(); err != nil { - c.logger.Errorw("failed to close current camera before stopping buffer worker", "error", err) - } + c.reader = reader + c.driver = driver + c.disconnected = false + if c.targetPath == "" { + c.targetPath = label } + c.logger = c.logger.WithFields("camera_name", c.Name().ShortName(), "camera_label", c.targetPath) - if frameBufferWorker != nil { - frameBufferWorker.Stop() - } + // only set once we're good + c.conf = *nativeConf - // Release buffer - c.mu.Lock() - defer c.mu.Unlock() - if c.buffer != nil { - if c.buffer.release != nil { - c.buffer.release() - c.buffer.release = nil - } + if c.conf.FrameRate == 0.0 { + c.conf.FrameRate = defaultFrameRate } - c.reader = nil - c.driver = nil - c.cameraModel = camera.NewPinholeModelWithBrownConradyDistortion(newConf.CameraParameters, newConf.DistortionParameters) - driverReinitNotNeeded := c.conf.Format == newConf.Format && - c.conf.Path == newConf.Path && - c.conf.Width == newConf.Width && - c.conf.Height == newConf.Height + // Start both workers after successful configuration + c.startMonitorWorker() + c.startBufferWorker() - if c.driver != nil && c.reader != nil && driverReinitNotNeeded { - c.conf = *newConf - return nil - } - c.logger.CDebug(ctx, "reinitializing driver") + return c, nil +} - c.targetPath = newConf.Path - if err := c.reconnectCamera(newConf); err != nil { - return err +// ensureActive checks the camera's state and returns the appropriate error if it is not active. +// Must be called with mu held. +func (c *webcam) ensureActive() error { + if c.closed { + return errClosed } - - c.hasLoggedIntrinsicsInfo = false - - // only set once we're good - c.conf = *newConf - - if c.conf.FrameRate == 0.0 { - c.conf.FrameRate = defaultFrameRate + if c.disconnected { + return errDisconnected } - c.buffer = NewWebcamBuffer(c.workers.Context()) - c.startFrameBufferWorker() - return nil } // isCameraConnected is a helper for monitoring connectivity to the driver. +// Must be called with mu held. func (c *webcam) isCameraConnected() (bool, error) { - c.mu.RLock() - defer c.mu.RUnlock() if c.driver == nil { - return true, errors.New("no configured camera") + return false, fmt.Errorf("cannot determine camera status: %w", errNoDriver) } // TODO(RSDK-1959): this only works for linux @@ -218,44 +190,9 @@ func (c *webcam) isCameraConnected() (bool, error) { return !errors.Is(err, availability.ErrNoDevice), nil } -// reconnectCamera tries to reconnect the camera to a driver that matches the config. -// Assumes a write lock is held. -func (c *webcam) reconnectCamera(conf *WebcamConfig) error { - if c.driver != nil { - c.logger.Debug("closing current camera") - if err := c.driver.Close(); err != nil { - c.logger.Errorw("failed to close current camera", "error", err) - } - c.driver = nil - c.reader = nil - } - - reader, driver, foundLabel, err := findReaderAndDriver(conf, c.targetPath, c.logger) - if err != nil { - return errors.Wrap(err, "failed to find camera") - } - - c.reader = reader - c.driver = driver - c.disconnected = false - c.closed = false - if c.targetPath == "" { - c.targetPath = foundLabel - } - - c.logger = c.logger.WithFields("camera_label", c.targetPath) - - return nil -} - -// Monitor is responsible for monitoring the liveness of a camera. An example -// is connectivity. Since the model itself knows best about how to maintain this state, -// the reconfigurable offers a safe way to notify if a state needs to be reset due -// to some exceptional event (like a reconnect). -// It is expected that the monitoring code is tied to the lifetime of the resource -// and once the resource is closed, so should the monitor. That is, it should -// no longer send any resets once a Close on its associated resource has returned. -func (c *webcam) Monitor() { +// startMonitorWorker starts a worker that monitors camera connectivity and handles reconnection. +// This worker runs continuously until the context is cancelled (via workers.Stop()). +func (c *webcam) startMonitorWorker() { c.workers.Add(func(ctx context.Context) { ticker := time.NewTicker(500 * time.Millisecond) defer ticker.Stop() @@ -263,13 +200,14 @@ func (c *webcam) Monitor() { for { select { case <-ctx.Done(): + c.logger.Debug("monitor worker context done") return case <-ticker.C: - c.mu.RLock() + c.mu.Lock() logger := c.logger - c.mu.RUnlock() - ok, err := c.isCameraConnected() + c.mu.Unlock() + if err != nil { logger.Debugw("cannot determine camera status", "error", err) continue @@ -285,36 +223,41 @@ func (c *webcam) Monitor() { for { select { case <-ctx.Done(): + c.logger.Debug("reconnect loop context done") + c.mu.Unlock() return case <-ticker.C: - cont := func() bool { - c.mu.Lock() - if err := c.reconnectCamera(&c.conf); err != nil { - c.mu.Unlock() - c.logger.Debugw("failed to reconnect camera", "error", err) - return true - } - c.logger.Infow("camera reconnected") + c.mu.Lock() - // Stop the buffer worker outside of the mutex to avoid deadlock - var oldWorker *goutils.StoppableWorkers - if c.buffer != nil && c.buffer.worker != nil { - oldWorker = c.buffer.worker + // Close current driver if it exists + if c.driver != nil { + c.logger.Debug("closing current camera") + if err := c.driver.Close(); err != nil { + c.logger.Errorw("failed to close current camera", "error", err) } + c.driver = nil + c.reader = nil + } - c.buffer = NewWebcamBuffer(c.workers.Context()) - c.startFrameBufferWorker() + // Try to find and reconnect to camera + reader, driver, label, err := findReaderAndDriver(&c.conf, c.targetPath, c.logger) + if err != nil { + c.logger.Debugw("failed to reconnect camera", "error", err) c.mu.Unlock() - - if oldWorker != nil { - oldWorker.Stop() - } - - return false - }() - if cont { continue } + + // Successfully reconnected + c.reader = reader + c.driver = driver + c.disconnected = false + if c.targetPath == "" { + c.targetPath = label + } + c.logger = c.logger.WithFields("camera_name", c.Name().ShortName(), "camera_label", c.targetPath) + + c.logger.Infow("camera reconnected") + c.mu.Unlock() break reconnectLoop } } @@ -324,94 +267,103 @@ func (c *webcam) Monitor() { }) } -func (c *webcam) Images( - ctx context.Context, - filterSourceNames []string, - extra map[string]interface{}, -) ([]camera.NamedImage, resource.ResponseMetadata, error) { - c.mu.RLock() - defer c.mu.RUnlock() - if err := c.ensureActive(); err != nil { - return nil, resource.ResponseMetadata{}, err - } +// startBufferWorker starts a worker that continuously reads frames from the camera and writes them to the buffer. +// When disconnected, it skips reading but continues running to resume immediately upon reconnection. +func (c *webcam) startBufferWorker() { + c.mu.Lock() + frameRate := c.conf.FrameRate + c.mu.Unlock() - img, err := c.getLatestFrame() - if err != nil { - return nil, resource.ResponseMetadata{}, errors.Wrap(err, "monitoredWebcam: call to get Images failed") - } + interFrameDuration := time.Duration(float32(time.Second) / frameRate) + ticker := time.NewTicker(interFrameDuration) - namedImg, err := camera.NamedImageFromImage(img, c.Name().Name, utils.MimeTypeJPEG, data.Annotations{}) - if err != nil { - return nil, resource.ResponseMetadata{}, err - } - return []camera.NamedImage{namedImg}, resource.ResponseMetadata{CapturedAt: time.Now()}, nil -} + c.workers.Add(func(ctx context.Context) { + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + c.logger.Debug("buffer worker context done") + return + case <-ticker.C: + c.mu.Lock() -// ensureActive is a helper that guards logic that requires the camera to be actively connected. -func (c *webcam) ensureActive() error { - if c.closed { - return errClosed - } - if c.disconnected { - return errDisconnected - } - return nil -} + if c.disconnected { + c.mu.Unlock() + continue + } -func (c *webcam) Image(ctx context.Context, mimeType string, extra map[string]interface{}) ([]byte, camera.ImageMetadata, error) { - c.mu.RLock() - defer c.mu.RUnlock() + reader := c.reader + if reader == nil { + c.mu.Unlock() + continue + } - if err := c.ensureActive(); err != nil { - return nil, camera.ImageMetadata{}, err - } - if c.reader == nil { - return nil, camera.ImageMetadata{}, errors.New("underlying reader is nil") - } - img, err := c.getLatestFrame() - if err != nil { - return nil, camera.ImageMetadata{}, err - } + // Get the release function to call outside the lock to avoid potential deadlocks. + // + // Note: mediadevices decoders return empty release functions (no-ops), + // but we call it to follow the Reader interface, and in case the decoders + // do someday return a non-nil release function. + oldRelease := c.buffer.release + c.buffer.release = nil + c.mu.Unlock() - if mimeType == "" { - mimeType = utils.MimeTypeJPEG - } - imgBytes, err := rimage.EncodeImage(ctx, img, mimeType) - if err != nil { - return nil, camera.ImageMetadata{}, err - } - return imgBytes, camera.ImageMetadata{MimeType: mimeType}, nil -} + // Call release and read outside the lock to avoid holding the lock during I/O + if oldRelease != nil { + oldRelease() + } + img, release, err := reader.Read() -func (c *webcam) NextPointCloud(ctx context.Context, extra map[string]interface{}) (pointcloud.PointCloud, error) { - c.mu.RLock() - defer c.mu.RUnlock() + c.mu.Lock() + c.buffer.err = err + if err != nil { + c.buffer.release = nil + c.buffer.frame = nil + c.logger.Errorf("error reading frame: %v", err) + c.mu.Unlock() + continue + } + c.buffer.frame = img + c.buffer.release = release + c.mu.Unlock() + } + } + }) +} +func (c *webcam) Images(_ context.Context, _ []string, _ map[string]interface{}) ([]camera.NamedImage, resource.ResponseMetadata, error) { + c.mu.Lock() + defer c.mu.Unlock() if err := c.ensureActive(); err != nil { - return nil, err + return nil, resource.ResponseMetadata{}, err } - if c.cameraModel.PinholeCameraIntrinsics == nil { - return nil, transform.NewNoIntrinsicsError("cannot do a projection to a point cloud") + if c.buffer.frame == nil { + if c.buffer.err != nil { + return nil, resource.ResponseMetadata{}, c.buffer.err + } + return nil, resource.ResponseMetadata{}, errNoFrames } - img, release, err := c.reader.Read() + img := c.buffer.frame + namedImg, err := camera.NamedImageFromImage(img, c.Name().Name, utils.MimeTypeJPEG, data.Annotations{}) if err != nil { - return nil, err + return nil, resource.ResponseMetadata{}, fmt.Errorf("failed to create named image: %w", err) } - defer release() - dm, err := rimage.ConvertImageToDepthMap(ctx, img) + return []camera.NamedImage{namedImg}, resource.ResponseMetadata{CapturedAt: time.Now()}, nil +} + +func (c *webcam) Image(ctx context.Context, _ string, extra map[string]interface{}) ([]byte, camera.ImageMetadata, error) { + imgBytes, resMetadata, err := camera.GetImageFromGetImages(ctx, nil, c, extra, nil) if err != nil { - return nil, errors.Wrapf(err, "cannot project to a point cloud") + return nil, camera.ImageMetadata{}, err } - - return depthadapter.ToPointCloud(dm, c.cameraModel.PinholeCameraIntrinsics), nil + return imgBytes, resMetadata, nil } func (c *webcam) Properties(ctx context.Context) (camera.Properties, error) { - c.mu.RLock() - defer c.mu.RUnlock() + c.mu.Lock() + defer c.mu.Unlock() if err := c.ensureActive(); err != nil { return camera.Properties{}, err @@ -422,7 +374,7 @@ func (c *webcam) Properties(ctx context.Context) (camera.Properties, error) { frameRate = c.conf.FrameRate } return camera.Properties{ - SupportsPCD: c.cameraModel.PinholeCameraIntrinsics != nil, + SupportsPCD: false, // RGB webcams cannot generate point clouds ImageType: camera.ColorStream, IntrinsicParams: c.cameraModel.PinholeCameraIntrinsics, DistortionParams: c.cameraModel.Distortion, @@ -431,101 +383,41 @@ func (c *webcam) Properties(ctx context.Context) (camera.Properties, error) { }, nil } -func (c *webcam) Geometries(ctx context.Context, extra map[string]interface{}) ([]spatialmath.Geometry, error) { - return make([]spatialmath.Geometry, 0), nil -} - -// NewWebcamBuffer creates a new WebcamBuffer struct. -func NewWebcamBuffer(ctx context.Context) *WebcamBuffer { - return &WebcamBuffer{ - worker: goutils.NewStoppableWorkers(ctx), - } -} - -// Must lock the mutex before calling this function. -func (c *webcam) getLatestFrame() (image.Image, error) { - if c.buffer.frame == nil { - if c.buffer.err != nil { - return nil, c.buffer.err - } - return nil, errors.New("no frames available to read") - } - - return c.buffer.frame, nil +func (c *webcam) NextPointCloud(ctx context.Context, extra map[string]interface{}) (pointcloud.PointCloud, error) { + return nil, errors.New("not supported for webcams") } -func (c *webcam) startFrameBufferWorker() { - if c.buffer.frame != nil { - return // frame buffer worker already started - } - - interFrameDuration := time.Duration(float32(time.Second) / c.conf.FrameRate) - ticker := time.NewTicker(interFrameDuration) - c.buffer.worker.Add(func(closedCtx context.Context) { - defer ticker.Stop() - for { - select { - case <-closedCtx.Done(): - return - case <-ticker.C: - // Make a private copy of the previously published frame so consumers can continue to read it, - // then return the Reader-managed buffer before we kick off the next read. This avoids holding - // on to driver memory while still serving the last frame. - var prevRelease func() - c.mu.Lock() - if c.buffer.release != nil && c.buffer.frame != nil { - c.buffer.frame = rimage.CloneImage(c.buffer.frame) - } - prevRelease = c.buffer.release - c.buffer.release = nil - c.mu.Unlock() - - if prevRelease != nil { - prevRelease() - } - - img, release, err := c.reader.Read() - - c.mu.Lock() - c.buffer.err = err - if err != nil { - c.buffer.release = nil - c.buffer.frame = nil - c.logger.Errorf("error reading frame: %v", err) - isEOF := errors.Is(err, io.EOF) - if isEOF { - c.logger.Warnf("camera disconnected (EOF), stopping buffer worker. Error: %v", err) - c.disconnected = true - } - c.mu.Unlock() - if isEOF { - return - } - continue - } - c.buffer.frame = img - c.buffer.release = release - c.mu.Unlock() - } - } - }) +func (c *webcam) Geometries(ctx context.Context, extra map[string]interface{}) ([]spatialmath.Geometry, error) { + return make([]spatialmath.Geometry, 0), nil } func (c *webcam) Close(ctx context.Context) error { + // Stop workers before acquiring mu c.workers.Stop() + c.mu.Lock() defer c.mu.Unlock() + if c.closed { - return errors.New("webcam already closed") + return fmt.Errorf("webcam failed to close: %w", errClosed) } c.closed = true - if c.buffer != nil && c.buffer.worker != nil { - c.buffer.worker.Stop() + if c.buffer.release != nil { + c.buffer.release() } + c.buffer.release = nil + c.buffer.frame = nil + + c.reader = nil if c.driver != nil { - return c.driver.Close() + err := c.driver.Close() + c.driver = nil + if err != nil { + return fmt.Errorf("webcam failed to close (failed to close camera driver): %w", err) + } } + return nil } From da690ff4738ae0bfbee45ffbb4003f7487cfb936 Mon Sep 17 00:00:00 2001 From: hexbabe Date: Mon, 8 Dec 2025 12:19:24 -0500 Subject: [PATCH 7/8] Warn, not fail, on mismatched dimensions --- components/camera/videosource/query.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/camera/videosource/query.go b/components/camera/videosource/query.go index 7ad96b22f4f..c500376d85a 100644 --- a/components/camera/videosource/query.go +++ b/components/camera/videosource/query.go @@ -112,8 +112,7 @@ func findReaderAndDriver( if conf.Width != 0 && conf.Height != 0 { if img.Bounds().Dx() != conf.Width || img.Bounds().Dy() != conf.Height { - return nil, nil, "", errors.Errorf("requested width and height (%dx%d) are not available for this webcam"+ - " (closest driver found supports resolution %dx%d)", + logger.Warnf("requested width and height (%dx%d) do not match actual webcam resolution (%dx%d); using actual resolution", conf.Width, conf.Height, img.Bounds().Dx(), img.Bounds().Dy()) } } From d6b5e91660d08288f5f726f9ee2d992eeeb8b525 Mon Sep 17 00:00:00 2001 From: hexbabe Date: Mon, 8 Dec 2025 13:04:15 -0500 Subject: [PATCH 8/8] Fix error msg and generally just clean up --- components/camera/videosource/webcam.go | 100 ++++++++++--------- components/camera/videosource/webcam_test.go | 6 +- 2 files changed, 57 insertions(+), 49 deletions(-) diff --git a/components/camera/videosource/webcam.go b/components/camera/videosource/webcam.go index b9630ff1c30..455b4b36483 100644 --- a/components/camera/videosource/webcam.go +++ b/components/camera/videosource/webcam.go @@ -63,11 +63,11 @@ func (c WebcamConfig) Validate(path string) ([]string, []string, error) { if c.Width < 0 || c.Height < 0 { return nil, nil, fmt.Errorf( "got illegal negative dimensions for width_px and height_px (%d, %d) fields set for webcam camera", - c.Height, c.Width) + c.Width, c.Height) } if c.FrameRate < 0 { return nil, nil, fmt.Errorf( - "got illegal non-positive dimension for frame rate (%.2f) field set for webcam camera", + "got illegal negative frame rate (%.2f) field set for webcam camera", c.FrameRate) } @@ -212,54 +212,62 @@ func (c *webcam) startMonitorWorker() { logger.Debugw("cannot determine camera status", "error", err) continue } + if ok { + continue + } - if !ok { - c.mu.Lock() - c.disconnected = true - c.mu.Unlock() - - logger.Error("camera no longer connected; reconnecting") - reconnectLoop: - for { - select { - case <-ctx.Done(): - c.logger.Debug("reconnect loop context done") - c.mu.Unlock() - return - case <-ticker.C: - c.mu.Lock() - - // Close current driver if it exists - if c.driver != nil { - c.logger.Debug("closing current camera") - if err := c.driver.Close(); err != nil { - c.logger.Errorw("failed to close current camera", "error", err) - } - c.driver = nil - c.reader = nil - } - - // Try to find and reconnect to camera - reader, driver, label, err := findReaderAndDriver(&c.conf, c.targetPath, c.logger) - if err != nil { - c.logger.Debugw("failed to reconnect camera", "error", err) - c.mu.Unlock() - continue - } + c.mu.Lock() + c.disconnected = true + c.mu.Unlock() - // Successfully reconnected - c.reader = reader - c.driver = driver - c.disconnected = false - if c.targetPath == "" { - c.targetPath = label + logger.Error("camera no longer connected; reconnecting") + reconnectLoop: + for { + select { + case <-ctx.Done(): + c.logger.Debug("reconnect loop context done") + return + case <-ticker.C: + c.mu.Lock() + + // Close current driver if it exists + if c.driver != nil { + c.logger.Debug("closing current camera") + if err := c.driver.Close(); err != nil { + c.logger.Errorw("failed to close current camera", "error", err) } - c.logger = c.logger.WithFields("camera_name", c.Name().ShortName(), "camera_label", c.targetPath) + c.driver = nil + c.reader = nil + } - c.logger.Infow("camera reconnected") + // Try to find and reconnect to camera + reader, driver, label, err := findReaderAndDriver(&c.conf, c.targetPath, c.logger) + if err != nil { + c.logger.Debugw("failed to reconnect camera", "error", err) c.mu.Unlock() - break reconnectLoop + continue } + + // Successfully reconnected + c.reader = reader + c.driver = driver + c.disconnected = false + if c.targetPath == "" { + c.targetPath = label + } + c.logger = c.logger.WithFields("camera_name", c.Name().ShortName(), "camera_label", c.targetPath) + + // Reset buffer state + c.buffer.err = nil + c.buffer.frame = nil + if c.buffer.release != nil { + c.buffer.release() + c.buffer.release = nil + } + + c.logger.Infow("camera reconnected") + c.mu.Unlock() + break reconnectLoop } } } @@ -275,9 +283,9 @@ func (c *webcam) startBufferWorker() { c.mu.Unlock() interFrameDuration := time.Duration(float32(time.Second) / frameRate) - ticker := time.NewTicker(interFrameDuration) c.workers.Add(func(ctx context.Context) { + ticker := time.NewTicker(interFrameDuration) defer ticker.Stop() for { select { @@ -318,7 +326,7 @@ func (c *webcam) startBufferWorker() { if err != nil { c.buffer.release = nil c.buffer.frame = nil - c.logger.Errorf("error reading frame: %v", err) + c.logger.Errorw("error reading frame", "error", err) c.mu.Unlock() continue } diff --git a/components/camera/videosource/webcam_test.go b/components/camera/videosource/webcam_test.go index f4027c27dc9..64fb57e80e5 100644 --- a/components/camera/videosource/webcam_test.go +++ b/components/camera/videosource/webcam_test.go @@ -32,7 +32,7 @@ func TestWebcamValidation(t *testing.T) { webCfg.Width = -200 deps, _, err = webCfg.Validate("path") test.That(t, err.Error(), test.ShouldEqual, - "got illegal negative dimensions for width_px and height_px (0, -200) fields set for webcam camera") + "got illegal negative dimensions for width_px and height_px (-200, 0) fields set for webcam camera") test.That(t, deps, test.ShouldBeNil) // error with a negative height @@ -40,7 +40,7 @@ func TestWebcamValidation(t *testing.T) { webCfg.Height = -200 deps, _, err = webCfg.Validate("path") test.That(t, err.Error(), test.ShouldEqual, - "got illegal negative dimensions for width_px and height_px (-200, 200) fields set for webcam camera") + "got illegal negative dimensions for width_px and height_px (200, -200) fields set for webcam camera") test.That(t, deps, test.ShouldBeNil) // error with a negative frame rate @@ -48,6 +48,6 @@ func TestWebcamValidation(t *testing.T) { webCfg.FrameRate = -100 deps, _, err = webCfg.Validate("path") test.That(t, err.Error(), test.ShouldEqual, - "got illegal non-positive dimension for frame rate (-100.00) field set for webcam camera") + "got illegal negative frame rate (-100.00) field set for webcam camera") test.That(t, deps, test.ShouldBeNil) }