Re: [PATCH v2 03/10] media: v4l2-ctrl: Document V4L2_CID_CAMERA_SENSOR_ROTATION

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Jacopo,

On Mon, Sep 02, 2019 at 01:20:49PM +0200, Jacopo Mondi wrote:
> On Tue, Aug 27, 2019 at 03:51:32PM +0300, Laurent Pinchart wrote:
> > On Tue, Aug 27, 2019 at 11:23:29AM +0200, Jacopo Mondi wrote:
> >> Add documentation for the V4L2_CID_CAMERA_SENSOR_ROTATION camera
> >> control. The newly added read-only control reports the camera device
> >> mounting rotation.
> >>
> >> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> >> ---
> >>  Documentation/media/uapi/v4l/ext-ctrls-camera.rst | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> >> index ecf151f3f0f4..03d1c23d18f7 100644
> >> --- a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> >> +++ b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> >> @@ -544,6 +544,15 @@ enum v4l2_scene_mode -
> >>
> >>
> >>
> >> +``V4L2_CID_CAMERA_SENSOR_ROTATION (integer)``
> >> +    This read-only control describes the camera sensor orientation by
> >> +    reporting its mounting rotation in respect to the device intended usage
> >> +    orientation, expressed in counter clockwise degrees. The control value is
> >
> > Counter clockwise relative to what ? :-)
> 
> I think it is expressed in the previous line:
> "mounting rotation in respect to the device intended usage
> orientation, expressed in counter clockwise degrees"
> 
> Would you prefer to have this re-phrased as:
> 
> This read-only control describes the camera sensor orientation
> expressed as rotation in counter clockwise degrees in respect to
> the device intended usage orientation.
> 
> ?

Rotation is expressed in degrees around an axis, which is itself
expressed as a vector. It's pretty intuitive that the rotation vector
should be perpendicular to the plane of the sensor (which itself should
be parallel to the plane of the device side on which it is mounted, as
expressed by the location property), but there are still two possible
directions for the vector, facing in the same direction as the sensor or
the opposite direction. Unless there's a good reason to do so, I would
use the same direction as the one defined by Android. I don't know what
direction that is though :-)

> >> +    constant and not modifiable by software and its value is retrieved from the
> >> +    firmware interface by parsing the 'rotation' property. Typical values are 0
> >> +    degrees for upright mounted sensors and 180 degrees for sensors mounted
> >> +    upside down.
> >
> > Same comment as for 02/10 regarding how the value is known to the
> > driver.
> >
> >> +
> >>  .. [#f1]
> >>     This control may be changed to a menu control in the future, if more
> >>     options are required.

-- 
Regards,

Laurent Pinchart