| Message ID | 20220903181037.1406-4-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 6225d647b47982833fa654bf9cf65eb8c04d2036 |
| Headers | show |
| Series |
|
| Related | show |
Quoting Laurent Pinchart via libcamera-devel (2022-09-03 19:10:37) > A UVC device could expose only formats that are not supported by > libcamera. The pipeline handler doesn't currently consider this as an > error, and happily creates a camera. The camera won't be usable, and > worse, generateConfiguration() and validate() will crash as those > functions assume that at least one format is supported. > > Fix this by failing match() if none of the formats exposed by the camera > are supported. Log an error message in that case to notify the user. This seems quite reasonble to me. I suspect it's worth waiting for some tested by tags from Christian before merging, but with that: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=145 > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index be0fbaea508a..a28195450634 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -457,6 +457,13 @@ int UVCCameraData::init(MediaDevice *media) > } > } > > + if (formats_.empty()) { > + LOG(UVC, Error) > + << "Camera " << id_ << " (" << media->model() > + << ") doesn't expose any supported format"; > + return -EINVAL; > + } > + > /* Populate the camera properties. */ > properties_.set(properties::Model, utils::toAscii(media->model())); > > -- > Regards, > > Laurent Pinchart >
On Tue, Sep 06, 2022 at 10:35:51AM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart via libcamera-devel (2022-09-03 19:10:37) > > A UVC device could expose only formats that are not supported by > > libcamera. The pipeline handler doesn't currently consider this as an > > error, and happily creates a camera. The camera won't be usable, and > > worse, generateConfiguration() and validate() will crash as those > > functions assume that at least one format is supported. > > > > Fix this by failing match() if none of the formats exposed by the camera > > are supported. Log an error message in that case to notify the user. > > This seems quite reasonble to me. I suspect it's worth waiting for some > tested by tags from Christian before merging, but with that: I would prefer that, although I've successfully tested the series with a camera that produces the V4L2 GREY format. Disabling that format in libcamera made the pipeline handler crash, this series fixed it. I'll wait one more day and merge. > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=145 > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > index be0fbaea508a..a28195450634 100644 > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > @@ -457,6 +457,13 @@ int UVCCameraData::init(MediaDevice *media) > > } > > } > > > > + if (formats_.empty()) { > > + LOG(UVC, Error) > > + << "Camera " << id_ << " (" << media->model() > > + << ") doesn't expose any supported format"; > > + return -EINVAL; > > + } > > + > > /* Populate the camera properties. */ > > properties_.set(properties::Model, utils::toAscii(media->model())); > >
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index be0fbaea508a..a28195450634 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -457,6 +457,13 @@ int UVCCameraData::init(MediaDevice *media) } } + if (formats_.empty()) { + LOG(UVC, Error) + << "Camera " << id_ << " (" << media->model() + << ") doesn't expose any supported format"; + return -EINVAL; + } + /* Populate the camera properties. */ properties_.set(properties::Model, utils::toAscii(media->model()));
A UVC device could expose only formats that are not supported by libcamera. The pipeline handler doesn't currently consider this as an error, and happily creates a camera. The camera won't be usable, and worse, generateConfiguration() and validate() will crash as those functions assume that at least one format is supported. Fix this by failing match() if none of the formats exposed by the camera are supported. Log an error message in that case to notify the user. Bug: https://bugs.libcamera.org/show_bug.cgi?id=145 Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 7 +++++++ 1 file changed, 7 insertions(+)