[libcamera-devel,04/23] libcamera: properties: Add rotation property

Message ID 20200113164245.52535-5-jacopo@jmondi.org
State Accepted
Headers show
Series
  • Properties and compound controls
Related show

Commit Message

Jacopo Mondi Jan. 13, 2020, 4:42 p.m. UTC
The rotation property describes the rotation of the camera sensor.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++
 1 file changed, 309 insertions(+)

Comments

Niklas Söderlund Jan. 14, 2020, 11:16 p.m. UTC | #1
Hi Jacopo,

Thanks for a great effort in writing this up!

On 2020-01-13 17:42:26 +0100, Jacopo Mondi wrote:
> The rotation property describes the rotation of the camera sensor.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

This matches my view of our discussions, nice work!

My only concern is that a shark is swimming in front of the laptop 
webcam example ;-)

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++
>  1 file changed, 309 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index aaadcbd3e52b..243af7bd0a03 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -25,4 +25,313 @@ controls:
>            description: |
>              The camera is attached to the device in a way that allows it to
>              be moved freely
> +
> +  - Rotation:
> +      type: int32_t
> +      description: |
> +        The camera rotation is expressed as the angular difference in degrees
> +        between two reference systems, one implicitly defined by the camera
> +        module intrinsics characteristics, and one artificially defined on the
> +        external world scene to be captured when projected on the image sensor
> +        pixel array.
> +
> +        A camera sensor has an implicitly defined 2-dimensional reference
> +        system 'Rc' defined by its pixel array scan-out order, with its origin
> +        posed at pixel address (0,0), the x-axis progressing from there towards
> +        the last scanned out column of the pixel array and the y-axis
> +        progressing towards the last scanned out line.
> +
> +        A typical example for a sensor with a (2592x1944) pixel array matrix
> +        observed from the front is
> +
> +                   (2592)      x-axis          0
> +                      <------------------------+ 0
> +                      .......... ... ..........!
> +                      .......... ... ..........! y-axis
> +                                 ...           !
> +                      .......... ... ..........!
> +                      .......... ... ..........! (1944)
> +                                               V
> +
> +        The external world scene reference system scene 'Rs' is defined as a
> +        2-dimensional reference system on the parallel plane posed in front
> +        of the camera module's focal plane, with its origin placed on the
> +        visible top-left corner, the x-axis progressing towards the right from
> +        there and the y-axis progressing towards the bottom of the visible
> +        scene.
> +
> +        A typical example of a (very common) picture of a shark swimming from
> +        left to right is
> +
> +                                 x-axis
> +                   (0,0)---------------------->
> +                     !
> +                     !
> +                     !       |\____)\___
> +                     !       ) _____  __`<
> +                     !       |/     )/
> +                     !
> +                     V
> +                   y-axis
> +
> +        With the reference plane posed in front of the camera module and
> +        parallel to its focal plane
> +
> +                                   !
> +                                 / !
> +                                /  !
> +                               /   !
> +                        _     /    !
> +                     +-/ \-+ /     !
> +                     | (o) |       ! 'Rs' reference plane
> +                     +-----+ \     !
> +                              \    !
> +                               \   !
> +                                \  !
> +                                 \ !
> +                                   !
> +
> +        When projected on the sensor's pixel array, the image and the associated
> +        reference system 'Rs' are typically inverted, due to the camera module's
> +        lens optical inversion effect.
> +
> +        Assuming the above represented scene of the swimming shark, the lens
> +        inversion projects on the sensor pixel array the reference plane 'Rp'
> +
> +                  y-axis
> +                     ^
> +                     !
> +                     !       |\_____)\__
> +                     !       ) ____  ___.<
> +                     !       |/    )/
> +                     !
> +                     !
> +                   (0,0)---------------------->
> +                                 x-axis
> +
> +        The camera rotation property is then defined as the angular difference
> +        in counterclockwise direction between the origin of the camera reference
> +        system 'Rc', defined by the camera sensor scan-out direction and its
> +        mounting position, and the origin of the projected scene reference
> +        system 'Rp', result of the optical projection of the scene reference
> +        system 'Rs' on the sensor pixel array.
> +
> +        Examples
> +
> +        0 degrees camera rotation
> +
> +                          y-Rp
> +                    y-Rc   ^
> +                     ^     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !   (0,0)---------------------->
> +                     !                 x-Rp
> +                   0 +------------------------------------->
> +                      0            x-Rc
> +
> +
> +                                     x-Rc          0
> +                           <------------------------+ 0
> +                                x-Rp                !
> +                     <-----------------------(0,0)  !
> +                                               !    !
> +                                               !    !
> +                                               !    !
> +                                               !    V
> +                                               !  y-Rc
> +                                               V
> +                                             y-Rp
> +
> +        90 degrees camera rotation
> +
> +                      0        y-Rc
> +                   0 +----------------------->
> +                     !
> +                     !    y-Rp
> +                     !     ^
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !   (0,0)---------------------->
> +                     !                 x-Rp
> +                     !
> +                     V
> +                   x-Rc
> +
> +        180 degrees camera rotation
> +
> +                                   x-Cr          0
> +                         <------------------------+ 0
> +                     y-Rp                         !
> +                      ^                           !
> +                      !                           ! y-Cr
> +                      !                           !
> +                      !                           !
> +                      !                           V
> +                      !
> +                      !
> +                    (0,0)--------------------->
> +                                   x-Rp
> +
> +        270 degrees camera rotation
> +
> +                      0        y-Rc
> +                   0 +----------------------->
> +                     !            x-Rp
> +                     ! <-----------------------(0,0
> +                     !                           !
> +                     !                           !
> +                     !                           !
> +                     !                           !
> +                     !                           !
> +                     !                           V
> +                     !                         y-Rp
> +                     !
> +                     !
> +                     V
> +                   x-Rc
> +
> +
> +
> +        Example one - Webcam
> +
> +        A camera module installed on the user facing part of a laptop screen
> +        casing used for video calls. The captured images are meant to be
> +        displayed in landscape mode (width > height) on the laptop screen.
> +
> +        The camera is typically mounted 180 degrees rotated to compensate the
> +        lens optical inversion effect.
> +
> +                          y-Rp
> +                    y-Rc   ^
> +                     ^     !
> +                     !     !       |\_____)\__
> +                     !     !       ) ____  ___.<
> +                     !     !       |/    )/
> +                     !     !
> +                     !     !
> +                     !   (0,0)---------------------->
> +                     !                 x-Rp
> +                   0 +------------------------------------->
> +                      0                x-Rc
> +
> +        The two reference systems are aligned, the resulting camera rotation is
> +        0 degrees, no rotation correction should be applied to the resulting
> +        image once captured to memory buffers to correctly display it to users.
> +
> +                     +--------------------------+
> +                     !       |\____)\___        !
> +                     !       ) _____  __`<      !
> +                     !       |/     )/          !
> +                     +--------------------------+
> +
> +        If the camera module is not mounted 180 degrees rotated to compensate
> +        the lens optical inversion, the two reference system will result not
> +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'
> +        plane.
> +
> +                                   x-Rc          0
> +                         <------------------------+ 0
> +                     y-Rp                         !
> +                      ^                           !
> +                      !                           ! y-Rc
> +                      !      |\_____)\__          !
> +                      !      ) ____  ___.<        !
> +                      !      |/    )/             V
> +                      !
> +                      !
> +                    (0,0)--------------------->
> +                                   x-Rp
> +
> +        The image once captured to memory will then be 180 degrees rotated
> +
> +                     +--------------------------+
> +                     !          __/(_____/|     !
> +                     !        >.___  ____ (     !
> +                     !             \(    \|     !
> +                     +--------------------------+
> +
> +        A software rotation correction of 180 degrees should be applied to
> +        correctly display the image.
> +
> +                     +--------------------------+
> +                     !       |\____)\___        !
> +                     !       ) _____  __`<      !
> +                     !       |/     )/          !
> +                     +--------------------------+
> +
> +        Example two - Phone camera
> +
> +        A camera installed on the back-side of a mobile device facing away from
> +        the user. The captured images are meant to be displayed in portrait mode
> +        (height > width) to match the device screen orientation and the device
> +        usage orientation used when taking the picture.
> +
> +        The camera is typically mounted with its pixel array longer side aligned
> +        to the device longer side, 180 degrees rotated to compensate the lens
> +        optical inversion effect.
> +
> +                      0        y-Rc
> +                   0 +----------------------->
> +                     !
> +                     !    y-Rp
> +                     !     ^
> +                     !     !
> +                     !     !       |\_____)\__
> +                     !     !       ) ____  ___.<
> +                     !     !       |/    )/
> +                     !     !
> +                     !     !
> +                     !   (0,0)---------------------->
> +                     !                 x-Rp
> +                     !
> +                     !
> +                     V
> +                   x-Rc
> +
> +        The two reference systems are not aligned and the 'Rp' reference
> +        system is 90 degrees rotated in counterclockwise direction in respect
> +        to the 'Rc' reference system.
> +
> +        The image, when captured to memory buffer will be rotated.
> +
> +                     +---------------------------------------+
> +                     |                  _ _                  |
> +                     |                 \   /                 |
> +                     |                  | |                  |
> +                     |                  | |                  |
> +                     |                  |  >                 |
> +                     |                 <  |                  |
> +                     |                  | |                  |
> +                     |                    .                  |
> +                     |                   V                   |
> +                     +---------------------------------------+
> +
> +        A correction of 90 degrees in counterclockwise direction has to be
> +        applied to correctly display the image in portrait mode on the device
> +        screen.
> +
> +                                +-----------------+
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |  |\____)\___    |
> +                                |  ) _____  __`<  |
> +                                |  |/     )/      |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                +-----------------+
>  ...
> -- 
> 2.24.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Feb. 3, 2020, 11:42 p.m. UTC | #2
Hi Jacopo,

Thank you for the patch.

This looks really good. The description is quite long, but that's
unavoidable if we want to be both precise and generic enough to cover
all use cases.

Please see below for a few minor comments.

On Mon, Jan 13, 2020 at 05:42:26PM +0100, Jacopo Mondi wrote:
> The rotation property describes the rotation of the camera sensor.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++
>  1 file changed, 309 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index aaadcbd3e52b..243af7bd0a03 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -25,4 +25,313 @@ controls:
>            description: |
>              The camera is attached to the device in a way that allows it to
>              be moved freely
> +
> +  - Rotation:
> +      type: int32_t
> +      description: |
> +        The camera rotation is expressed as the angular difference in degrees
> +        between two reference systems, one implicitly defined by the camera

I wouldn't say "implicitly defined" here, as the reference system is
defined below, quite explicitly :-) How about "one relative to the
camera module" ?

> +        module intrinsics characteristics, and one artificially defined on the
> +        external world scene to be captured when projected on the image sensor
> +        pixel array.
> +
> +        A camera sensor has an implicitly defined 2-dimensional reference
> +        system 'Rc' defined by its pixel array scan-out order, with its origin

