[libcamera-devel] [PATCH v5 0/7] libcamera: Allow for user-friendly names in applications
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Sep 28 19:57:07 CEST 2020
On Mon, Sep 28, 2020 at 07:36:56PM +0200, Tomasz Figa wrote:
> On Mon, Sep 28, 2020 at 7:20 PM Laurent Pinchart wrote:
> > On Sat, Sep 26, 2020 at 03:53:26PM +0200, Tomasz Figa wrote:
> > > On Sat, Sep 26, 2020 at 3:47 PM Niklas Söderlund wrote:
> > > > On 2020-09-26 14:24:20 +0200, Tomasz Figa wrote:
> > > > > On Fri, Sep 25, 2020 at 5:08 PM Niklas Söderlund 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?
> >
> > The design idea here (and it should really be documented somewhere, like
> > in the Camera class documentation) is that libcamera should provide
> > enough information to applications to construct meaningful names for
> > end-users, with the option to localize them. This patch series
> > demonstrates how this can be done in cam and qcam. The exact scheme to
> > construct camera names is left to applications (or frameworks), as
> > different applications have different types of end-users.
> >
> > The examples you gave above, with wide-angle and telephoto are very
> > good, and that's what I would expect the camera service in a phone or
> > tablet to create. libcamera needs to provide the location of the camera
> > and its resolution (we do so already, although it's not plumbed to the
> > firmware on all devices we support). Lens information is also needed,
> > and that's currently missing. We will add it, but I don't think it
> > should be a blocker for this series :-)
>
> Ah, okay, somehow I was convinced that it was an already formatted
> name returned by libcamera. Yes, if we could expose the information
> about the max. resolution and lens type, we should be able to
> construct really helpful names from the user point of view.
Another hint that we need to document this better :-)
> > Now the real question is what cam should output. cam is both a sample
> > application (it's thus important to showcase how to create a camera
> > name), but also a developer-oriented swiss army knife tool for
> > libcamera. Its expected audience is thus developers, and the sensor
> > names can thus be more relevant than for end-users of a phone. Maybe we
> > could go for something like "Internal front 5MP camera (ov5693)", to
> > both showcase how to create a camera name from different components, but
> > also keep the sensor name given the tool's audience ?
>
> Since cam is a simple application, perhaps only for testing purposes,
> I guess what Niklas already implemented could be good enough?
I think adding the resolution would be a good exercise to showcase
camera name creation, but not a blocker for this series.
> > > > > >
> > > > > > $ 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(-)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list