Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Jacopo Mondi <jacopo@jmondi.org>
To: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	tfiga@google.com, pavel@ucw.cz,
	"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)" 
	<linux-media@vger.kernel.org>
Subject: Re: [PATCH v3 04/11] media: v4l2-ctrl: Document V4L2_CID_CAMERA_SENSOR_ROTATION
Date: Fri, 13 Sep 2019 20:49:06 +0200
Message-ID: <20190913184906.6tpl374n4anzja5c@uno.localdomain> (raw)
In-Reply-To: <549569c7-64bd-f6bd-30f6-e0fe27687780@xs4all.nl>

[-- Attachment #1: Type: text/plain, Size: 7688 bytes --]

Hi Hans,

On Fri, Sep 13, 2019 at 04:02:45PM +0200, Hans Verkuil wrote:
> On 9/12/19 10:10 PM, 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>
> > ---
> >  .../media/uapi/v4l/ext-ctrls-camera.rst       | 117 ++++++++++++++++++
> >  1 file changed, 117 insertions(+)
> >
> > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> > index f879dcc9409c..74991522ca3a 100644
> > --- a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> > +++ b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> > @@ -542,6 +542,123 @@ enum v4l2_scene_mode -
> >
> >
> >
> > +``V4L2_CID_CAMERA_SENSOR_ROTATION (integer)``
> > +    This read-only control describes the sensor orientation expressed as
> > +    rotation in counterclockwise degrees along the axis perpendicular to the
> > +    device mounting plane, and directed away from the sensor lens. Possible
> > +    values for the control are 90, 180 and 270 degrees. To compensate the device
>
> compensate -> compensate for
>
> > +    mounting rotation on the captured images, a rotation of the same amount of
> > +    degrees, in the same counterclockwise rotation direction should be applied
> > +    along the axis directed from the observer to the captured image when
> > +    displayed on a screen.
>
> Is this right? Shouldn't that be "in the clockwise direction"? If the sensor is
> mounted 90 degrees counterclockwise, then I need to rotate by 90 degrees clockwise
> to compensate for that, right?
>

It really depend along which axis direction you are applying the mounting
rotation and the compensation rotation... See below...

> > +
> > +    To better understand the effect of the sensor rotation on the acquired
> > +    images when displayed on a screen, it is helpful to consider a fictional
> > +    scan-out sequence of the sensor's pixels, assuming the pixel array having
> > +    its top-left pixel at position (0, 0) with values on the 'x' axis increasing
> > +    towards the right direction, and values on the 'y' axis increasing towards
> > +    the bottom. The effect of sensor rotation could be easily visualized
> > +    considering the sequence of captured pixels.
> > +
> > +    Assuming the following scene has to be captured::
> > +
> > +                o
> > +               -|-
> > +               / \
> > +
> > +    An upright mounted sensor has its pixel array displaced as follow::
> > +
> > +                                      x
> > +            (0,0)---------------------->
> > +              ! 0,0 0,1 0,2 ... 0,line-len
>
> Isn't that 0,0 ... 0,num-col?

Yes indeed sorry

> line-len is a weird name, shouldn't that be num-lines?
>
> line-len sounds like it is the same as num-col.
>
> I'm totally confused.
>

num-col is totally wrong, that should have been num-lines

In general
s/line-len/num-col
s/num-col/num-lines

> > +              ! 1,0 1,1 1,2 ...
> > +              ! ...
> > +              ! ...
> > +              ! (num-col,0)...  (num-col,line-len)
> > +            y V
> > +
> > +
> > +    Assuming pixels are scanned out from (0,0) to (num-col,line-len)
> > +    progressively::
> > +
> > +             (0,0) ---->-------------> (0,line-len)---!
> > +             !------------------------------------<a--!
> > +             V
> > +             (1,0) ---->-------------> (1,line-len)---!
> > +             !------------------------------------<---!
> > +             V
> > +             (...) .-->--------------> ( ,,,, )    ---!
> > +             !------------------------------------<---!
> > +             V
> > +             (num-col,0)------------->(num-col,line-len)
> > +
> > +
> > +    If a rotation of 90 degrees counterclockwise along the axis perpendicular to
> > +    the sensor's lens and directed towards the scene to be captured is applied
> > +    to the sensor, the pixel array would then be rotated as follows::
> > +
> > +            x ^  0,line-len,,,(num-col,line-len
> > +              !  ....
> > +              !  0,2 1,2      ...
> > +              !  0,1 1,1      ...
> > +              !  0,0 1,0 ... num-col,0
> > +             (0,0)------------------------>
> > +                                   y
> > +
> > +    And the pixel scan-out sequence would then proceed as follows::
> > +
> > +            (0,line-len)            (num-cols,line-len)
> > +                 ^\    ^\    ^\    ^\    ^
> > +                 ! \   ! \   ! \   ! \   !
> > +                 !  \  !  \  !  \  !  \  !
> > +                 !   \ !   \ !   \ !   \ !
> > +                 !    \!    \!    \!    \!
> > +               (0,0)  (1,0) ....      (num-cols,0)
> > +
> > +    Which when applied to the capture scene gives::
> > +
> > +            (0,line-len)            (num-cols,line-len)
> > +                ^\    ^\    ^\    ^\    ^
> > +                ! \   ! \   0 \   ! \   !
> > +                !  \  !  \ -|- \  !  \  !
> > +                !   \ !    / \  \ !   \ !
> > +                !    \!    \!    \!    \!
> > +              (0,0)  (1,0) ....      (num-cols,0)
> > +
> > +    Producing the following image once captured to memory and
> > +    displayed to the user::
> > +
> > +             \ !
> > +               --0
> > +             / !
> > +
> > +    Which has a rotation of the same amount of degrees applied on the opposite
> > +    rotation direction along the axis that goes from the observer to the
> > +    displayed image.
> > +
> > +    In order to compensate the sensor mounting rotation, when expressed
> > +    as counterclockwise rotation along the axis directed from the sensor to
> > +    the captured scene, a rotation of the same amount of degrees in the
> > +    same counterclockwise rotation direction but applied along the axis
> > +    directed from the observer to the captured image, has to be applied.::
>
> .:: -> :
>

Don't I need the :: to mark the following block of text as verbatim ?

> > +
> > +                -------   90 degree counterclockwise
> > +                |   o  |  mounting rotation applied
> > +                |  -|- |  along the axis directed
> > +                |  / \ |  away from the sensor lens
> > +                -------
> > +                -------
> > +                | \ !  |  Resulting captured
> > +                |  --0 |  image when displayed
> > +                | / !  |  on screen
> > +                -------
>
> Trying this with my webcam turning it 90 degrees counterclockwise, I
> and up with my head to the left, not to the right.
>

Along which axis direction are you rotating the camera counterclockwise ?

If you see your face, and you rotate the camera counterclockwise while
looking at it, you're actually rotating along the axis directed -towards-
the sensor.

The rotation here in the example and in the 'rotation' property
description has to be applied along the axis pointing aways from the
sensor, so what you're actually doing is rotating clockwise along that
direction (I guess)... So yes, to compensate that, you need to rotate
clockwise when you look at the image on the screen... Confusing,
right?

> > +                -------
> > +                |   o  |  Rotation compensation
> > +                |  -|- |  is 90 degrees counterclockwise
> > +                |  / \ |  along the axis directed to the
> > +                -------   displayed image
> > +
> > +
> >  .. [#f1]
> >     This control may be changed to a menu control in the future, if more
> >     options are required.
> >
>
> Regards,
>
> 	Hans

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-12 20:10 [PATCH v3 00/11] media: Report camera sensor properties Jacopo Mondi
2019-09-12 20:10 ` [PATCH v3 01/11] dt-bindings: video-interfaces: Document 'location' property Jacopo Mondi
2019-09-13 13:45   ` Hans Verkuil
2019-09-12 20:10 ` [PATCH v3 02/11] media: v4l2-ctrl: Document V4L2_CID_CAMERA_SENSOR_LOCATION Jacopo Mondi
2019-09-13 13:48   ` Hans Verkuil
2019-09-12 20:10 ` [PATCH v3 03/11] dt-bindings: video-interfaces: Expand rotation description Jacopo Mondi
2019-09-13 13:50   ` Hans Verkuil
2019-09-12 20:10 ` [PATCH v3 04/11] media: v4l2-ctrl: Document V4L2_CID_CAMERA_SENSOR_ROTATION Jacopo Mondi
2019-09-13 14:02   ` Hans Verkuil
2019-09-13 18:49     ` Jacopo Mondi [this message]
2019-09-12 20:10 ` [PATCH v3 05/11] media: v4l2-ctrls: Add camera location and rotation Jacopo Mondi
2019-09-12 20:10 ` [PATCH v3 06/11] media: v4l2-fwnode: Add helper to parse device properties Jacopo Mondi
2019-09-13 14:08   ` Hans Verkuil
2019-09-13 19:04     ` Jacopo Mondi
2019-09-12 20:10 ` [PATCH v3 07/11] include: v4l2-ctrl: Sort forward declarations Jacopo Mondi
2019-09-12 20:10 ` [PATCH v3 08/11] media: v4l2-ctrls: Sort includes alphabetically Jacopo Mondi
2019-09-12 20:10 ` [PATCH v3 09/11] media: v4l2-ctrls: Add helper to register properties Jacopo Mondi
2019-09-13 14:25   ` Hans Verkuil
2019-09-12 20:10 ` [PATCH v3 10/11] media: i2c: ov5670: Parse and " Jacopo Mondi
2019-09-12 20:10 ` [PATCH v3 11/11] media: i2c: ov13858: " Jacopo Mondi

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190913184906.6tpl374n4anzja5c@uno.localdomain \
    --to=jacopo@jmondi.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=pavel@ucw.cz \
    --cc=sakari.ailus@linux.intel.com \
    --cc=tfiga@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox