| Message ID | 20221110144556.7858-3-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Dave On Thu, Nov 10, 2022 at 02:45:52PM +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@raspberrypi.com> As discussed on the RFC, this seems correct to me! Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > src/libcamera/camera_sensor.cpp | 49 ++++++++++++++++++++++++++------- > 1 file changed, 39 insertions(+), 10 deletions(-) > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 572a313a..cbac9e78 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,45 @@ 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; > + 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; > + > + /* 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); > + > + if (bayerFormat.isValid()) > + mbusCode = bayerFormat.transform(transform).toMbusCode(); > + > + if (mbusCode) > + 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; > @@ -189,7 +218,7 @@ int CameraSensor::init() > * \todo The control API ought to have a flag to specify if a control > * is read-only which could be used below. > */ > - const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK); > + const ControlInfo hblank = subdev_->controls().at(V4L2_CID_HBLANK); > const int32_t hblankMin = hblank.min().get<int32_t>(); > const int32_t hblankMax = hblank.max().get<int32_t>(); > > -- > 2.30.2 >
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 572a313a..cbac9e78 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,45 @@ 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; + 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; + + /* 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); + + if (bayerFormat.isValid()) + mbusCode = bayerFormat.transform(transform).toMbusCode(); + + if (mbusCode) + 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; @@ -189,7 +218,7 @@ int CameraSensor::init() * \todo The control API ought to have a flag to specify if a control * is read-only which could be used below. */ - const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK); + const ControlInfo hblank = subdev_->controls().at(V4L2_CID_HBLANK); const int32_t hblankMin = hblank.min().get<int32_t>(); const int32_t hblankMax = hblank.max().get<int32_t>();
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@raspberrypi.com> --- src/libcamera/camera_sensor.cpp | 49 ++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 10 deletions(-)