[libcamera-devel] [PATCH v3 03/10] libcamera: properties: Generate libcamera properties

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Jan 03, 2020 at 11:45:30AM +0100, Jacopo Mondi wrote:
> Hello,
>     following discussions on the 'rotation' property here defined, it
> seems we're struggling to find consensus on its definition.
> 
> What I had here come from the kernel definition, and poorly applies to
> a camera library control.
> 
> I'll here have another go, trying to list the possible use cases as
> suggested by Laurent, in the attempt to latr find a generic enough
> definition.
> 
> With this clarified, I'll re-send the series (almost fully reviewed)
> along with compound controls support on top.
> 
> On Mon, Dec 09, 2019 at 05:34:39PM +0100, Jacopo Mondi wrote:
> > Re-use the Control generation infrastructure to generate libcamera properties.
> >
> > Introduce three additional files:
> > - include/libcamera/property_ids.h
> >   Defines the properties ids
> >
> > - src/libcamera/property_ids.cpp
> >   Defines the properties Control<> instances
> >
> > - src/libcamera/property_ids.yaml
> >   Provide properties definitions
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> 
> [snip]
> 
> > +
> > +  - Rotation:
> > +      type: int32_t
> > +      description: |
> > +        Camera mounting rotation expressed as counterclockwise rotation degrees
> > +        towards the axis perpendicular to the sensor surface and directed away
> > +        from it
> 
> This comes straight from the DT property I am trying to push to kernel
> space, but for libcamera it might be more opportune to define rotation
> as the -correction- to apply to the captured images to obtain the best
> possible orientation for the device the camera module is installed on.

This is what the Android camera HAL API does, but I don't think we can
do the same. The big difference is that an Android camera HAL
implementation can hardcode information about the device and its
display, while we don't have that luxury in libcamera.

Android defines the android.sensor.orientation property as

"Clockwise angle through which the output image needs to be rotated to
be upright on the device screen in its native orientation."

For all we know, we may not even have a screen :-) We thus need to
define the rotation relatively to the device but without involving its
screen (or at least not in the general case).

> I think what we're missing here is a definition of best possible
> orientation that is generic and clear enough.
> 
> I'll try to list a few use cases
> - phone/tablet
>   Images should be presented to the user with their longer side
>   aligned to the device's longer side, to minimize resizing/cropping.
> - laptop (user facing camera)
>   Images should be presented with the longer side parallel to the
>   usage surface
> - sport/action cameras
>   The FOV should be maximed in its horizontal dimension

This really depends on the sport, some are more vertically oriented
(although if they have to be shown on TV, horizontal orientation is
usually preferred).

You're very focussed on the end-user market in that list. How about
surveillance cameras, face scanners in security gates, computer vision
cameras for industrial applications, and even linear cameras (where the
sensor has a single line and infinitely long images are created by
moving a conveyor belt in front of the sensor) ?)

> So it seems to me each use case has a different 'designated
> orientation' and the rotation correction really depends on the device
> intended usage. This rises the question, who sets this property ?
> We're moving this to a CameraSensor subclass, while it really seems to
> me this is -device- dependent (and yes, calls for a config file, I'm
> sorry).

I still fully disagree :-) In any case that would be completely
impractical as nobody will be able to maintain an up-to-date database of
such configuration files for every platform out there, and we need a
solution for end-users.

> So I would go with:
> "Rotation correction, expressed as counterclockwise rotation degrees
> to apply to captured images to align their display orientation with
> the device intended usage orientation. For mobile devices such as
> phones or tablets, images are usually displayed with their longer side
> aligned to the device longer edge."
> 
> How would that feel ?

Let me try and make a proposal (unless somebody wants to beat me to it
:-)).

> Please note also the DT property and the proposed v4l2 control would
> need some feedbacks, as ideally, the two series should go in more or
> less at the same time.

Agreed, I think we need to come up with an agreement for the whole chain
(DT, V4L2 and libcamera) before merging any code.

-- 
Regards,

Laurent Pinchart