[libcamera-devel] [RFC PATCH 2/2] libcamera: camera_sensor: Do not clear camera flips when listing formats

[Dave Stevenson]

Hi David

On Thu, 3 Nov 2022 at 10:40, David Plowman via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Previously the code used to clear the camnera's h and v flip bits when
> enumerating the supported formats so as to obtain any Bayer formats in
> the sensor's native (untransformed) orientation. However this fails
> when the camera is already in use elsewhere.
>
> Instead, we query the current state of the flip bits and transform the
> formats - which we obtain in their flipped orientation - back into
> their native orientation to be stored.

I don't believe that libcamera knows for definite whether the sensor
changes the Bayer order or not. Several of the OnSemi sensors I have
seen do not change, presumably as they shift their crop by 1 pixel.

The original comment mentioned that scenario:
"This is harmless for sensors where the flips don't affect the Bayer order"

  Dave

> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/libcamera/camera_sensor.cpp | 51 +++++++++++++++++++++++++++------
>  1 file changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 572a313a..6670dfb9 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -19,6 +19,8 @@
>
>  #include <libcamera/base/utils.h>
>
> +#include <libcamera/transform.h>
> +
>  #include "libcamera/internal/bayer_format.h"
>  #include "libcamera/internal/camera_lens.h"
>  #include "libcamera/internal/camera_sensor_properties.h"
> @@ -108,18 +110,51 @@ int CameraSensor::init()
>                 return ret;
>
>         /*
> -        * Clear any flips to be sure we get the "native" Bayer order. This is
> -        * harmless for sensors where the flips don't affect the Bayer order.
> +        * We want to get the native mbus codes for the sensor, without any flips.
> +        * We can't clear any flips here, so we have to read the current values
> +        * (if the flip controls exist), decide whether they actually modify any
> +        * output Bayer pattern, and finally undo their effect on the formats.
> +        *
> +        * First, check if the flip controls exist and if so read them.
>          */
>         ControlList ctrls(subdev_->controls());
> -       if (subdev_->controls().find(V4L2_CID_HFLIP) != subdev_->controls().end())
> -               ctrls.set(V4L2_CID_HFLIP, 0);
> -       if (subdev_->controls().find(V4L2_CID_VFLIP) != subdev_->controls().end())
> -               ctrls.set(V4L2_CID_VFLIP, 0);
> -       subdev_->setControls(&ctrls);
> +       std::vector<uint32_t> flipCtrlIds;
> +       bool hasHflip = subdev_->controls().find(V4L2_CID_HFLIP) != subdev_->controls().end();
> +       bool hasVflip = subdev_->controls().find(V4L2_CID_VFLIP) != subdev_->controls().end();
> +       if (hasHflip)
> +               flipCtrlIds.push_back(V4L2_CID_HFLIP);
> +       if (hasVflip)
> +               flipCtrlIds.push_back(V4L2_CID_VFLIP);
> +       ControlList flipCtrls = subdev_->getControls(flipCtrlIds);
> +
> +       /* Now construct a transform that would undo any flips. */
> +       Transform transform = Transform::Identity;
> +       if (hasHflip && flipCtrls.get(V4L2_CID_HFLIP).get<int>()) {
> +               const struct v4l2_query_ext_ctrl *extCtrl = subdev_->controlInfo(V4L2_CID_HFLIP);
> +               if (extCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT)
> +                       transform |= Transform::HFlip;
> +       }
> +       if (hasVflip && flipCtrls.get(V4L2_CID_VFLIP).get<int>()) {
> +               const struct v4l2_query_ext_ctrl *extCtrl = subdev_->controlInfo(V4L2_CID_VFLIP);
> +               if (extCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT)
> +                       transform |= Transform::VFlip;
> +       }
> +
> +       /* Finally get the formats, and apply the transform to the mbus codes. */
> +       auto formats = subdev_->formats(pad_);
> +       for (const auto &format : formats) {
> +               unsigned int mbusCode = format.first;
> +               BayerFormat bayerFormat = BayerFormat::fromMbusCode(mbusCode);
> +               bool valid = true;
> +
> +               if (bayerFormat.isValid())
> +                       mbusCode = bayerFormat.transform(transform).toMbusCode(valid);
> +
> +               if (valid)
> +                       formats_[mbusCode] = std::move(format.second);
> +       }
>
>         /* Enumerate, sort and cache media bus codes and sizes. */
> -       formats_ = subdev_->formats(pad_);
>         if (formats_.empty()) {
>                 LOG(CameraSensor, Error) << "No image format found";
>                 return -EINVAL;
> --
> 2.30.2
>