"scan-out" is mostly used for displays, for camera sensors the usual
term is "read-out".

> +        posed at pixel address (0,0), the x-axis progressing from there towards

Maybe "placed" instead of "posed" ? The latter doesn't match a
definition I can find, but I may be wrong.

Pixels don't have addresses, they have coordinates.

Unless I'm mistaken axes are usually named with capital letters.

> +        the last scanned out column of the pixel array and the y-axis
> +        progressing towards the last scanned out line.

To match the above,

"A camera sensor has a 2-dimensional reference system 'Rc' defined by
its pixel array read-out order. The origin is set to the first pixel
being read out, the X-axis points along the column read-out direction
towards the last columns, and the Y-axis along the row read-out
direction towards the last row."

> +
> +        A typical example for a sensor with a (2592x1944) pixel array matrix

You can drop the parentheses here, they're used when expressing
coordinate tuples, but not for sizes.

> +        observed from the front is
> +
> +                   (2592)      x-axis          0

s/x/X/ (and below, and same for y)

> +                      <------------------------+ 0
> +                      .......... ... ..........!
> +                      .......... ... ..........! y-axis
> +                                 ...           !
> +                      .......... ... ..........!
> +                      .......... ... ..........! (1944)
> +                                               V

2592 and 1944 should be 2591 and 1943 respectively as you count from 0.
The parentheses are also not needed.

Typically sensors also have dark and active boundary rows and columns,
but that may be too much details to include here.

> +        The external world scene reference system scene 'Rs' is defined as a

s/system scene/system/ ?

> +        2-dimensional reference system on the parallel plane posed in front
> +        of the camera module's focal plane, with its origin placed on the

This is a bit complicated, how about simply saying that the reference
system is defined on the focal plane of the sensor ?

> +        visible top-left corner, the x-axis progressing towards the right from
> +        there and the y-axis progressing towards the bottom of the visible
> +        scene.

Shouldn't we point out that the terms top, left, right and bottom are
intentionally not defined ?

With the above comments applied here,

"The external world scene reference system 'Rs' is a 2-dimensional
reference system on the focal plane of the camera module. The origin is
placed on the top-left corner of the visible scene, the X-axis points
towards the right, and the Y-axis points towards the bottom of the
scene. The top, bottom, left and right directions are intentionally not
defined and depend on the environment in which the camera is used."

> +
> +        A typical example of a (very common) picture of a shark swimming from
> +        left to right is

"from left to right, as seen from the camera, is"

> +
> +                                 x-axis
> +                   (0,0)---------------------->
> +                     !
> +                     !
> +                     !       |\____)\___
> +                     !       ) _____  __`<
> +                     !       |/     )/
> +                     !
> +                     V
> +                   y-axis
> +
> +        With the reference plane posed in front of the camera module and
> +        parallel to its focal plane
> +
> +                                   !
> +                                 / !
> +                                /  !
> +                               /   !
> +                        _     /    !
> +                     +-/ \-+ /     !
> +                     | (o) |       ! 'Rs' reference plane
> +                     +-----+ \     !
> +                              \    !
> +                               \   !
> +                                \  !
> +                                 \ !
> +                                   !
> +
> +        When projected on the sensor's pixel array, the image and the associated
> +        reference system 'Rs' are typically inverted, due to the camera module's

Maybe "typically (but not always)" ?

> +        lens optical inversion effect.
> +
> +        Assuming the above represented scene of the swimming shark, the lens
> +        inversion projects on the sensor pixel array the reference plane 'Rp'

I got confused for a second, wondering why only the Y axis was inverted,
until I realized the this drawing shows the scene projected onto the
camera module as seen when looking at the camera module. I think we
should mention this explicitly:

"Assuming the above represented scene of the swimming shark, the lens
inversion projects the scene and its reference system onto the sensor
pixel array as follows, seen from the front of the camera sensor."

> +
> +                  y-axis
> +                     ^
> +                     !
> +                     !       |\_____)\__
> +                     !       ) ____  ___.<
> +                     !       |/    )/
> +                     !
> +                     !
> +                   (0,0)---------------------->
> +                                 x-axis

"Note the shark being upside-down.

The resulting projected reference system is named 'Rp'."

> +
> +        The camera rotation property is then defined as the angular difference
> +        in counterclockwise direction between the origin of the camera reference

Nitpicking again, an angular difference can't be defined between
origins, as origins are points. It should be defined between reference
systems.

We should specify that the rotation is expressed in degrees as a number
in the [0, 360[ range.

> +        system 'Rc', defined by the camera sensor scan-out direction and its
> +        mounting position, and the origin of the projected scene reference
> +        system 'Rp', result of the optical projection of the scene reference
> +        system 'Rs' on the sensor pixel array.

I don't think we need to repeat the definitions of the two reference
systems.

"The camera rotation property is then defined as the angular difference
in the counterclockwise direction between the camera reference system
'Rc' and the projected scene reference system 'Rp'. It is expressed in
degrees as a number in the range [0, 360[."

> +
> +        Examples
> +
> +        0 degrees camera rotation
> +
> +                          y-Rp
> +                    y-Rc   ^
> +                     ^     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !   (0,0)---------------------->
> +                     !                 x-Rp
> +                   0 +------------------------------------->
> +                      0            x-Rc

The X-axis 0 should be aligned with the +, or the Y-axis 0 moved one
line up. You may want to use the same notation for the Rc and Rp
reference systems ('(0,0)' or split coordinates in both cases).

Should the x-Rc axis have the same length as the x-Rp axis ?

> +
> +
> +                                     x-Rc          0
> +                           <------------------------+ 0
> +                                x-Rp                !
> +                     <-----------------------(0,0)  !
> +                                               !    !
> +                                               !    !
> +                                               !    !
> +                                               !    V
> +                                               !  y-Rc
> +                                               V
> +                                             y-Rp
> +
> +        90 degrees camera rotation
> +
> +                      0        y-Rc
> +                   0 +----------------------->
> +                     !
> +                     !    y-Rp
> +                     !     ^
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !     !
> +                     !   (0,0)---------------------->
> +                     !                 x-Rp
> +                     !
> +                     V
> +                   x-Rc
> +
> +        180 degrees camera rotation
> +
> +                                   x-Cr          0

s/Cr/Rc/

> +                         <------------------------+ 0
> +                     y-Rp                         !
> +                      ^                           !
> +                      !                           ! y-Cr
> +                      !                           !
> +                      !                           !
> +                      !                           V
> +                      !
> +                      !
> +                    (0,0)--------------------->
> +                                   x-Rp
> +
> +        270 degrees camera rotation
> +
> +                      0        y-Rc
> +                   0 +----------------------->
> +                     !            x-Rp
> +                     ! <-----------------------(0,0

Missing ) at the end of the line.

> +                     !                           !
> +                     !                           !
> +                     !                           !
> +                     !                           !
> +                     !                           !
> +                     !                           V
> +                     !                         y-Rp
> +                     !
> +                     !
> +                     V
> +                   x-Rc
> +
> +
> +
> +        Example one - Webcam
> +
> +        A camera module installed on the user facing part of a laptop screen
> +        casing used for video calls. The captured images are meant to be
> +        displayed in landscape mode (width > height) on the laptop screen.
> +
> +        The camera is typically mounted 180 degrees rotated to compensate the
> +        lens optical inversion effect.

I understand what you mean here, but can be confusing for the reader.
The direction of the Rc and Rp reference systems depicted below
correspond to the 0° rotation above, and here the text states 180°. How
about writing "The camera sensor is typically mounted upside-down to
compensate ..." ?

> +
> +                          y-Rp
> +                    y-Rc   ^
> +                     ^     !
> +                     !     !       |\_____)\__
> +                     !     !       ) ____  ___.<
> +                     !     !       |/    )/
> +                     !     !
> +                     !     !
> +                     !   (0,0)---------------------->
> +                     !                 x-Rp
> +                   0 +------------------------------------->
> +                      0                x-Rc
> +
> +        The two reference systems are aligned, the resulting camera rotation is
> +        0 degrees, no rotation correction should be applied to the resulting

s/should/needs to/ ?

> +        image once captured to memory buffers to correctly display it to users.
> +
> +                     +--------------------------+
> +                     !       |\____)\___        !
> +                     !       ) _____  __`<      !
> +                     !       |/     )/          !
> +                     +--------------------------+
> +
> +        If the camera module is not mounted 180 degrees rotated to compensate

And here, for the reason explained above, "is not mounted upside-down to
compensate ..." ?

> +        the lens optical inversion, the two reference system will result not

s/system/systems/

> +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'

s/will result not aligned/will not be aligned/ ?

> +        plane.

As above, we should probably not use plane to refer to Rc and Rp, as
they're defined as reference systems. How about the following ?

"If the camera sensor is not mounted upside-down to compensate for the
lens optical inversion, the two reference systems will not be aligned,
with 'Rp' being rotated 180 degrees relatively to 'Rc'."

> +                                   x-Rc          0
> +                         <------------------------+ 0
> +                     y-Rp                         !
> +                      ^                           !
> +                      !                           ! y-Rc
> +                      !      |\_____)\__          !
> +                      !      ) ____  ___.<        !
> +                      !      |/    )/             V
> +                      !
> +                      !
> +                    (0,0)--------------------->
> +                                   x-Rp
> +
> +        The image once captured to memory will then be 180 degrees rotated

"will then be rotated by 180 degrees" ?

> +
> +                     +--------------------------+
> +                     !          __/(_____/|     !
> +                     !        >.___  ____ (     !
> +                     !             \(    \|     !
> +                     +--------------------------+

Should we add two lines above and below the shark to keep the aspect
ration ?

> +
> +        A software rotation correction of 180 degrees should be applied to
> +        correctly display the image.
> +
> +                     +--------------------------+
> +                     !       |\____)\___        !
> +                     !       ) _____  __`<      !
> +                     !       |/     )/          !
> +                     +--------------------------+

Here too.

> +
> +        Example two - Phone camera
> +
> +        A camera installed on the back-side of a mobile device facing away from

s/back-side/back side/

> +        the user. The captured images are meant to be displayed in portrait mode
> +        (height > width) to match the device screen orientation and the device
> +        usage orientation used when taking the picture.
> +
> +        The camera is typically mounted with its pixel array longer side aligned

s/camera/camera sensor/

> +        to the device longer side, 180 degrees rotated to compensate the lens

s/compensate/compensate for/ ?

> +        optical inversion effect.
> +
> +                      0        y-Rc
> +                   0 +----------------------->
> +                     !
> +                     !    y-Rp
> +                     !     ^
> +                     !     !
> +                     !     !       |\_____)\__
> +                     !     !       ) ____  ___.<
> +                     !     !       |/    )/
> +                     !     !
> +                     !     !
> +                     !   (0,0)---------------------->
> +                     !                 x-Rp
> +                     !
> +                     !
> +                     V
> +                   x-Rc
> +
> +        The two reference systems are not aligned and the 'Rp' reference
> +        system is 90 degrees rotated in counterclockwise direction in respect

"... is rotated by 90 degrees in the counterclockwise direction
relatively to the ..."

> +        to the 'Rc' reference system.
> +
> +        The image, when captured to memory buffer will be rotated.

To match the previous example,

"The image once captured to memory will be rotated."

> +
> +                     +---------------------------------------+
> +                     |                  _ _                  |
> +                     |                 \   /                 |
> +                     |                  | |                  |
> +                     |                  | |                  |
> +                     |                  |  >                 |
> +                     |                 <  |                  |
> +                     |                  | |                  |
> +                     |                    .                  |
> +                     |                   V                   |
> +                     +---------------------------------------+
> +
> +        A correction of 90 degrees in counterclockwise direction has to be
> +        applied to correctly display the image in portrait mode on the device
> +        screen.
> +
> +                                +-----------------+
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |  |\____)\___    |
> +                                |  ) _____  __`<  |
> +                                |  |/     )/      |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                |                 |
> +                                +-----------------+

