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

[Naushir Patuck]

On Thu, 3 Nov 2022 at 11:02, Dave Stevenson via libcamera-devel <
libcamera-devel at lists.libcamera.org> wrote:

> 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.
>

There is a flag associated with the H/V flips that can tell us if the
sensor changes the bayer order:

const struct v4l2_query_ext_ctrl *hflipCtrl =
sensor->controlInfo(V4L2_CID_HFLIP);
data->flipsAlterBayerOrder_ = hflipCtrl->flags &
V4L2_CTRL_FLAG_MODIFY_LAYOUT;

This ought to help here.


> 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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221103/e09f7c33/attachment.htm>