[libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: Infer camera location from removable attribute

[Laurent Pinchart]

On Thu, May 26, 2022 at 03:03:47PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2022-05-26 14:30:25)
> > Use the removable attribute exposed by the USB device in sysfs to infer
> > if the camera is an external (when removable) or front (when
> > non-removable) camera. This is likely not perfect as the removable
> > attribute is derived from ACPI data which may not always be accurate,
> > and non-ACPI platforms likely report all devices as removable. Still,
> > this is a first step upon which a better method could be built.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 33 +++++++++++++++++---
> >  1 file changed, 28 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > index 2ebf2788c3b4..53b2f23ab029 100644
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > @@ -501,13 +501,36 @@ int UVCCameraData::init(MediaDevice *media)
> >  
> >         video_->bufferReady.connect(this, &UVCCameraData::bufferReady);
> >  
> > -       /*
> > -        * \todo Find a way to tell internal and external UVC cameras apart.
> > -        * Until then, treat all UVC cameras as external.
> > -        */
> > -       properties_.set(properties::Location, properties::CameraLocationExternal);
> >         properties_.set(properties::Model, utils::toAscii(media->model()));
> >  
> > +       /*
> > +        * Derive the location from the device removable attribute in sysfs.
> > +        * Non-removable devices are assumed to be front as we lack detailed
> > +        * location information, and removable device are considered external.
> > +        *
> > +        * The sysfs removable attribute is derived from the ACPI _UPC attribute
> > +        * if available, or from the USB hub descriptors otherwise. ACPI data
> > +        * may not be very reliable, and the USB hub descriptors may not be
> > +        * accurate on DT-based platforms. A heuristic may need to be
> > +        * implemented later if too many devices end up being miscategorized.
> > +        *
> > +        * \todo Find a way to tell front and back devices apart. This could
> > +        * come from the ACPI _PLD, but that may be even more unreliable than
> > +        * the _UPC.
> > +        */
> > +       properties::LocationEnum location = properties::CameraLocationExternal;
> > +       std::ifstream file(video_->devicePath() + "/../removable");
> 
> I expected this devicePath() to be /dev/video0 ... but given the above,
> I presume it's mapping elsewhere. Is it guaranteed to map to the sysfs
> path? or do we need to build it with the information we have?

Quoting
https://libcamera.org/api-html/classlibcamera_1_1V4L2Device.html#a8454df564fffa7ad160468709aa172ff,

"This function returns the sysfs path to the physical device backing the
V4L2 device. The path is guaranteed to be an absolute path, without any
symbolic link."

So we're good :-)

> Anyway, if it fails, it will fail to what we have anyway so - if it
> works in some cases, that's still 'progress'...
> 
> With the path understood, (which I'm sure you do anyway...)
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > +       if (file.is_open()) {
> > +               std::string value;
> > +               std::getline(file, value);
> > +               file.close();
> > +
> > +               if (value == "fixed")
> > +                       location = properties::CameraLocationFront;
> > +       }
> > +
> > +       properties_.set(properties::Location, location);
> > +
> >         /*
> >          * Get the current format in order to initialize the sensor array
> >          * properties.
> > 
> > base-commit: f8e4649680303ce9b7dace5ad1344fcb450013ac
> > prerequisite-patch-id: edbf31ed9d7f5374e17b34936fe1aa8dea2eac44

-- 
Regards,

Laurent Pinchart