I had to think a bit to check if the sharks are in the right
orientation, and they are :-) I would however reduce the frame a little
bit to match the size of the references systems above.

>  ...

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Jacopo Mondi Feb. 5, 2020, 10:51 a.m. UTC | #3
Hi Laurent,

On Tue, Feb 04, 2020 at 01:42:49AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> This looks really good. The description is quite long, but that's
> unavoidable if we want to be both precise and generic enough to cover
> all use cases.
>
> Please see below for a few minor comments.
>
> On Mon, Jan 13, 2020 at 05:42:26PM +0100, Jacopo Mondi wrote:
> > The rotation property describes the rotation of the camera sensor.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++
> >  1 file changed, 309 insertions(+)
> >
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index aaadcbd3e52b..243af7bd0a03 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -25,4 +25,313 @@ controls:
> >            description: |
> >              The camera is attached to the device in a way that allows it to
> >              be moved freely
> > +
> > +  - Rotation:
> > +      type: int32_t
> > +      description: |
> > +        The camera rotation is expressed as the angular difference in degrees
> > +        between two reference systems, one implicitly defined by the camera
>
> I wouldn't say "implicitly defined" here, as the reference system is
> defined below, quite explicitly :-) How about "one relative to the
> camera module" ?
>

Would you keep "camera module intrinsics characteristics" or just
"camera module" ?

I would prefer the former, as the coordintate system is defined by the
sensor's pixel array displacement.

> > +        module intrinsics characteristics, and one artificially defined on the
> > +        external world scene to be captured when projected on the image sensor
> > +        pixel array.
> > +
> > +        A camera sensor has an implicitly defined 2-dimensional reference
> > +        system 'Rc' defined by its pixel array scan-out order, with its origin
>
> "scan-out" is mostly used for displays, for camera sensors the usual
> term is "read-out".
>

Ack

> > +        posed at pixel address (0,0), the x-axis progressing from there towards
>
> Maybe "placed" instead of "posed" ? The latter doesn't match a
> definition I can find, but I may be wrong.
>
> Pixels don't have addresses, they have coordinates.

I've seen address used in many sensor chip manuals, I think I took it
from there, but I can indeed change it.

>
> Unless I'm mistaken axes are usually named with capital letters.
>
> > +        the last scanned out column of the pixel array and the y-axis
> > +        progressing towards the last scanned out line.
>
> To match the above,
>
> "A camera sensor has a 2-dimensional reference system 'Rc' defined by
> its pixel array read-out order. The origin is set to the first pixel
> being read out, the X-axis points along the column read-out direction
> towards the last columns, and the Y-axis along the row read-out
> direction towards the last row."
>

I'm not sure I'm super happy with the expression

"The X-axis points along the column read-out direction towards the
last column."

if compared to:

"The X-axis progressing from the here [the first read-out pixel]
towards the last read-out column."

> > +
> > +        A typical example for a sensor with a (2592x1944) pixel array matrix
>
> You can drop the parentheses here, they're used when expressing
> coordinate tuples, but not for sizes.
>

Ack

> > +        observed from the front is
> > +
> > +                   (2592)      x-axis          0
>
> s/x/X/ (and below, and same for y)
>

Ack

> > +                      <------------------------+ 0
> > +                      .......... ... ..........!
> > +                      .......... ... ..........! y-axis
> > +                                 ...           !
> > +                      .......... ... ..........!
> > +                      .......... ... ..........! (1944)
> > +                                               V
>
> 2592 and 1944 should be 2591 and 1943 respectively as you count from 0.
> The parentheses are also not needed.
>

Ack

> Typically sensors also have dark and active boundary rows and columns,
> but that may be too much details to include here.
>

Not a detail I would include here. And, by the way, I the number I
have used here have been taken from the full pixel array size of a
sensor producing 2560x1920 images. They include black and calibration
pixels.

> > +        The external world scene reference system scene 'Rs' is defined as a
>
> s/system scene/system/ ?
>

Ack

> > +        2-dimensional reference system on the parallel plane posed in front
> > +        of the camera module's focal plane, with its origin placed on the
>
> This is a bit complicated, how about simply saying that the reference
> system is defined on the focal plane of the sensor ?
>

Ack

> > +        visible top-left corner, the x-axis progressing towards the right from
> > +        there and the y-axis progressing towards the bottom of the visible
> > +        scene.
>
> Shouldn't we point out that the terms top, left, right and bottom are
> intentionally not defined ?
>
> With the above comments applied here,
>
> "The external world scene reference system 'Rs' is a 2-dimensional
> reference system on the focal plane of the camera module. The origin is
> placed on the top-left corner of the visible scene, the X-axis points
> towards the right, and the Y-axis points towards the bottom of the
> scene. The top, bottom, left and right directions are intentionally not
> defined and depend on the environment in which the camera is used."
>

Ack

> > +
> > +        A typical example of a (very common) picture of a shark swimming from
> > +        left to right is
>
> "from left to right, as seen from the camera, is"
>

Ack

> > +
> > +                                 x-axis
> > +                   (0,0)---------------------->
> > +                     !
> > +                     !
> > +                     !       |\____)\___
> > +                     !       ) _____  __`<
> > +                     !       |/     )/
> > +                     !
> > +                     V
> > +                   y-axis
> > +
> > +        With the reference plane posed in front of the camera module and
> > +        parallel to its focal plane
> > +
> > +                                   !
> > +                                 / !
> > +                                /  !
> > +                               /   !
> > +                        _     /    !
> > +                     +-/ \-+ /     !
> > +                     | (o) |       ! 'Rs' reference plane
> > +                     +-----+ \     !
> > +                              \    !
> > +                               \   !
> > +                                \  !
> > +                                 \ !
> > +                                   !
> > +
> > +        When projected on the sensor's pixel array, the image and the associated
> > +        reference system 'Rs' are typically inverted, due to the camera module's
>
> Maybe "typically (but not always)" ?
>
> > +        lens optical inversion effect.
> > +
> > +        Assuming the above represented scene of the swimming shark, the lens
> > +        inversion projects on the sensor pixel array the reference plane 'Rp'
>
> I got confused for a second, wondering why only the Y axis was inverted,
> until I realized the this drawing shows the scene projected onto the
> camera module as seen when looking at the camera module. I think we
> should mention this explicitly:
>

Indeed, thanks for pointing it out. I inverted the position of the
observer without even noticing.

> "Assuming the above represented scene of the swimming shark, the lens
> inversion projects the scene and its reference system onto the sensor
> pixel array as follows, seen from the front of the camera sensor."
>

Ack

> > +
> > +                  y-axis
> > +                     ^
> > +                     !
> > +                     !       |\_____)\__
> > +                     !       ) ____  ___.<
> > +                     !       |/    )/
> > +                     !
> > +                     !
> > +                   (0,0)---------------------->
> > +                                 x-axis
>
> "Note the shark being upside-down.
>
> The resulting projected reference system is named 'Rp'."
>
> > +
> > +        The camera rotation property is then defined as the angular difference
> > +        in counterclockwise direction between the origin of the camera reference
>
> Nitpicking again, an angular difference can't be defined between
> origins, as origins are points. It should be defined between reference
> systems.
>
> We should specify that the rotation is expressed in degrees as a number
> in the [0, 360[ range.
>

true

> > +        system 'Rc', defined by the camera sensor scan-out direction and its
> > +        mounting position, and the origin of the projected scene reference
> > +        system 'Rp', result of the optical projection of the scene reference
> > +        system 'Rs' on the sensor pixel array.
>
> I don't think we need to repeat the definitions of the two reference
> systems.
>

I was not sure it was needed, but it seemed to me it was not too heavy
to read, but it can be dropped indeed

> "The camera rotation property is then defined as the angular difference
> in the counterclockwise direction between the camera reference system
> 'Rc' and the projected scene reference system 'Rp'. It is expressed in
> degrees as a number in the range [0, 360[."
>

Thanks, works well

> > +
> > +        Examples
> > +
> > +        0 degrees camera rotation
> > +
> > +                          y-Rp
> > +                    y-Rc   ^
> > +                     ^     !
> > +                     !     !
> > +                     !     !
> > +                     !     !
> > +                     !     !
> > +                     !     !
> > +                     !   (0,0)---------------------->
> > +                     !                 x-Rp
> > +                   0 +------------------------------------->
> > +                      0            x-Rc
>
> The X-axis 0 should be aligned with the +, or the Y-axis 0 moved one
> line up. You may want to use the same notation for the Rc and Rp
> reference systems ('(0,0)' or split coordinates in both cases).
>

I am not sure why I used two different notations for the camera and
the projected scene reference systems. I will use a single one

> Should the x-Rc axis have the same length as the x-Rp axis ?
>

They could, let's see how the drawing looks like.

> > +
> > +
> > +                                     x-Rc          0
> > +                           <------------------------+ 0
> > +                                x-Rp                !
> > +                     <-----------------------(0,0)  !
> > +                                               !    !
> > +                                               !    !
> > +                                               !    !
> > +                                               !    V
> > +                                               !  y-Rc
> > +                                               V
> > +                                             y-Rp
> > +
> > +        90 degrees camera rotation
> > +
> > +                      0        y-Rc
> > +                   0 +----------------------->
> > +                     !
> > +                     !    y-Rp
> > +                     !     ^
> > +                     !     !
> > +                     !     !
> > +                     !     !
> > +                     !     !
> > +                     !     !
> > +                     !     !
> > +                     !   (0,0)---------------------->
> > +                     !                 x-Rp
> > +                     !
> > +                     V
> > +                   x-Rc
> > +
> > +        180 degrees camera rotation
> > +
> > +                                   x-Cr          0
>
> s/Cr/Rc/
>
> > +                         <------------------------+ 0
> > +                     y-Rp                         !
> > +                      ^                           !
> > +                      !                           ! y-Cr
> > +                      !                           !
> > +                      !                           !
> > +                      !                           V
> > +                      !
> > +                      !
> > +                    (0,0)--------------------->
> > +                                   x-Rp
> > +
> > +        270 degrees camera rotation
> > +
> > +                      0        y-Rc
> > +                   0 +----------------------->
> > +                     !            x-Rp
> > +                     ! <-----------------------(0,0
>
> Missing ) at the end of the line.
>
> > +                     !                           !
> > +                     !                           !
> > +                     !                           !
> > +                     !                           !
> > +                     !                           !
> > +                     !                           V
> > +                     !                         y-Rp
> > +                     !
> > +                     !
> > +                     V
> > +                   x-Rc
> > +
> > +
> > +
> > +        Example one - Webcam
> > +
> > +        A camera module installed on the user facing part of a laptop screen
> > +        casing used for video calls. The captured images are meant to be
> > +        displayed in landscape mode (width > height) on the laptop screen.
> > +
> > +        The camera is typically mounted 180 degrees rotated to compensate the
> > +        lens optical inversion effect.
>
> I understand what you mean here, but can be confusing for the reader.
> The direction of the Rc and Rp reference systems depicted below
> correspond to the 0° rotation above, and here the text states 180°. How
> about writing "The camera sensor is typically mounted upside-down to
> compensate ..." ?
>

Ack

> > +
> > +                          y-Rp
> > +                    y-Rc   ^
> > +                     ^     !
> > +                     !     !       |\_____)\__
> > +                     !     !       ) ____  ___.<
> > +                     !     !       |/    )/
> > +                     !     !
> > +                     !     !
> > +                     !   (0,0)---------------------->
> > +                     !                 x-Rp
> > +                   0 +------------------------------------->
> > +                      0                x-Rc
> > +
> > +        The two reference systems are aligned, the resulting camera rotation is
> > +        0 degrees, no rotation correction should be applied to the resulting
>
> s/should/needs to/ ?
>

ack

> > +        image once captured to memory buffers to correctly display it to users.
> > +
> > +                     +--------------------------+
> > +                     !       |\____)\___        !
> > +                     !       ) _____  __`<      !
> > +                     !       |/     )/          !
> > +                     +--------------------------+
> > +
> > +        If the camera module is not mounted 180 degrees rotated to compensate
>
> And here, for the reason explained above, "is not mounted upside-down to
> compensate ..." ?
>
> > +        the lens optical inversion, the two reference system will result not
>
> s/system/systems/
>
> > +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'
>
> s/will result not aligned/will not be aligned/ ?
>

