[libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jul 25 02:03:56 CEST 2020
Hello,
On Fri, Jul 24, 2020 at 12:12:45PM +0200, Niklas Söderlund wrote:
> On 2020-07-20 16:48:26 +0200, Jacopo Mondi wrote:
> > 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)
The friendly name should really be "Front camera" and "Back camera" on a
Soraka device. Same on a Scarlet device, but on a different rkisp1-based
system, there may not be a front and a back camera. Imagine a digital
microscope, it would be nice to have a friendly name along the lines of
"Microscope camera". How to make this happen will likely be challenging,
and I'm fine solving the friendly name issue separately from the unique
name issue (the former being more complex than the latter), but I'd like
to start thinking about it.
> > > - 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)
And if we have two identical USB webcams, it would be nice to name them
"Logitech Webcam C930e (on port 1)" and "Logitech Webcam C930e (on port
3)" for example.
> > > - vimc
> > > Sensor B (platform/vimc.0)
This one is easy, it's for testing purpose, so we can hardcode any name
we find friendly.
> >
> > 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.
How can is be too long and too simplistic at the same time ? :-)
This name should never be presented to the end-user (except by test
applications, when the end-user is a developer), so I don't mind much if
it's long. That being said, if there's a way to shorten it while still
keeping the name stable, I think that's better.
> > 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
Should we take into account the possibility of later implementing
support for sensor A, we will have two cameras for the same vimc device.
I think this should already be taken into account.
Let's try to also consider other cases, even if we don't implement
support for them now. One use case I can think of is cameras made of
multiple sensors (for instance a sensor with a wide angle lens and a
sensor with a tele lens, with seamless switching between the two when
zooming). This should however not be too complicated, we can probably
just pick one of the two sensors to construct the camera name.
Brainstorming mode turned on, what other use cases can we have ?
> > I2c is trivial, use the device identifier and walk the path back until
> > the i2c parent identifier is found, then couple the 2.
Is the I2C bus number stable ? On DT system, for SoC I2C controllers, it
should be *if* you have I2C bus aliases in your device tree, but for
anything behind a PCI device for instance, I don't think we have any
such guarantee.
> > 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
It may also be worth looking at how device names are constructed in the
kernel (usb_make_path() for instance). The USB devices are usually
identified by walking the USB tree from the root hub, keeping in mind
there can be multiple root hubs, and using port numbers along the way as
those are stable. The reason why the PCI device is included may be
because USB bus numbers are assigned dynamically, and thus may not be
stable across reboots.
> > 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?
I'd rather keep it optional if possible. As stated in a separate reply,
if we make the device enumerator responsible for providing the name, we
can have different implementations for udev and udev-less systems, so I
don't see any issue in relying on udev when available.
> > > 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