[libcamera-devel] [PATCH 4/9] libcamera: camera_sensor: Verify flips support

[David Plowman]

Hi Jacopo

Thanks for the patch.

On Thu, 24 Nov 2022 at 12:12, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> During the camera sensor driver validation, verify if the sensor
> supports horizontal and vertical flips and store a flag as
> CameraSensor::supportFlips_ class member.
>
> The flag will be later inspected when applying flips.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

I'm certainly fine with this, though I should perhaps add that when we
originally did this in the Pi PH we discussed a little bit whether a
camera might support H flips but not V, or vice versa. We slightly
passed over the question because it was "only the Pi", but maybe we
want to re-examine that?

There's also the question as to whether the flips modify the Bayer
order. Don't know if that's a thing we want to store here? (And
separately for H and V flips?)

But as I said previously, this all works for me, so:

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

Thanks!
David

> ---
>  include/libcamera/internal/camera_sensor.h |  1 +
>  src/libcamera/camera_sensor.cpp            | 15 ++++++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index b9f4d7867854..878f3c28a3c9 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -101,6 +101,7 @@ private:
>         Size pixelArraySize_;
>         Rectangle activeArea_;
>         const BayerFormat *bayerFormat_;
> +       bool supportFlips_;
>
>         ControlList properties_;
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index cbac9e7801ae..3afcbc482095 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -57,7 +57,8 @@ LOG_DEFINE_CATEGORY(CameraSensor)
>   */
>  CameraSensor::CameraSensor(const MediaEntity *entity)
>         : entity_(entity), pad_(UINT_MAX), staticProps_(nullptr),
> -         bayerFormat_(nullptr), properties_(properties::properties)
> +         bayerFormat_(nullptr), supportFlips_(false),
> +         properties_(properties::properties)
>  {
>  }
>
> @@ -271,6 +272,18 @@ int CameraSensor::validateSensorDriver()
>                 }
>         }
>
> +       /* Verify if sensor supports horizontal/vertical flips. */
> +       const struct v4l2_query_ext_ctrl *hflipInfo = subdev_->controlInfo(V4L2_CID_HFLIP);
> +       const struct v4l2_query_ext_ctrl *vflipInfo = subdev_->controlInfo(V4L2_CID_VFLIP);
> +       if (hflipInfo && vflipInfo &&
> +           !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) &&
> +           !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY))
> +               supportFlips_ = true;
> +
> +       if (!supportFlips_)
> +               LOG(CameraSensor, Warning)
> +                       << "Camera sensor does not support horizontal/vertical flip";
> +
>         /*
>          * Make sure the required selection targets are supported.
>          *
> --
> 2.38.1
>