[libcamera-devel] [PATCH] libcamera: camera_sensor: Adjust properties::Rotation
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Mon Jun 19 14:20:36 CEST 2023
Hi Laurent
on top eventually as the patch has been pushed (by me) :)
On Mon, Jun 19, 2023 at 03:00:51PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Jun 14, 2023 at 12:59:57PM +0200, Jacopo Mondi via libcamera-devel wrote:
> > As the CameraSensor::validateTransform() function compensate
> > for the sensor's mounting rotation, the properties::Rotation value
> > should be adjusted to make sure application that receive already
> > "corrected" images do not get confused by Rotation still reporting
> > a value.
> >
> > However, as an image sensor can only compensate rotations by applying
> > H/V flips, only correct Rotation when the mounting rotation is 180
> > degrees.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > ---
> > src/libcamera/camera.cpp | 3 +--
> > src/libcamera/camera_sensor.cpp | 21 ++++++++++++++++++---
> > 2 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 99683e498697..3e252f3b8f8d 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -397,8 +397,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF
> > *
> > * The transform is a user-specified 2D plane transform that will be applied
> > * to the camera images by the processing pipeline before being handed to
> > - * the application. This is subsequent to any transform that is already
> > - * required to fix up any platform-defined rotation.
> > + * the application.
> > *
> > * The usual 2D plane transforms are allowed here (horizontal/vertical
> > * flips, multiple of 90-degree rotations etc.), but the validate() function
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index 60bf87b49e6e..f3a5aa37149f 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -461,7 +461,17 @@ int CameraSensor::initProperties()
> >
> > const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);
> > if (rotationControl != controls.end()) {
> > + /*
> > + * 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.
> > + */
> > propertyValue = rotationControl->second.def().get<int32_t>();
> > + if (propertyValue == 180 && supportFlips_)
> > + propertyValue = 0;
> > properties_.set(properties::Rotation, propertyValue);
> > }
> >
> > @@ -1028,10 +1038,15 @@ void CameraSensor::updateControlInfo()
> > */
> > Transform CameraSensor::validateTransform(Transform *transform) const
> > {
> > - /* Adjust the requested transform to compensate the sensor rotation. */
> > - int32_t rotation = properties().get(properties::Rotation).value_or(0);
> > - bool success;
> > + /* Adjust the requested transform to compensate the sensor mounting rotation. */
> > + const ControlInfoMap &controls = subdev_->controls();
> > + int rotation = 0;
> >
> > + const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);
>
> How about caching the rotation value in the CameraSensor class instead
> of looking it up every time ? You could actually even cache the value of
> rotationTransform, simplifying the code in this function.
>
> > + if (rotationControl != controls.end())
> > + rotation = rotationControl->second.def().get<int32_t>();
> > +
> > + bool success;
> > Transform rotationTransform = transformFromRotation(rotation, &success);
> > if (!success)
> > LOG(CameraSensor, Warning) << "Invalid rotation of " << rotation
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list