[libcamera-devel] [PATCH v2 0/6] libcamera: Generate unique and stable camera names
Jacopo Mondi
jacopo at jmondi.org
Tue Jul 28 09:24:22 CEST 2020
Hi Niklas,
On Tue, Jul 28, 2020 at 02:30:52AM +0200, Niklas Söderlund wrote:
> Hello,
>
> This series aims to make and enforce unique camera names that are static
> between system resets while keeping them user-friendly and adding more
> information describing where the cameras are located in the system.
> This v2 is a complete rewrite of v1 (libcamera: camera: Add camera ID)
> of this series that centered around bus information instead of this v2
> that focus more on user friendly names.
>
> The weakness in this series is that not a lot of platforms describe the
> rather new location and rotation properties in their devicetree/ACPI.
> This leads to issues that can be observed below where both cameras on
> the ipu3 and rkisp1 platforms are reported to be located on the front
> while in reality one is located on the front and the other on the back.
> This is not a shortcoming of this series however and will solve itself
> once the platforms gets update firmware or when CameraSensor learns to
> read this information from configuration files (or by some other means).
>
> Patch 3/6 also needs more work to query the device enumerator for the
> sysfs path instead of building a path that assumes the standard naming
> schema. This is a implementation detail and I think it's more important
> to get speedy feedback on the over all approach of the series.
>
> Before this series camera 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
> Venus USB2.0 Camera: Venus USB2
> Logitech Webcam C930e
> - vimc
> VIMC Sensor B
>
> With this series applied camera names on the same systems:
>
> - ipu3
> ov13858 location front (PipelineHandlerIPU3)
> ov5670 location front (PipelineHandlerIPU3)
> - raspberrypi
> imx219 location front (PipelineHandlerRPi)
> - rkisp1
> ov5695 location front (PipelineHandlerRkISP1)
> ov2685 location front (PipelineHandlerRkISP1)
> - uvcvideo
> Venus USB2.0 Camera: Venus USB2 on bus 3:9 (PipelineHandlerUVC)
> Logitech Webcam C930e on bus 3:4 serial 9F8F445E (PipelineHandlerUVC)
> - vimc
> Sensor B location front (PipelineHandlerVimc)
I'm sorry if I sound negative, but I have comments on the general
naming scheme this series builds on. I'm sorry you went down to a full
implementation, discussing the names provided in that cover letter
would have been enough to get feedbacks imho.
That said, a few comments on the naming scheme
1) location: it is reported through camera properties, I don't see
much value having it in the name, maybe in the "friendly" one only,
but if I'm not mistaken this series makes the "friendly" name unique.
I expect most applications (most, a lot..) to select cameras based on
their location, and they will get it through the property, not by
parsing the name.
1.a) In a system like a RPi, where cameras are connected through flex
cables, cameras will be marked as 'external'. If you can connect two
sensors named "amazingsensor" you would get two identical camera names
'amazingsensor location external'
2) pipeline handler name: I can see UVC co-existing with other
pipeline handlers, but the UVC camera names won't collide with
built-in camera names, so I don't see much value in reporting the
pipeline handler that has created the camera I'm afraid.
If we need to convey this information, this should be a camera
property imho.
We've ruled out -a lot- of information we could use to generate camera
names: video device numbers are not stable, sysfs paths are not
because of that, the bus identifiers might be re-assigned between
reboots if behing a PCI bus... there's not much in the system we can
rely on to construct a stable and unique name if we consider all
systems to be equal. I however still think it should be useful to
define a set of use cases we support and for each of them identify if
there's some information we can use.
In example, on a generic DT-based embedded system it would be enough
to rely on
1) the media entity name
2) alphabetical order
Combine the two and you have an ordering
1 ov13858-2-0047
2 ov5670-2-0036
Works for system with the same sensor, as they could not live on the
same bus
1 ov5670-2-0036
2 ov5670-4-0036
For non-PCI system, the i2c bus number is stable, hence, the camera on
the first bus comes first.
For PCI, is there a PCI id or something -stable- we can rely on ? UVC
you have built the vid/pid identifier here, right ? that seems stable
to me but is it identical between two instances of the same device ?
I think we really need to sort out this question before going into
coding. That's why I have pointed you to udev, and systemd predictable
names, because even if in a different context they do what we should
aim to do: define a naming scheme, and I see there reported
information we might found useful about "PCI slot number" and "USB
port number chain". Knowing how they build those part could help us
solve this issue, but it needs to be researched first.
Thanks
j
>
> Niklas Söderlund (6):
> libcamera: camera: Append pipeline name to camera name
> libcamera: camera: Generate camera name from a CameraSensor
> libcamera: v4l2_device: Add method to lookup device path
> libcamera: media_device: Expose media device serial number
> libcamera: pipeline: uvcvideo: Generate unique camera names
> libcamera: camera_manager: Enforce unique camera names
>
> include/libcamera/camera.h | 5 +++
> include/libcamera/internal/media_device.h | 2 +
> include/libcamera/internal/v4l2_device.h | 1 +
> src/libcamera/camera.cpp | 42 ++++++++++++++++++-
> src/libcamera/camera_manager.cpp | 6 +--
> src/libcamera/media_device.cpp | 7 ++++
> src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++---
> .../pipeline/raspberrypi/raspberrypi.cpp | 3 +-
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> src/libcamera/pipeline/simple/simple.cpp | 2 +-
> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 41 +++++++++++++++++-
> src/libcamera/pipeline/vimc/vimc.cpp | 4 +-
> src/libcamera/v4l2_device.cpp | 24 +++++++++++
> 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 +-
> test/serialization/serialization_test.h | 2 +-
> 21 files changed, 142 insertions(+), 25 deletions(-)
>
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list