[libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jul 25 01:44:07 CEST 2020
Hello,
On Fri, Jul 24, 2020 at 12:08:35PM +0200, Niklas Söderlund wrote:
> On 2020-07-20 15:07:07 +0200, Jacopo Mondi wrote:
> > On Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:
> > > Hello,
> >
> > minors apart that I will reply to patch-by-patch, I have two questions
> > on the series in general
> >
> > 1) The series introduce and 'id' to be used alongside and
> > alternatively to the camera 'name'. This might just be a matter of
> > terminology, but I find this a bit confusing. Ideally, the 'name'
> > should be the unique part, to which a 'model' (to mimic the API we
> > have for camera sensors) could be added.
>
> I'm not against this change. But maybe removing the name() and adding a
> model() could be done on top as this series main goal is to add a id()
> field?
We're entering the bikeshedding territory a little bit, but I think it's
an important question. We may want to drop "name" altogether, as it's a
confusing name (no pun intended). Using friendlyName() and uniqueName(),
for instance, would make the API clearer. We can continue bikeshedding
on the name, but I think whatever function names we pick, they should be
fairly explicit.
> > 2) The API and the cam implementation allow to use 'name' and 'id'
> > interchangibily. Is this a good thing ? Applications should always use
> > 'id' when interfacing to libcamera, and ideally 'name' should be a
> > shortcut for users, to easily select a camera (provided it is unique).
> > If I'm not mistaken it is currently possible to ask libcamera for a
> > 'camera' name, should we drop this and implement that part in the
> > application ? 'cam' and alike can and should use mnemonic names to
> > users, but should libcamera do the same? Do we want an API that allows
> > selecting camera with a name which is not guaranteed to be unique and
> > consistent ? I would say we don't...
>
> Also I'm not against this change and I think if we all agree this series
> can be modified to only allow selecting cameras based on id() instead of
> id() or name() as this version of this series allows.
I'm with Jacopo on this, I'd prefer if libcamera pushed applications to
use a safer API, while still offering the option of an application-side
manual implementation based on the friendly name.
> > What do you think ?
> >
> > > This series aims to add a ID to each camera in addition to it's more
> > > user-friendly name. The ID is unique and persistent between reboots of
> > > the same system. The use-case for this is to create a single
> > > machine-friendly ID that can be stored and used to always resolve to the
> > > same camera.
> > >
> > > The idea on how to generate a ID is to take the sysfs path of the sensor
> > > device which is part of each camera pipeline. As the path describes the
> > > location of the sensor hardware it is persistent across reboots and as
> > > the path is read from sysfs it's guaranteed to be unique in the system.
> > >
> > > For pipelines that do not have a sensor (UVC) the sysfs path of the main
> > > video device is used instead. That path resolves to the USB device and
> > > includes the USB bus information so it satisfy the ID requirements.
> > >
> > > While working with this problem it became apparent that two pipelines
> > > diverge from the others on how they name their cameras, raspberrypi and
> > > vimc. This series aligns these two and adds a helper to avoid such
> > > situations in the future. Unfortunately this means the user-friendly
> > > name of the sensor changes but this proves the need for a
> > > machine-friendly ID which luckily this series also adds :-)
> > >
> > > Before this series camera user-friendly names on different systems
> > > looked like this (I do not have access to a simple pipeline device):
> > >
> > > - ipu3
> > > ov13858 8-0010
> > > ov5670 10-0036
> > > - raspberrypi
> > > imx219
> > > - rkisp1
> > > ov5695 7-0036
> > > ov2685 7-003c
> > > - uvcvideo
> > > Logitech Webcam C930e
> > > - vimc
> > > VIMC Sensor B
> > >
> > > With this series applied the user-friendly names machine-friendly ID on
> > > the same systems look like this:
> > >
> > > The format is:
> > > <user-friendly name> (<machine-friendly ID>)
> > >
> > > - ipu3
> > > ov13858 8-0010 (pci0000:00/0000:00:15.2/i2c_designware.2/i2c-8/i2c-OVTID858:00)
> > > ov5670 10-0036 (pci0000:00/0000:00:19.2/i2c_designware.5/i2c-10/i2c-INT3479:00)
> > > - raspberrypi
> > > imx219 10-0010 (platform/soc/3f205000.i2c/i2c-11/i2c-10/10-0010)
> > > - rkisp1
> > > ov5695 7-0036 (platform/ff160000.i2c/i2c-7/7-0036)
> > > ov2685 7-003c (platform/ff160000.i2c/i2c-7/7-003c)
> > > - uvcvideo
> > > Logitech Webcam C930e (pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0)
> > > - vimc
> > > Sensor B (platform/vimc.0)
> > >
> > > Where it previously where possible to select a camera by its
> > > user-friendly name its now possible to also select it using its
> > > machine-friendly one. The following is therefor two equivalent
> > > commands:
> > >
> > > $ cam -c "Logitech Webcam C930e" -C
> > > $ cam -c "pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0" -C
> > >
> > > Niklas Söderlund (9):
> > > libcamera: v4l2_device: Add method to lookup device path
> > > libcamera: camera_sensor: Expose a sensor ID
> > > libcamera: camera: Add camera ID
> > > libcamera: camera_manager: Enforce unique camera IDs
> > > libcamera: camera_manager: Try to match camera IDs first
> > > libcamera: pipeline: vimc: Align camera name
> > > libcamera: pipeline: raspberrypi: Align camera name
> > > libcamera: camera: Add create() that operates on CameraSensor
> > > cam: Print camera IDs when listing cameras
> > >
> > > include/libcamera/camera.h | 11 +++-
> > > include/libcamera/internal/camera_sensor.h | 2 +
> > > include/libcamera/internal/v4l2_device.h | 1 +
> > > src/cam/main.cpp | 3 +-
> > > src/libcamera/camera.cpp | 54 +++++++++++++++----
> > > src/libcamera/camera_manager.cpp | 13 +++++
> > > src/libcamera/camera_sensor.cpp | 17 ++++++
> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 ++--
> > > .../pipeline/raspberrypi/raspberrypi.cpp | 3 +-
> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> > > src/libcamera/pipeline/simple/simple.cpp | 3 +-
> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 7 ++-
> > > src/libcamera/pipeline/vimc/vimc.cpp | 4 +-
> > > src/libcamera/v4l2_device.cpp | 27 ++++++++++
> > > test/camera/buffer_import.cpp | 2 +-
> > > test/camera/capture.cpp | 2 +-
> > > test/camera/configuration_default.cpp | 2 +-
> > > test/camera/configuration_set.cpp | 2 +-
> > > test/camera/statemachine.cpp | 2 +-
> > > test/controls/control_info_map.cpp | 2 +-
> > > test/controls/control_list.cpp | 2 +-
> > > 21 files changed, 138 insertions(+), 32 deletions(-)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list