[libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID

[Jacopo Mondi]

Hi Niklas,

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.

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...

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(-)
>
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel