[libcamera-devel] [PATCH v5 0/7] libcamera: Allow for user-friendly names in applications

[Tomasz Figa]

On Sat, Sep 26, 2020 at 3:47 PM Niklas Söderlund
<niklas.soderlund at ragnatech.se> wrote:
>
> Hi Tomasz,
>
> Thanks for your feedback.
>
> On 2020-09-26 14:24:20 +0200, Tomasz Figa wrote:
> > Hi Niklas,
> >
> > On Fri, Sep 25, 2020 at 5:08 PM Niklas Söderlund
> > <niklas.soderlund at ragnatech.se> wrote:
> > >
> > > Hello,
> > >
> > > This series trues to advance the topic of user-friendly names in
> > > applications.
> > >
> > > Patch 1/7 adds a new camera property 'Model' that may be used to record
> > > a cameras model. Patch 2/7, 3/7, 4/7 and 5/7 then implements this new
> > > property for all pipeline handlers.
> > >
> > > Patch 6/7 introduce user-friendly names to the cam utility.
> > >
> > > The final patch 7/7 records the model information in the DNG files
> > > created by qcam. More work is needed on top of this series to create
> > > user-friendly names for qcam. My intention is to do this in a follow up
> > > series once this is merged as I fear some bikeshedding on the style of
> > > the user-friendly name so lets battle that out in cam first :-)
> > >
> > > Example of listing cameras with and without this series,
> > >
> > > Without:
> > >     $ cam -l
> > >     Available cameras:
> > >     1: \_SB_.PCI0.RP05.PXSX-2.4:1.0-046d:0843
> > >     2: platform/vimc.0 Sensor B
> > >
> > >     $ cam -l
> > >     Available cameras:
> > >     1: /base/i2c at ff160000/camera at 36
> > >     2: /base/i2c at ff160000/camera at 3c
> > >
> > >     $ cam -l
> > >     Available cameras:
> > >     1: /base/soc/i2c0mux/i2c at 1/imx219 at 10
> > >
> > >     $ cam -l
> > >     Available cameras:
> > >     1: \_SB_.PCI0.I2C2.CAM0
> > >     2: \_SB_.PCI0.I2C4.CAM1
> > >
> > >
> > > With:
> > >     $ cam -l
> > >     Available cameras:
> > >     1: External camera Logitech Webcam C930e (\_SB_.PCI0.RP05.PXSX-2.4:1.0-046d:0843)
> > >     2: Internal front camera Sensor B (platform/vimc.0 Sensor B)
> > >
> > >     $ cam -l
> > >     Available cameras:
> > >     1: Internal front camera ov5695 (/base/i2c at ff160000/camera at 36)
> > >     2: Internal front camera ov2685 (/base/i2c at ff160000/camera at 3c)
> >
> > First of all, thanks for working on this. The new names are indeed
> > much more friendly. I still wonder, though, if for the user the exact
> > sensor model of an internal camera has any meaning. Perhaps something
> > like "Internal 5 megapixel front camera" and "Internal 2 megapixel
> > front camera" would be even friendlier? What do you think?
>
> I agree with you that user-friendly names can be enhanced even further
> to make them more useful for humans. On the devices I have access to the
> location property is not set in firmware (yet), and I expect this to be
> the case for most firmware as the property is a rather new. Until we
> have that sorted and common place in most firmware I think the sensor
> model adds value. But as you point out going forward a true location
> brings much more value.
>
> Do you think keeping the sensor model in the name for now is a good idea
> or should we drop it and try to add pressure to get the location
> reporting correct?
>

How about the sensor resolutions, as in the examples I listed? I think
it's really hard to tell which camera is which for the user by looking
at the sensor model, but in most of the cases, the resolutions are
very well specified and understood.

Of course the location is critical too, but it doesn't solve the
problem fully either - we may have multiple sensors at the same
location. Actually, one could imagine something like "Internal front
wide-angle camera" and "Internal front telephoto camera". Maybe these
user-friendly names should be configurable somehow?

> >
> > Best regards,
> > Tomasz
> >
> > >
> > >     $ cam -l
> > >     Available cameras:
> > >     1: Internal front camera imx219 (/base/soc/i2c0mux/i2c at 1/imx219 at 10)
> > >
> > >     $ cam -l
> > >     Available cameras:
> > >     1: Internal front camera ov13858 (\_SB_.PCI0.I2C2.CAM0)
> > >     2: Internal front camera ov5670 (\_SB_.PCI0.I2C4.CAM1)
> > >
> > > It can be observed above that all Cameras that report the Location
> > > property do so by stating they face the front. We know this is not true
> > > for some devices. This is however not a fault of this series as it only
> > > prints what is reported by the Camera. Once we teach the Camera to
> > > report true values for these properties cam will print the correct
> > > information.
> > >
> > > Niklas Söderlund (7):
> > >   libcamera: properties: Add model property
> > >   libcamera: utils: Add method to remove non-ASCII characters
> > >   libcamera: camera_sensor: Set sensor model property
> > >   libcamera: pipeline: uvcvideo: Initialize CameraData from MediaDevice
> > >   libcamera: pipeline: uvcvideo: Set sensor model property
> > >   cam: Print user-friendly camera names
> > >   qcam: dng_writer: Record camera model
> > >
> > >  include/libcamera/internal/utils.h           |  2 ++
> > >  src/cam/main.cpp                             | 31 +++++++++++++++++++-
> > >  src/libcamera/camera_sensor.cpp              |  2 ++
> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 31 ++++++++++----------
> > >  src/libcamera/property_ids.yaml              | 20 +++++++++++++
> > >  src/libcamera/utils.cpp                      | 17 +++++++++++
> > >  src/qcam/dng_writer.cpp                      | 13 ++++++--
> > >  7 files changed, 97 insertions(+), 19 deletions(-)
> > >
> > > --
> > > 2.28.0
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel at lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund