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

[Laurent Pinchart]

Hi Jacopo,

On Tue, Jan 07, 2020 at 09:39:31AM +0100, Jacopo Mondi wrote:
> On Mon, Jan 06, 2020 at 11:35:59PM +0200, Laurent Pinchart wrote:
> > 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.

We've discussed this offline and you've posted a new version, so I won't
reply here has it would be rather long, but please let me know if any
unanswered question still needs a reply.

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

For OEM devices shipping libcamera, that's not a problem. For other
devices where the end user will install libcamera (such as a laptop with
a regular Linux distribution), there will be no OEM writing
configuration files, and I don't think we can expect distributions to
assume this role either if the configuration has to be device-specific.

> 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