[libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID
Niklas Söderlund
niklas.soderlund at ragnatech.se
Fri Jul 24 12:12:45 CEST 2020
Hi Jacopo,
Thanks for your feedback.
On 2020-07-20 16:48:26 +0200, Jacopo Mondi wrote:
> Hi Niklas,
>
> On Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:
> > Hello,
> >
> > 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)
>
> Really an open question, are those name really useful ?
>
> I've spent some time going through systemd predictable interfaces
> names and udevadm test-builtin path_id and indeed they use the same
> information you have here collected, but they produce a name without /
> and with a well defined naming scheme (systemd at least) that could be
> guessed by knowing how the devices are integrated in the system.
>
> I don't think we need to go as far as making our camera names
> predictable, at least I don't see a urgent use case for that, but
> taking the raw sysfs path seems a bit too simplicistic, especially
> when you then have to deal with ids like:
> 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
>
> which seems a bit long to me.
>
> I know this is not expected to be used by users directly, but as per
> my previous comment I think we should not expose an API in libcamera
> that works on names which are not guranteed to be unique (althoug
> application should be able to provide shortcut, maybe using friendly
> names as reported by pipeline handlers.
>
> Trying to list which kind of sensor we should deal with, their number
> is fairly limited for my understanding: usb and i2c. Wouldn't coupling
> the bus identifier with the sensor identifier enough to provide a
> unique and almost-as-friendly-as-a-name id ?
>
> Using the use example you have above provided:
> - ipu3
> i2c-8_i2c-OVTID858:00
> i2c-10_i2c-INT3479:00
> - raspberrypi
> i2c-10_10-0010
> - rkisp1
> i2c-7_7-0036
> i2c-7_7-003c
> - uvcvideo
> usb3_3-2_3-2.4_3-2.4:1.0
> - vimc
> vimc.0
>
> I2c is trivial, use the device identifier and walk the path back until
> the i2c parent identifier is found, then couple the 2.
>
> usb seems a bit more tricky as a full name like
> usb3_3-2_3-2.4_3-2.4:1.0
> could probably be reduced to
> usb3_3-2.4:1.0
>
> (however, udevadm for my UVC camera gives me:
> udevadm test-builtin path_id /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.0
> ID_PATH_TAG=pci-0000_00_14_0-usb-0_8_1_0
> while I would have expected usb1_1-8:1.0
>
> I know nothing about USB, but it might be worth reading why udev
> thinks that path tag should in that form
> https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-path_id.c
>
> Vimc is kind of special, you cannot have 2 vimc instances in your
> system loaded at the same time, so I guess the device name should be
> enough.
>
> What options have you considered in place of the full sysfs path ? Or
> do you think there's no need to and a string like
> 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
> could be easily managed by applications ?
Thanks for pointing out the udev possibility to this problem, I was not
aware of it. I kind of like it and would like to use it instead of using
the sysfs device path as this series does.
The only problem I see is that it would make udev a required dependency.
In the past we have gone to great length of keeping udev optional, we
even implemented a sysfs based device enumerator to allow for this. If
we with what we know now are willing to reevaluate this design idea I
would be all for using udev as you describe above. So before I set out
to try this out how do we all fell about making udev mandatory?
>
> Thanks
> j
>
> >
> > 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(-)
> >
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list