ack on all the above comments

> > +        plane.
>
> As above, we should probably not use plane to refer to Rc and Rp, as
> they're defined as reference systems. How about the following ?
>
> "If the camera sensor is not mounted upside-down to compensate for the
> lens optical inversion, the two reference systems will not be aligned,
> with 'Rp' being rotated 180 degrees relatively to 'Rc'."
>

It's more clear, thanks

> > +                                   x-Rc          0
> > +                         <------------------------+ 0
> > +                     y-Rp                         !
> > +                      ^                           !
> > +                      !                           ! y-Rc
> > +                      !      |\_____)\__          !
> > +                      !      ) ____  ___.<        !
> > +                      !      |/    )/             V
> > +                      !
> > +                      !
> > +                    (0,0)--------------------->
> > +                                   x-Rp
> > +
> > +        The image once captured to memory will then be 180 degrees rotated
>
> "will then be rotated by 180 degrees" ?
>

matter of tastes I guess, but ok

> > +
> > +                     +--------------------------+
> > +                     !          __/(_____/|     !
> > +                     !        >.___  ____ (     !
> > +                     !             \(    \|     !
> > +                     +--------------------------+
>
> Should we add two lines above and below the shark to keep the aspect
> ration ?
>
> > +
> > +        A software rotation correction of 180 degrees should be applied to
> > +        correctly display the image.
> > +
> > +                     +--------------------------+
> > +                     !       |\____)\___        !
> > +                     !       ) _____  __`<      !
> > +                     !       |/     )/          !
> > +                     +--------------------------+
>
> Here too.
>
> > +
> > +        Example two - Phone camera
> > +
> > +        A camera installed on the back-side of a mobile device facing away from
>
> s/back-side/back side/
>
> > +        the user. The captured images are meant to be displayed in portrait mode
> > +        (height > width) to match the device screen orientation and the device
> > +        usage orientation used when taking the picture.
> > +
> > +        The camera is typically mounted with its pixel array longer side aligned
>
> s/camera/camera sensor/
>
> > +        to the device longer side, 180 degrees rotated to compensate the lens
>
> s/compensate/compensate for/ ?
>
> > +        optical inversion effect.
> > +
> > +                      0        y-Rc
> > +                   0 +----------------------->
> > +                     !
> > +                     !    y-Rp
> > +                     !     ^
> > +                     !     !
> > +                     !     !       |\_____)\__
> > +                     !     !       ) ____  ___.<
> > +                     !     !       |/    )/
> > +                     !     !
> > +                     !     !
> > +                     !   (0,0)---------------------->
> > +                     !                 x-Rp
> > +                     !
> > +                     !
> > +                     V
> > +                   x-Rc
> > +
> > +        The two reference systems are not aligned and the 'Rp' reference
> > +        system is 90 degrees rotated in counterclockwise direction in respect
>
> "... is rotated by 90 degrees in the counterclockwise direction
> relatively to the ..."
>
> > +        to the 'Rc' reference system.
> > +
> > +        The image, when captured to memory buffer will be rotated.
>
> To match the previous example,
>
> "The image once captured to memory will be rotated."
>
> > +
> > +                     +---------------------------------------+
> > +                     |                  _ _                  |
> > +                     |                 \   /                 |
> > +                     |                  | |                  |
> > +                     |                  | |                  |
> > +                     |                  |  >                 |
> > +                     |                 <  |                  |
> > +                     |                  | |                  |
> > +                     |                    .                  |
> > +                     |                   V                   |
> > +                     +---------------------------------------+
> > +
> > +        A correction of 90 degrees in counterclockwise direction has to be
> > +        applied to correctly display the image in portrait mode on the device
> > +        screen.
> > +
> > +                                +-----------------+
> > +                                |                 |
> > +                                |                 |
> > +                                |                 |
> > +                                |                 |
> > +                                |                 |
> > +                                |                 |
> > +                                |  |\____)\___    |
> > +                                |  ) _____  __`<  |
> > +                                |  |/     )/      |
> > +                                |                 |
> > +                                |                 |
> > +                                |                 |
> > +                                |                 |
> > +                                |                 |
> > +                                +-----------------+
>
> I had to think a bit to check if the sharks are in the right
> orientation, and they are :-) I would however reduce the frame a little
> bit to match the size of the references systems above.
>

I received the opposed comments from Andrey and I agree with him that
we should try to keep the image proportions similar to the ones above
to avoid the idea of cropping. In my view, the coordinate system does
not represent the size of the captured image, but I can work to make
them look like it.

Thanks
  j

> >  ...
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart Feb. 13, 2020, 9:01 p.m. UTC | #4
Hi Jacopo,

On Wed, Feb 05, 2020 at 11:51:05AM +0100, jacopo mondi wrote:
> On Tue, Feb 04, 2020 at 01:42:49AM +0200, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > This looks really good. The description is quite long, but that's
> > unavoidable if we want to be both precise and generic enough to cover
> > all use cases.
> >
> > Please see below for a few minor comments.
> >
> > On Mon, Jan 13, 2020 at 05:42:26PM +0100, Jacopo Mondi wrote:
> > > The rotation property describes the rotation of the camera sensor.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 309 insertions(+)
> > >
> > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > > index aaadcbd3e52b..243af7bd0a03 100644
> > > --- a/src/libcamera/property_ids.yaml
> > > +++ b/src/libcamera/property_ids.yaml
> > > @@ -25,4 +25,313 @@ controls:
> > >            description: |
> > >              The camera is attached to the device in a way that allows it to
> > >              be moved freely
> > > +
> > > +  - Rotation:
> > > +      type: int32_t
> > > +      description: |
> > > +        The camera rotation is expressed as the angular difference in degrees
> > > +        between two reference systems, one implicitly defined by the camera
> >
> > I wouldn't say "implicitly defined" here, as the reference system is
> > defined below, quite explicitly :-) How about "one relative to the
> > camera module" ?
> 
> Would you keep "camera module intrinsics characteristics" or just
> "camera module" ?
> 
> I would prefer the former, as the coordintate system is defined by the
> sensor's pixel array displacement.

I would prefer the latter actually, I'm not sure what "relative to the
camera module intrinsic characteristics" actually mean.

> > > +        module intrinsics characteristics, and one artificially defined on the
> > > +        external world scene to be captured when projected on the image sensor
> > > +        pixel array.
> > > +
> > > +        A camera sensor has an implicitly defined 2-dimensional reference
> > > +        system 'Rc' defined by its pixel array scan-out order, with its origin
> >
> > "scan-out" is mostly used for displays, for camera sensors the usual
> > term is "read-out".
> 
> Ack
> 
> > > +        posed at pixel address (0,0), the x-axis progressing from there towards
> >
> > Maybe "placed" instead of "posed" ? The latter doesn't match a
> > definition I can find, but I may be wrong.
> >
> > Pixels don't have addresses, they have coordinates.
> 
> I've seen address used in many sensor chip manuals, I think I took it
> from there, but I can indeed change it.
> 
> > Unless I'm mistaken axes are usually named with capital letters.
> >
> > > +        the last scanned out column of the pixel array and the y-axis
> > > +        progressing towards the last scanned out line.
> >
> > To match the above,
> >
> > "A camera sensor has a 2-dimensional reference system 'Rc' defined by
> > its pixel array read-out order. The origin is set to the first pixel
> > being read out, the X-axis points along the column read-out direction
> > towards the last columns, and the Y-axis along the row read-out
> > direction towards the last row."
> 
> I'm not sure I'm super happy with the expression
> 
> "The X-axis points along the column read-out direction towards the
> last column."
> 
> if compared to:
> 
> "The X-axis progressing from the here [the first read-out pixel]
> towards the last read-out column."

