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

[Jacopo Mondi]

Hi Laurent,

On Mon, Jan 06, 2020 at 11:35:59PM +0200, Laurent Pinchart wrote:
> 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 don't thin "best possible orientation" implies the usage of a
screen. At least, that's not what I meant with "

> > as the -correction- to apply to the captured images to obtain the best
> > possible orientation for the device the camera module is installed on.

>
> > 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) ?)

I wonder how can we possibly know that. Is the conveyor belt
horizontal ? Does it move vertically ? Are the pieces moved taller
than larger ? I'm still of the idea the best 'orientation' (which is
basically portrait vs landscape) depends on the use case.

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

Why a database ? As and OEM you have to provide a file for your
device, why there should be a shared database ?

Otherwise we defer the decision to applications, but that kind of
defeat the whole purpose of generalizing this definition.

> > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200107/ae5ba52d/attachment.sig>