[libcamera-devel] [RFC PATCH 2/2] libcamera: camera_sensor: Do not clear camera flips when listing formats
David Plowman
david.plowman at raspberrypi.com
Fri Nov 4 13:57:11 CET 2022
Hi Jacopo
Thanks for this suggestion!
On Fri, 4 Nov 2022 at 12:17, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi David
>
> On Thu, Nov 03, 2022 at 10:40:27AM +0000, David Plowman via libcamera-devel 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.
> >
> > 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;
> > + }
>
> Might avoid a few lookups with:
>
> 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)
> flipCtrlIds.push_back(V4L2_CID_HFLIP);
> if (vflipInfo)
> 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 (hflipInfo && flipCtrls.get(V4L2_CID_HFLIP).get<int>() &&
> (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT))
> transform |= Transform::HFlip;
> if (vflipInfo && flipCtrls.get(V4L2_CID_VFLIP).get<int>() &&
> (vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT))
> transform |= Transform::VFlip;
>
>
> Tested with a sensor without flips and with flips but not flag
Yes, I think that looks a bit nicer, doesn't it? I'll post a v2 with
that update shortly.
Thanks!
David
>
>
> > +
> > + /* 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
> >
More information about the libcamera-devel
mailing list