Did you mean "there" instead of "the here" ? I find the second a bit
strange (otherwise I wouldn't have commented on it :-)), but it may be a
matter of personal taste.

Kieran, what can your native English skills teach us ?

> > > +
> > > +        A typical example for a sensor with a (2592x1944) pixel array matrix
> >
> > You can drop the parentheses here, they're used when expressing
> > coordinate tuples, but not for sizes.
> 
> Ack
> 
> > > +        observed from the front is
> > > +
> > > +                   (2592)      x-axis          0
> >
> > s/x/X/ (and below, and same for y)
> 
> Ack
> 
> > > +                      <------------------------+ 0
> > > +                      .......... ... ..........!
> > > +                      .......... ... ..........! y-axis
> > > +                                 ...           !
> > > +                      .......... ... ..........!
> > > +                      .......... ... ..........! (1944)
> > > +                                               V
> >
> > 2592 and 1944 should be 2591 and 1943 respectively as you count from 0.
> > The parentheses are also not needed.
> 
> Ack
> 
> > Typically sensors also have dark and active boundary rows and columns,
> > but that may be too much details to include here.
> 
> Not a detail I would include here. And, by the way, I the number I
> have used here have been taken from the full pixel array size of a
> sensor producing 2560x1920 images. They include black and calibration
> pixels.
> 
> > > +        The external world scene reference system scene 'Rs' is defined as a
> >
> > s/system scene/system/ ?
> 
> Ack
> 
> > > +        2-dimensional reference system on the parallel plane posed in front
> > > +        of the camera module's focal plane, with its origin placed on the
> >
> > This is a bit complicated, how about simply saying that the reference
> > system is defined on the focal plane of the sensor ?
> 
> Ack
> 
> > > +        visible top-left corner, the x-axis progressing towards the right from
> > > +        there and the y-axis progressing towards the bottom of the visible
> > > +        scene.
> >
> > Shouldn't we point out that the terms top, left, right and bottom are
> > intentionally not defined ?
> >
> > With the above comments applied here,
> >
> > "The external world scene reference system 'Rs' is a 2-dimensional
> > reference system on the focal plane of the camera module. The origin is
> > placed on the top-left corner of the visible scene, the X-axis points
> > towards the right, and the Y-axis points towards the bottom of the
> > scene. The top, bottom, left and right directions are intentionally not
> > defined and depend on the environment in which the camera is used."
> 
> Ack
> 
> > > +
> > > +        A typical example of a (very common) picture of a shark swimming from
> > > +        left to right is
> >
> > "from left to right, as seen from the camera, is"
> 
> Ack
> 
> > > +
> > > +                                 x-axis
> > > +                   (0,0)---------------------->
> > > +                     !
> > > +                     !
> > > +                     !       |\____)\___
> > > +                     !       ) _____  __`<
> > > +                     !       |/     )/
> > > +                     !
> > > +                     V
> > > +                   y-axis
> > > +
> > > +        With the reference plane posed in front of the camera module and
> > > +        parallel to its focal plane
> > > +
> > > +                                   !
> > > +                                 / !
> > > +                                /  !
> > > +                               /   !
> > > +                        _     /    !
> > > +                     +-/ \-+ /     !
> > > +                     | (o) |       ! 'Rs' reference plane
> > > +                     +-----+ \     !
> > > +                              \    !
> > > +                               \   !
> > > +                                \  !
> > > +                                 \ !
> > > +                                   !
> > > +
> > > +        When projected on the sensor's pixel array, the image and the associated
> > > +        reference system 'Rs' are typically inverted, due to the camera module's
> >
> > Maybe "typically (but not always)" ?
> >
> > > +        lens optical inversion effect.
> > > +
> > > +        Assuming the above represented scene of the swimming shark, the lens
> > > +        inversion projects on the sensor pixel array the reference plane 'Rp'
> >
> > I got confused for a second, wondering why only the Y axis was inverted,
> > until I realized the this drawing shows the scene projected onto the
> > camera module as seen when looking at the camera module. I think we
> > should mention this explicitly:
> 
> Indeed, thanks for pointing it out. I inverted the position of the
> observer without even noticing.
> 
> > "Assuming the above represented scene of the swimming shark, the lens
> > inversion projects the scene and its reference system onto the sensor
> > pixel array as follows, seen from the front of the camera sensor."
> 
> Ack
> 
> > > +
> > > +                  y-axis
> > > +                     ^
> > > +                     !
> > > +                     !       |\_____)\__
> > > +                     !       ) ____  ___.<
> > > +                     !       |/    )/
> > > +                     !
> > > +                     !
> > > +                   (0,0)---------------------->
> > > +                                 x-axis
> >
> > "Note the shark being upside-down.
> >
> > The resulting projected reference system is named 'Rp'."
> >
> > > +
> > > +        The camera rotation property is then defined as the angular difference
> > > +        in counterclockwise direction between the origin of the camera reference
> >
> > Nitpicking again, an angular difference can't be defined between
> > origins, as origins are points. It should be defined between reference
> > systems.
> >
> > We should specify that the rotation is expressed in degrees as a number
> > in the [0, 360[ range.
> 
> true
> 
> > > +        system 'Rc', defined by the camera sensor scan-out direction and its
> > > +        mounting position, and the origin of the projected scene reference
> > > +        system 'Rp', result of the optical projection of the scene reference
> > > +        system 'Rs' on the sensor pixel array.
> >
> > I don't think we need to repeat the definitions of the two reference
> > systems.
> 
> I was not sure it was needed, but it seemed to me it was not too heavy
> to read, but it can be dropped indeed
> 
> > "The camera rotation property is then defined as the angular difference
> > in the counterclockwise direction between the camera reference system
> > 'Rc' and the projected scene reference system 'Rp'. It is expressed in
> > degrees as a number in the range [0, 360[."
> 
> Thanks, works well
> 
> > > +
> > > +        Examples
> > > +
> > > +        0 degrees camera rotation
> > > +
> > > +                          y-Rp
> > > +                    y-Rc   ^
> > > +                     ^     !
> > > +                     !     !
> > > +                     !     !
> > > +                     !     !
> > > +                     !     !
> > > +                     !     !
> > > +                     !   (0,0)---------------------->
> > > +                     !                 x-Rp
> > > +                   0 +------------------------------------->
> > > +                      0            x-Rc
> >
> > The X-axis 0 should be aligned with the +, or the Y-axis 0 moved one
> > line up. You may want to use the same notation for the Rc and Rp
> > reference systems ('(0,0)' or split coordinates in both cases).
> 
> I am not sure why I used two different notations for the camera and
> the projected scene reference systems. I will use a single one
> 
> > Should the x-Rc axis have the same length as the x-Rp axis ?
> 
> They could, let's see how the drawing looks like.
> 
> > > +
> > > +
> > > +                                     x-Rc          0
> > > +                           <------------------------+ 0
> > > +                                x-Rp                !
> > > +                     <-----------------------(0,0)  !
> > > +                                               !    !
> > > +                                               !    !
> > > +                                               !    !
> > > +                                               !    V
> > > +                                               !  y-Rc
> > > +                                               V
> > > +                                             y-Rp
> > > +
> > > +        90 degrees camera rotation
> > > +
> > > +                      0        y-Rc
> > > +                   0 +----------------------->
> > > +                     !
> > > +                     !    y-Rp
> > > +                     !     ^
> > > +                     !     !
> > > +                     !     !
> > > +                     !     !
> > > +                     !     !
> > > +                     !     !
> > > +                     !     !
> > > +                     !   (0,0)---------------------->
> > > +                     !                 x-Rp
> > > +                     !
> > > +                     V
> > > +                   x-Rc
> > > +
> > > +        180 degrees camera rotation
> > > +
> > > +                                   x-Cr          0
> >
> > s/Cr/Rc/
> >
> > > +                         <------------------------+ 0
> > > +                     y-Rp                         !
> > > +                      ^                           !
> > > +                      !                           ! y-Cr
> > > +                      !                           !
> > > +                      !                           !
> > > +                      !                           V
> > > +                      !
> > > +                      !
> > > +                    (0,0)--------------------->
> > > +                                   x-Rp
> > > +
> > > +        270 degrees camera rotation
> > > +
> > > +                      0        y-Rc
> > > +                   0 +----------------------->
> > > +                     !            x-Rp
> > > +                     ! <-----------------------(0,0
> >
> > Missing ) at the end of the line.
> >
> > > +                     !                           !
> > > +                     !                           !
> > > +                     !                           !
> > > +                     !                           !
> > > +                     !                           !
> > > +                     !                           V
> > > +                     !                         y-Rp
> > > +                     !
> > > +                     !
> > > +                     V
> > > +                   x-Rc
> > > +
> > > +
> > > +
> > > +        Example one - Webcam
> > > +
> > > +        A camera module installed on the user facing part of a laptop screen
> > > +        casing used for video calls. The captured images are meant to be
> > > +        displayed in landscape mode (width > height) on the laptop screen.
> > > +
> > > +        The camera is typically mounted 180 degrees rotated to compensate the
> > > +        lens optical inversion effect.
> >
> > I understand what you mean here, but can be confusing for the reader.
> > The direction of the Rc and Rp reference systems depicted below
> > correspond to the 0° rotation above, and here the text states 180°. How
> > about writing "The camera sensor is typically mounted upside-down to
> > compensate ..." ?
> 
> Ack
> 
> > > +
> > > +                          y-Rp
> > > +                    y-Rc   ^
> > > +                     ^     !
> > > +                     !     !       |\_____)\__
> > > +                     !     !       ) ____  ___.<
> > > +                     !     !       |/    )/
> > > +                     !     !
> > > +                     !     !
> > > +                     !   (0,0)---------------------->
> > > +                     !                 x-Rp
> > > +                   0 +------------------------------------->
> > > +                      0                x-Rc
> > > +
> > > +        The two reference systems are aligned, the resulting camera rotation is
> > > +        0 degrees, no rotation correction should be applied to the resulting
> >
> > s/should/needs to/ ?
> 
> ack
> 
> > > +        image once captured to memory buffers to correctly display it to users.
> > > +
> > > +                     +--------------------------+
> > > +                     !       |\____)\___        !
> > > +                     !       ) _____  __`<      !
> > > +                     !       |/     )/          !
> > > +                     +--------------------------+
> > > +
> > > +        If the camera module is not mounted 180 degrees rotated to compensate
> >
> > And here, for the reason explained above, "is not mounted upside-down to
> > compensate ..." ?
> >
> > > +        the lens optical inversion, the two reference system will result not
> >
> > s/system/systems/
> >
> > > +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'
> >
> > s/will result not aligned/will not be aligned/ ?
> 
> ack on all the above comments
> 
> > > +        plane.
> >
> > As above, we should probably not use plane to refer to Rc and Rp, as
> > they're defined as reference systems. How about the following ?
> >
> > "If the camera sensor is not mounted upside-down to compensate for the
> > lens optical inversion, the two reference systems will not be aligned,
> > with 'Rp' being rotated 180 degrees relatively to 'Rc'."
> 
> It's more clear, thanks
> 
> > > +                                   x-Rc          0
> > > +                         <------------------------+ 0
> > > +                     y-Rp                         !
> > > +                      ^                           !
> > > +                      !                           ! y-Rc
> > > +                      !      |\_____)\__          !
> > > +                      !      ) ____  ___.<        !
> > > +                      !      |/    )/             V
> > > +                      !
> > > +                      !
> > > +                    (0,0)--------------------->
> > > +                                   x-Rp
> > > +
> > > +        The image once captured to memory will then be 180 degrees rotated
> >
> > "will then be rotated by 180 degrees" ?
> 
> matter of tastes I guess, but ok
> 
> > > +
> > > +                     +--------------------------+
> > > +                     !          __/(_____/|     !
> > > +                     !        >.___  ____ (     !
> > > +                     !             \(    \|     !
> > > +                     +--------------------------+
> >
> > Should we add two lines above and below the shark to keep the aspect
> > ration ?
> >
> > > +
> > > +        A software rotation correction of 180 degrees should be applied to
> > > +        correctly display the image.
> > > +
> > > +                     +--------------------------+
> > > +                     !       |\____)\___        !
> > > +                     !       ) _____  __`<      !
> > > +                     !       |/     )/          !
> > > +                     +--------------------------+
> >
> > Here too.
> >
> > > +
> > > +        Example two - Phone camera
> > > +
> > > +        A camera installed on the back-side of a mobile device facing away from
> >
> > s/back-side/back side/
> >
> > > +        the user. The captured images are meant to be displayed in portrait mode
> > > +        (height > width) to match the device screen orientation and the device
> > > +        usage orientation used when taking the picture.
> > > +
> > > +        The camera is typically mounted with its pixel array longer side aligned
> >
> > s/camera/camera sensor/
> >
> > > +        to the device longer side, 180 degrees rotated to compensate the lens
> >
> > s/compensate/compensate for/ ?
> >
> > > +        optical inversion effect.
> > > +
> > > +                      0        y-Rc
> > > +                   0 +----------------------->
> > > +                     !
> > > +                     !    y-Rp
> > > +                     !     ^
> > > +                     !     !
> > > +                     !     !       |\_____)\__
> > > +                     !     !       ) ____  ___.<
> > > +                     !     !       |/    )/
> > > +                     !     !
> > > +                     !     !
> > > +                     !   (0,0)---------------------->
> > > +                     !                 x-Rp
> > > +                     !
> > > +                     !
> > > +                     V
> > > +                   x-Rc
> > > +
> > > +        The two reference systems are not aligned and the 'Rp' reference
> > > +        system is 90 degrees rotated in counterclockwise direction in respect
> >
> > "... is rotated by 90 degrees in the counterclockwise direction
> > relatively to the ..."
> >
> > > +        to the 'Rc' reference system.
> > > +
> > > +        The image, when captured to memory buffer will be rotated.
> >
> > To match the previous example,
> >
> > "The image once captured to memory will be rotated."
> >
> > > +
> > > +                     +---------------------------------------+
> > > +                     |                  _ _                  |
> > > +                     |                 \   /                 |
> > > +                     |                  | |                  |
> > > +                     |                  | |                  |
> > > +                     |                  |  >                 |
> > > +                     |                 <  |                  |
> > > +                     |                  | |                  |
> > > +                     |                    .                  |
> > > +                     |                   V                   |
> > > +                     +---------------------------------------+
> > > +
> > > +        A correction of 90 degrees in counterclockwise direction has to be
> > > +        applied to correctly display the image in portrait mode on the device
> > > +        screen.
> > > +
> > > +                                +-----------------+
> > > +                                |                 |
> > > +                                |                 |
> > > +                                |                 |
> > > +                                |                 |
> > > +                                |                 |
> > > +                                |                 |
> > > +                                |  |\____)\___    |
> > > +                                |  ) _____  __`<  |
> > > +                                |  |/     )/      |
> > > +                                |                 |
> > > +                                |                 |
> > > +                                |                 |
> > > +                                |                 |
> > > +                                |                 |
> > > +                                +-----------------+
> >
> > I had to think a bit to check if the sharks are in the right
> > orientation, and they are :-) I would however reduce the frame a little
> > bit to match the size of the references systems above.
> 
> I received the opposed comments from Andrey and I agree with him that
> we should try to keep the image proportions similar to the ones above
> to avoid the idea of cropping. In my view, the coordinate system does
> not represent the size of the captured image, but I can work to make
> them look like it.
> 
> > >  ...
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Kieran Bingham Feb. 13, 2020, 9:46 p.m. UTC | #5
On 13/02/2020 21:01, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> On Wed, Feb 05, 2020 at 11:51:05AM +0100, jacopo mondi wrote:
>> On Tue, Feb 04, 2020 at 01:42:49AM +0200, Laurent Pinchart wrote:
>>> Hi Jacopo,
>>>
>>> Thank you for the patch.
>>>
>>> This looks really good. The description is quite long, but that's
>>> unavoidable if we want to be both precise and generic enough to cover
>>> all use cases.
>>>
>>> Please see below for a few minor comments.
>>>
>>> On Mon, Jan 13, 2020 at 05:42:26PM +0100, Jacopo Mondi wrote:
>>>> The rotation property describes the rotation of the camera sensor.
>>>>
>>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>>>> ---
>>>>  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++
>>>>  1 file changed, 309 insertions(+)
>>>>
>>>> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
>>>> index aaadcbd3e52b..243af7bd0a03 100644
>>>> --- a/src/libcamera/property_ids.yaml
>>>> +++ b/src/libcamera/property_ids.yaml
>>>> @@ -25,4 +25,313 @@ controls:
>>>>            description: |
>>>>              The camera is attached to the device in a way that allows it to
>>>>              be moved freely
>>>> +
>>>> +  - Rotation:
>>>> +      type: int32_t
>>>> +      description: |
>>>> +        The camera rotation is expressed as the angular difference in degrees
>>>> +        between two reference systems, one implicitly defined by the camera
>>>
>>> I wouldn't say "implicitly defined" here, as the reference system is
>>> defined below, quite explicitly :-) How about "one relative to the
>>> camera module" ?
>>
>> Would you keep "camera module intrinsics characteristics" or just
>> "camera module" ?
>>
>> I would prefer the former, as the coordintate system is defined by the
>> sensor's pixel array displacement.
> 
> I would prefer the latter actually, I'm not sure what "relative to the
> camera module intrinsic characteristics" actually mean.
> 
>>>> +        module intrinsics characteristics, and one artificially defined on the
>>>> +        external world scene to be captured when projected on the image sensor
>>>> +        pixel array.
>>>> +
>>>> +        A camera sensor has an implicitly defined 2-dimensional reference
>>>> +        system 'Rc' defined by its pixel array scan-out order, with its origin
>>>
>>> "scan-out" is mostly used for displays, for camera sensors the usual
>>> term is "read-out".
>>
>> Ack
>>
>>>> +        posed at pixel address (0,0), the x-axis progressing from there towards
>>>
>>> Maybe "placed" instead of "posed" ? The latter doesn't match a
>>> definition I can find, but I may be wrong.

