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

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

Hi Tomasz,

On Tue, Sep 03, 2019 at 01:16:21PM +0900, Tomasz Figa wrote:
> On Tue, Sep 3, 2019 at 1:43 AM Laurent Pinchart wrote:
> > 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 :-)
> 
> I don't think Android exposes camera rotation to the layers above the
> HAL. The stream is expected to be pre-rotated by the HAL, taking into
> account the desired target rotation of the stream itself [1].
> 
> [1] https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/camera3.h#1428
> 
> That said, Android seems to use "degrees counterclockwise" for rotations.

Counterclockwise seems better to me as that's coherent with
trigonometry. We still have to define the direction of the rotation axis
though :-)

-- 
Regards,

Laurent Pinchart