[libcamera-devel] [PATCH 3/3] pipeline: uvcvideo: Fail match() if the camera has no supported format

[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 at ideasonboard.com>
> 
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=145
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at 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