>>>
>>> Pixels don't have addresses, they have coordinates.

I'd use 'positioned at coordinate 0,0' or such then ...

The origin hasn't been 'placed'.

You could also use "located at ..."


>> I've seen address used in many sensor chip manuals, I think I took it
>> from there, but I can indeed change it.
>>
>>> Unless I'm mistaken axes are usually named with capital letters.
>>>
>>>> +        the last scanned out column of the pixel array and the y-axis
>>>> +        progressing towards the last scanned out line.
>>>
>>> To match the above,
>>>
>>> "A camera sensor has a 2-dimensional reference system 'Rc' defined by
>>> its pixel array read-out order. The origin is set to the first pixel
>>> being read out, the X-axis points along the column read-out direction
>>> towards the last columns, and the Y-axis along the row read-out
>>> direction towards the last row."
>>
>> I'm not sure I'm super happy with the expression
>>
>> "The X-axis points along the column read-out direction towards the
>> last column."
>>
>> if compared to:
>>
>> "The X-axis progressing from the here [the first read-out pixel]
>> towards the last read-out column."
> 
> Did you mean "there" instead of "the here" ? I find the second a bit
> strange (otherwise I wouldn't have commented on it :-)), but it may be a
> matter of personal taste.
> 
> Kieran, what can your native English skills teach us ?


We'd never really use "the here" ... but in the sentences above "there"
doesn't make sense either...

I think I'm missing the original context statement...


> 
>>>> +
>>>> +        A typical example for a sensor with a (2592x1944) pixel array matrix
>>>
>>> You can drop the parentheses here, they're used when expressing
>>> coordinate tuples, but not for sizes.
>>
>> Ack
>>
>>>> +        observed from the front is
>>>> +
>>>> +                   (2592)      x-axis          0
>>>
>>> s/x/X/ (and below, and same for y)
>>
>> Ack
>>
>>>> +                      <------------------------+ 0
>>>> +                      .......... ... ..........!
>>>> +                      .......... ... ..........! y-axis
>>>> +                                 ...           !
>>>> +                      .......... ... ..........!
>>>> +                      .......... ... ..........! (1944)
>>>> +                                               V
>>>
>>> 2592 and 1944 should be 2591 and 1943 respectively as you count from 0.
>>> The parentheses are also not needed.
>>
>> Ack
>>
>>> Typically sensors also have dark and active boundary rows and columns,
>>> but that may be too much details to include here.
>>
>> Not a detail I would include here. And, by the way, I the number I
>> have used here have been taken from the full pixel array size of a
>> sensor producing 2560x1920 images. They include black and calibration
>> pixels.
>>
>>>> +        The external world scene reference system scene 'Rs' is defined as a
>>>
>>> s/system scene/system/ ?
>>
>> Ack
>>
>>>> +        2-dimensional reference system on the parallel plane posed in front
>>>> +        of the camera module's focal plane, with its origin placed on the
>>>
>>> This is a bit complicated, how about simply saying that the reference
>>> system is defined on the focal plane of the sensor ?
>>
>> Ack
>>
>>>> +        visible top-left corner, the x-axis progressing towards the right from
>>>> +        there and the y-axis progressing towards the bottom of the visible
>>>> +        scene.
>>>
>>> Shouldn't we point out that the terms top, left, right and bottom are
>>> intentionally not defined ?
>>>
>>> With the above comments applied here,
>>>
>>> "The external world scene reference system 'Rs' is a 2-dimensional
>>> reference system on the focal plane of the camera module. The origin is
>>> placed on the top-left corner of the visible scene, the X-axis points
>>> towards the right, and the Y-axis points towards the bottom of the
>>> scene. The top, bottom, left and right directions are intentionally not
>>> defined and depend on the environment in which the camera is used."
>>
>> Ack
>>
>>>> +
>>>> +        A typical example of a (very common) picture of a shark swimming from
>>>> +        left to right is
>>>
>>> "from left to right, as seen from the camera, is"
>>
>> Ack
>>
>>>> +
>>>> +                                 x-axis
>>>> +                   (0,0)---------------------->
>>>> +                     !
>>>> +                     !
>>>> +                     !       |\____)\___
>>>> +                     !       ) _____  __`<
>>>> +                     !       |/     )/
>>>> +                     !
>>>> +                     V
>>>> +                   y-axis
>>>> +
>>>> +        With the reference plane posed in front of the camera module and
>>>> +        parallel to its focal plane
>>>> +
>>>> +                                   !
>>>> +                                 / !
>>>> +                                /  !
>>>> +                               /   !
>>>> +                        _     /    !
>>>> +                     +-/ \-+ /     !
>>>> +                     | (o) |       ! 'Rs' reference plane
>>>> +                     +-----+ \     !
>>>> +                              \    !
>>>> +                               \   !
>>>> +                                \  !
>>>> +                                 \ !
>>>> +                                   !
>>>> +
>>>> +        When projected on the sensor's pixel array, the image and the associated
>>>> +        reference system 'Rs' are typically inverted, due to the camera module's
>>>
>>> Maybe "typically (but not always)" ?
>>>
>>>> +        lens optical inversion effect.
>>>> +
>>>> +        Assuming the above represented scene of the swimming shark, the lens
>>>> +        inversion projects on the sensor pixel array the reference plane 'Rp'
>>>
>>> I got confused for a second, wondering why only the Y axis was inverted,
>>> until I realized the this drawing shows the scene projected onto the
>>> camera module as seen when looking at the camera module. I think we
>>> should mention this explicitly:
>>
>> Indeed, thanks for pointing it out. I inverted the position of the
>> observer without even noticing.
>>
>>> "Assuming the above represented scene of the swimming shark, the lens
>>> inversion projects the scene and its reference system onto the sensor
>>> pixel array as follows, seen from the front of the camera sensor."
>>
>> Ack
>>
>>>> +
>>>> +                  y-axis
>>>> +                     ^
>>>> +                     !
>>>> +                     !       |\_____)\__
>>>> +                     !       ) ____  ___.<
>>>> +                     !       |/    )/
>>>> +                     !
>>>> +                     !
>>>> +                   (0,0)---------------------->
>>>> +                                 x-axis
>>>
>>> "Note the shark being upside-down.
>>>
>>> The resulting projected reference system is named 'Rp'."
>>>
>>>> +
>>>> +        The camera rotation property is then defined as the angular difference
>>>> +        in counterclockwise direction between the origin of the camera reference

s/counterclockwise/counter-clockwise/


>>>
>>> Nitpicking again, an angular difference can't be defined between
>>> origins, as origins are points. It should be defined between reference
>>> systems.
>>>
>>> We should specify that the rotation is expressed in degrees as a number
>>> in the [0, 360[ range.
>>
>> true
>>
>>>> +        system 'Rc', defined by the camera sensor scan-out direction and its
>>>> +        mounting position, and the origin of the projected scene reference
>>>> +        system 'Rp', result of the optical projection of the scene reference
>>>> +        system 'Rs' on the sensor pixel array.
>>>
>>> I don't think we need to repeat the definitions of the two reference
>>> systems.
>>
>> I was not sure it was needed, but it seemed to me it was not too heavy
>> to read, but it can be dropped indeed
>>
>>> "The camera rotation property is then defined as the angular difference
>>> in the counterclockwise direction between the camera reference system
>>> 'Rc' and the projected scene reference system 'Rp'. It is expressed in
>>> degrees as a number in the range [0, 360[."
>>
>> Thanks, works well
>>
>>>> +
>>>> +        Examples
>>>> +
>>>> +        0 degrees camera rotation
>>>> +
>>>> +                          y-Rp
>>>> +                    y-Rc   ^
>>>> +                     ^     !
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !   (0,0)---------------------->
>>>> +                     !                 x-Rp
>>>> +                   0 +------------------------------------->
>>>> +                      0            x-Rc
>>>
>>> The X-axis 0 should be aligned with the +, or the Y-axis 0 moved one
>>> line up. You may want to use the same notation for the Rc and Rp
>>> reference systems ('(0,0)' or split coordinates in both cases).
>>
>> I am not sure why I used two different notations for the camera and
>> the projected scene reference systems. I will use a single one
>>
>>> Should the x-Rc axis have the same length as the x-Rp axis ?
>>
>> They could, let's see how the drawing looks like.
>>
>>>> +
>>>> +
>>>> +                                     x-Rc          0
>>>> +                           <------------------------+ 0
>>>> +                                x-Rp                !
>>>> +                     <-----------------------(0,0)  !
>>>> +                                               !    !
>>>> +                                               !    !
>>>> +                                               !    !
>>>> +                                               !    V
>>>> +                                               !  y-Rc
>>>> +                                               V
>>>> +                                             y-Rp
>>>> +
>>>> +        90 degrees camera rotation
>>>> +
>>>> +                      0        y-Rc
>>>> +                   0 +----------------------->
>>>> +                     !
>>>> +                     !    y-Rp
>>>> +                     !     ^
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !   (0,0)---------------------->
>>>> +                     !                 x-Rp
>>>> +                     !
>>>> +                     V
>>>> +                   x-Rc
>>>> +
>>>> +        180 degrees camera rotation
>>>> +
>>>> +                                   x-Cr          0
>>>
>>> s/Cr/Rc/
>>>
>>>> +                         <------------------------+ 0
>>>> +                     y-Rp                         !
>>>> +                      ^                           !
>>>> +                      !                           ! y-Cr
>>>> +                      !                           !
>>>> +                      !                           !
>>>> +                      !                           V
>>>> +                      !
>>>> +                      !
>>>> +                    (0,0)--------------------->
>>>> +                                   x-Rp
>>>> +
>>>> +        270 degrees camera rotation
>>>> +
>>>> +                      0        y-Rc
>>>> +                   0 +----------------------->
>>>> +                     !            x-Rp
>>>> +                     ! <-----------------------(0,0
>>>
>>> Missing ) at the end of the line.
>>>
>>>> +                     !                           !
>>>> +                     !                           !
>>>> +                     !                           !
>>>> +                     !                           !
>>>> +                     !                           !
>>>> +                     !                           V
>>>> +                     !                         y-Rp
>>>> +                     !
>>>> +                     !
>>>> +                     V
>>>> +                   x-Rc
>>>> +
>>>> +
>>>> +
>>>> +        Example one - Webcam
>>>> +
>>>> +        A camera module installed on the user facing part of a laptop screen
>>>> +        casing used for video calls. The captured images are meant to be
>>>> +        displayed in landscape mode (width > height) on the laptop screen.
>>>> +
>>>> +        The camera is typically mounted 180 degrees rotated to compensate the
>>>> +        lens optical inversion effect.
>>>
>>> I understand what you mean here, but can be confusing for the reader.
>>> The direction of the Rc and Rp reference systems depicted below
>>> correspond to the 0° rotation above, and here the text states 180°. How
>>> about writing "The camera sensor is typically mounted upside-down to
>>> compensate ..." ?
>>
>> Ack
>>
>>>> +
>>>> +                          y-Rp
>>>> +                    y-Rc   ^
>>>> +                     ^     !
>>>> +                     !     !       |\_____)\__
>>>> +                     !     !       ) ____  ___.<
>>>> +                     !     !       |/    )/
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !   (0,0)---------------------->
>>>> +                     !                 x-Rp
>>>> +                   0 +------------------------------------->
>>>> +                      0                x-Rc
>>>> +
>>>> +        The two reference systems are aligned, the resulting camera rotation is
>>>> +        0 degrees, no rotation correction should be applied to the resulting
>>>
>>> s/should/needs to/ ?
>>
>> ack
>>
>>>> +        image once captured to memory buffers to correctly display it to users.
>>>> +
>>>> +                     +--------------------------+
>>>> +                     !       |\____)\___        !
>>>> +                     !       ) _____  __`<      !
>>>> +                     !       |/     )/          !
>>>> +                     +--------------------------+
>>>> +
>>>> +        If the camera module is not mounted 180 degrees rotated to compensate
>>>
>>> And here, for the reason explained above, "is not mounted upside-down to
>>> compensate ..." ?
>>>
>>>> +        the lens optical inversion, the two reference system will result not
>>>
>>> s/system/systems/
>>>
>>>> +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'
>>>
>>> s/will result not aligned/will not be aligned/ ?
>>
>> ack on all the above comments
>>
>>>> +        plane.
>>>
>>> As above, we should probably not use plane to refer to Rc and Rp, as
>>> they're defined as reference systems. How about the following ?
>>>
>>> "If the camera sensor is not mounted upside-down to compensate for the
>>> lens optical inversion, the two reference systems will not be aligned,
>>> with 'Rp' being rotated 180 degrees relatively to 'Rc'."
>>
>> It's more clear, thanks
>>
>>>> +                                   x-Rc          0
>>>> +                         <------------------------+ 0
>>>> +                     y-Rp                         !
>>>> +                      ^                           !
>>>> +                      !                           ! y-Rc
>>>> +                      !      |\_____)\__          !
>>>> +                      !      ) ____  ___.<        !
>>>> +                      !      |/    )/             V
>>>> +                      !
>>>> +                      !
>>>> +                    (0,0)--------------------->
>>>> +                                   x-Rp
>>>> +
>>>> +        The image once captured to memory will then be 180 degrees rotated
>>>
>>> "will then be rotated by 180 degrees" ?
>>
>> matter of tastes I guess, but ok
>>
>>>> +
>>>> +                     +--------------------------+
>>>> +                     !          __/(_____/|     !
>>>> +                     !        >.___  ____ (     !
>>>> +                     !             \(    \|     !
>>>> +                     +--------------------------+
>>>
>>> Should we add two lines above and below the shark to keep the aspect
>>> ration ?
>>>
>>>> +
>>>> +        A software rotation correction of 180 degrees should be applied to
>>>> +        correctly display the image.
>>>> +
>>>> +                     +--------------------------+
>>>> +                     !       |\____)\___        !
>>>> +                     !       ) _____  __`<      !
>>>> +                     !       |/     )/          !
>>>> +                     +--------------------------+
>>>
>>> Here too.
>>>
>>>> +
>>>> +        Example two - Phone camera
>>>> +
>>>> +        A camera installed on the back-side of a mobile device facing away from
>>>
>>> s/back-side/back side/
>>>
>>>> +        the user. The captured images are meant to be displayed in portrait mode
>>>> +        (height > width) to match the device screen orientation and the device
>>>> +        usage orientation used when taking the picture.
>>>> +
>>>> +        The camera is typically mounted with its pixel array longer side aligned
>>>
>>> s/camera/camera sensor/
>>>
>>>> +        to the device longer side, 180 degrees rotated to compensate the lens
>>>
>>> s/compensate/compensate for/ ?
>>>
>>>> +        optical inversion effect.
>>>> +
>>>> +                      0        y-Rc
>>>> +                   0 +----------------------->
>>>> +                     !
>>>> +                     !    y-Rp
>>>> +                     !     ^
>>>> +                     !     !
>>>> +                     !     !       |\_____)\__
>>>> +                     !     !       ) ____  ___.<
>>>> +                     !     !       |/    )/
>>>> +                     !     !
>>>> +                     !     !
>>>> +                     !   (0,0)---------------------->
>>>> +                     !                 x-Rp
>>>> +                     !
>>>> +                     !
>>>> +                     V
>>>> +                   x-Rc
>>>> +
>>>> +        The two reference systems are not aligned and the 'Rp' reference
>>>> +        system is 90 degrees rotated in counterclockwise direction in respect
>>>
>>> "... is rotated by 90 degrees in the counterclockwise direction
>>> relatively to the ..."

s/counterclockwise/counter-clockwise/


>>>
>>>> +        to the 'Rc' reference system.
>>>> +
>>>> +        The image, when captured to memory buffer will be rotated.
>>>
>>> To match the previous example,
>>>
>>> "The image once captured to memory will be rotated."
>>>
>>>> +
>>>> +                     +---------------------------------------+
>>>> +                     |                  _ _                  |
>>>> +                     |                 \   /                 |
>>>> +                     |                  | |                  |
>>>> +                     |                  | |                  |
>>>> +                     |                  |  >                 |
>>>> +                     |                 <  |                  |
>>>> +                     |                  | |                  |
>>>> +                     |                    .                  |
>>>> +                     |                   V                   |
>>>> +                     +---------------------------------------+
>>>> +
>>>> +        A correction of 90 degrees in counterclockwise direction has to be

And again


>>>> +        applied to correctly display the image in portrait mode on the device
>>>> +        screen.
>>>> +
>>>> +                                +-----------------+
>>>> +                                |                 |
>>>> +                                |                 |
>>>> +                                |                 |
>>>> +                                |                 |
>>>> +                                |                 |
>>>> +                                |                 |
>>>> +                                |  |\____)\___    |
>>>> +                                |  ) _____  __`<  |
>>>> +                                |  |/     )/      |
>>>> +                                |                 |
>>>> +                                |                 |
>>>> +                                |                 |
>>>> +                                |                 |
>>>> +                                |                 |
>>>> +                                +-----------------+
>>>
>>> I had to think a bit to check if the sharks are in the right
>>> orientation, and they are :-) I would however reduce the frame a little
>>> bit to match the size of the references systems above.
>>
>> I received the opposed comments from Andrey and I agree with him that
>> we should try to keep the image proportions similar to the ones above
>> to avoid the idea of cropping. In my view, the coordinate system does
>> not represent the size of the captured image, but I can work to make
>> them look like it.
>>
>>>>  ...
>>>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

