[libcamera-devel] [PATCH v2 4/9] libcamera: properties: Make 'Rotation' the mounting rotation

[David Plowman]

Hi Jacopo

Thanks for the patch!

On Fri, 14 Jul 2023 at 15:16, Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Specify in the documentation that properties::Rotation specifies the
> mounting rotation of the camera module. This avoids confusion with the
> image orientation which is instead expressed by
> CameraConfiguration::orientation.
>
> For this reason, do not compensate the Rotation property when
> initializing the CameraSensor class but report the value of
> V4L2_CID_CAMERA_SENSOR_ROTATION or 0 if the control is not available.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Looks good to me!

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Thanks
David

> ---
>  src/libcamera/camera_sensor.cpp | 11 +----------
>  src/libcamera/property_ids.yaml |  8 ++++----
>  2 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 9a033459742f..3ba364c44a40 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -476,20 +476,11 @@ int CameraSensor::initProperties()
>                         rotationTransform_ = Transform::Identity;
>                 }
>
> -               /*
> -                * Adjust property::Rotation as validateTransform() compensates
> -                * for the mounting rotation. However, as a camera sensor can
> -                * only compensate rotations by applying H/VFlips, only rotation
> -                * of 180 degrees are automatically compensated. The other valid
> -                * rotations (Rot90 and Rot270) require transposition, which the
> -                * camera sensor cannot perform, so leave them untouched.
> -                */
> -               if (propertyValue == 180 && supportFlips_)
> -                       propertyValue = 0;
>                 properties_.set(properties::Rotation, propertyValue);
>         } else {
>                 LOG(CameraSensor, Warning)
>                         << "Rotation control not available, default to 0 degrees";
> +               properties_.set(properties::Rotation, propertyValue);
>                 rotationTransform_ = Transform::Identity;
>         }
>
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 5bddafc29364..fb53081c1bd2 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -29,10 +29,10 @@ controls:
>    - Rotation:
>        type: int32_t
>        description: |
> -        The camera rotation is expressed as the angular difference in degrees
> -        between two reference systems, one relative to the camera module, and
> -        one defined on the external world scene to be captured when projected
> -        on the image sensor pixel array.
> +        The camera physical mounting rotation, expressed as the angular
> +        difference in degrees between two reference systems, one relative to the
> +        camera module, and one defined on the external world scene to be
> +        captured when projected on the image sensor pixel array.
>
>          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
> --
> 2.40.1
>