Patch

diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
index aaadcbd3e52b..243af7bd0a03 100644
--- a/src/libcamera/property_ids.yaml
+++ b/src/libcamera/property_ids.yaml
@@ -25,4 +25,313 @@  controls:
           description: |
             The camera is attached to the device in a way that allows it to
             be moved freely
+
+  - Rotation:
+      type: int32_t
+      description: |
+        The camera rotation is expressed as the angular difference in degrees
+        between two reference systems, one implicitly defined by the camera
+        module intrinsics characteristics, and one artificially defined on the
+        external world scene to be captured when projected on the image sensor
+        pixel array.
+
+        A camera sensor has an implicitly defined 2-dimensional reference
+        system 'Rc' defined by its pixel array scan-out order, with its origin
+        posed at pixel address (0,0), the x-axis progressing from there towards
+        the last scanned out column of the pixel array and the y-axis
+        progressing towards the last scanned out line.
+
+        A typical example for a sensor with a (2592x1944) pixel array matrix
+        observed from the front is
+
+                   (2592)      x-axis          0
+                      <------------------------+ 0
+                      .......... ... ..........!
+                      .......... ... ..........! y-axis
+                                 ...           !
+                      .......... ... ..........!
+                      .......... ... ..........! (1944)
+                                               V
+
+        The external world scene reference system scene 'Rs' is defined as a
+        2-dimensional reference system on the parallel plane posed in front
+        of the camera module's focal plane, with its origin placed on the
+        visible top-left corner, the x-axis progressing towards the right from
+        there and the y-axis progressing towards the bottom of the visible
+        scene.
+
+        A typical example of a (very common) picture of a shark swimming from
+        left to right is
+
+                                 x-axis
+                   (0,0)---------------------->
+                     !
+                     !
+                     !       |\____)\___
+                     !       ) _____  __`<
+                     !       |/     )/
+                     !
+                     V
+                   y-axis
+
+        With the reference plane posed in front of the camera module and
+        parallel to its focal plane
+
+                                   !
+                                 / !
+                                /  !
+                               /   !
+                        _     /    !
+                     +-/ \-+ /     !
+                     | (o) |       ! 'Rs' reference plane
+                     +-----+ \     !
+                              \    !
+                               \   !
+                                \  !
+                                 \ !
+                                   !
+
+        When projected on the sensor's pixel array, the image and the associated
+        reference system 'Rs' are typically inverted, due to the camera module's
+        lens optical inversion effect.
+
+        Assuming the above represented scene of the swimming shark, the lens
+        inversion projects on the sensor pixel array the reference plane 'Rp'
+
+                  y-axis
+                     ^
+                     !
+                     !       |\_____)\__
+                     !       ) ____  ___.<
+                     !       |/    )/
+                     !
+                     !
+                   (0,0)---------------------->
+                                 x-axis
+
+        The camera rotation property is then defined as the angular difference
+        in counterclockwise direction between the origin of the camera reference
+        system 'Rc', defined by the camera sensor scan-out direction and its
+        mounting position, and the origin of the projected scene reference
+        system 'Rp', result of the optical projection of the scene reference
+        system 'Rs' on the sensor pixel array.
+
+        Examples
+
+        0 degrees camera rotation
+
+                          y-Rp
+                    y-Rc   ^
+                     ^     !
+                     !     !
+                     !     !
+                     !     !
+                     !     !
+                     !     !
+                     !   (0,0)---------------------->
+                     !                 x-Rp
+                   0 +------------------------------------->
+                      0            x-Rc
+
+
+                                     x-Rc          0
+                           <------------------------+ 0
+                                x-Rp                !
+                     <-----------------------(0,0)  !
+                                               !    !
+                                               !    !
+                                               !    !
+                                               !    V
+                                               !  y-Rc
+                                               V
+                                             y-Rp
+
+        90 degrees camera rotation
+
+                      0        y-Rc
+                   0 +----------------------->
+                     !
+                     !    y-Rp
+                     !     ^
+                     !     !
+                     !     !
+                     !     !
+                     !     !
+                     !     !
+                     !     !
+                     !   (0,0)---------------------->
+                     !                 x-Rp
+                     !
+                     V
+                   x-Rc
+
+        180 degrees camera rotation
+
+                                   x-Cr          0
+                         <------------------------+ 0
+                     y-Rp                         !
+                      ^                           !
+                      !                           ! y-Cr
+                      !                           !
+                      !                           !
+                      !                           V
+                      !
+                      !
+                    (0,0)--------------------->
+                                   x-Rp
+
+        270 degrees camera rotation
+
+                      0        y-Rc
+                   0 +----------------------->
+                     !            x-Rp
+                     ! <-----------------------(0,0
+                     !                           !
+                     !                           !
+                     !                           !
+                     !                           !
+                     !                           !
+                     !                           V
+                     !                         y-Rp
+                     !
+                     !
+                     V
+                   x-Rc
+
+
+
+        Example one - Webcam
+
+        A camera module installed on the user facing part of a laptop screen
+        casing used for video calls. The captured images are meant to be
+        displayed in landscape mode (width > height) on the laptop screen.
+
+        The camera is typically mounted 180 degrees rotated to compensate the
+        lens optical inversion effect.
+
+                          y-Rp
+                    y-Rc   ^
+                     ^     !
+                     !     !       |\_____)\__
+                     !     !       ) ____  ___.<
+                     !     !       |/    )/
+                     !     !
+                     !     !
+                     !   (0,0)---------------------->
+                     !                 x-Rp
+                   0 +------------------------------------->
+                      0                x-Rc
+
+        The two reference systems are aligned, the resulting camera rotation is
+        0 degrees, no rotation correction should be applied to the resulting
+        image once captured to memory buffers to correctly display it to users.
+
+                     +--------------------------+
+                     !       |\____)\___        !
+                     !       ) _____  __`<      !
+                     !       |/     )/          !
+                     +--------------------------+
+
+        If the camera module is not mounted 180 degrees rotated to compensate
+        the lens optical inversion, the two reference system will result not
+        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'
+        plane.
+
+                                   x-Rc          0
+                         <------------------------+ 0
+                     y-Rp                         !
+                      ^                           !
+                      !                           ! y-Rc
+                      !      |\_____)\__          !
+                      !      ) ____  ___.<        !
+                      !      |/    )/             V
+                      !
+                      !
+                    (0,0)--------------------->
+                                   x-Rp
+
+        The image once captured to memory will then be 180 degrees rotated
+
+                     +--------------------------+
+                     !          __/(_____/|     !
+                     !        >.___  ____ (     !
+                     !             \(    \|     !
+                     +--------------------------+
+
+        A software rotation correction of 180 degrees should be applied to
+        correctly display the image.
+
+                     +--------------------------+
+                     !       |\____)\___        !
+                     !       ) _____  __`<      !
+                     !       |/     )/          !
+                     +--------------------------+
+
+        Example two - Phone camera
+
+        A camera installed on the back-side of a mobile device facing away from
+        the user. The captured images are meant to be displayed in portrait mode
+        (height > width) to match the device screen orientation and the device
+        usage orientation used when taking the picture.
+
+        The camera is typically mounted with its pixel array longer side aligned
+        to the device longer side, 180 degrees rotated to compensate the lens
+        optical inversion effect.
+
+                      0        y-Rc
+                   0 +----------------------->
+                     !
+                     !    y-Rp
+                     !     ^
+                     !     !
+                     !     !       |\_____)\__
+                     !     !       ) ____  ___.<
+                     !     !       |/    )/
+                     !     !
+                     !     !
+                     !   (0,0)---------------------->
+                     !                 x-Rp
+                     !
+                     !
+                     V
+                   x-Rc
+
+        The two reference systems are not aligned and the 'Rp' reference
+        system is 90 degrees rotated in counterclockwise direction in respect
+        to the 'Rc' reference system.
+
+        The image, when captured to memory buffer will be rotated.
+
+                     +---------------------------------------+
+                     |                  _ _                  |
+                     |                 \   /                 |
+                     |                  | |                  |
+                     |                  | |                  |
+                     |                  |  >                 |
+                     |                 <  |                  |
+                     |                  | |                  |
+                     |                    .                  |
+                     |                   V                   |
+                     +---------------------------------------+
+
+        A correction of 90 degrees in counterclockwise direction has to be
+        applied to correctly display the image in portrait mode on the device
+        screen.
+
+                                +-----------------+
+                                |                 |
+                                |                 |
+                                |                 |
+                                |                 |
+                                |                 |
+                                |                 |
+                                |  |\____)\___    |
+                                |  ) _____  __`<  |
+                                |  |/     )/      |
+                                |                 |
+                                |                 |
+                                |                 |
+                                |                 |
+                                |                 |
+                                +-----------------+
 ...