sensor: camera_sensor_legacy: Support separate H/V flips

27526 diff mbox series

CameraSensorLegacy supports flipping on the sensor side only if the
driver supports V4L2_CID_HFLIP and V4L2_CID_VFLIP. In cases where a
driver only supports a single flip mode, flipping support gets disabled
completely. This is unexpected and annoying when validating sensor
drivers.

Fix that by handling vertical and horizontal flips separately.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
Hi all,

I had this patch somewhere in my tree for a long time. Now I stumbled
over this issue again and had to manually rebase the changes. So maybe
it is time to upstream :-)

Best regards,
Stefan

 src/libcamera/sensor/camera_sensor_legacy.cpp | 72 +++++++++++++------
 1 file changed, 52 insertions(+), 20 deletions(-)

Hi

2026. 07. 31. 16:37 keltezéssel, Stefan Klug írta:
> CameraSensorLegacy supports flipping on the sensor side only if the
> driver supports V4L2_CID_HFLIP and V4L2_CID_VFLIP. In cases where a
> driver only supports a single flip mode, flipping support gets disabled
> completely. This is unexpected and annoying when validating sensor
> drivers.

What about `CameraSensorRaw`?


> 
> Fix that by handling vertical and horizontal flips separately.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
> Hi all,
> 
> I had this patch somewhere in my tree for a long time. Now I stumbled
> over this issue again and had to manually rebase the changes. So maybe
> it is time to upstream :-)
> 
> Best regards,
> Stefan
> 
>   src/libcamera/sensor/camera_sensor_legacy.cpp | 72 +++++++++++++------
>   1 file changed, 52 insertions(+), 20 deletions(-)
> 
> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
> index 6a683821f219..0f8e3c8cf251 100644
> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp
> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
> @@ -133,7 +133,8 @@ private:
>   	Size pixelArraySize_;
>   	Rectangle activeArea_;
>   	const BayerFormat *bayerFormat_;
> -	bool supportFlips_;
> +	bool supportHFlips_;
> +	bool supportVFlips_;
>   	bool flipsAlterBayerOrder_;
>   	Orientation mountingOrientation_;
>   
> @@ -153,7 +154,7 @@ private:
>   
>   CameraSensorLegacy::CameraSensorLegacy(const MediaEntity *entity)
>   	: entity_(entity), pad_(UINT_MAX), staticProps_(nullptr),
> -	  bayerFormat_(nullptr), supportFlips_(false),
> +	  bayerFormat_(nullptr), supportHFlips_(false), supportVFlips_(false),
>   	  flipsAlterBayerOrder_(false), properties_(properties::properties)
>   {
>   }
> @@ -365,19 +366,37 @@ int CameraSensorLegacy::validateSensorDriver()
>   	 * \todo Handle horizontal and vertical flips independently.
>   	 */
>   	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 && !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) &&
> -	    vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
> -		supportFlips_ = true;
> +	if (hflipInfo && !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
> +		supportHFlips_ = true;
>   
> -		if (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT ||
> -		    vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT)
> +		if (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT)
>   			flipsAlterBayerOrder_ = true;
> +	} else {
> +		LOG(CameraSensor, Debug)
> +			<< "Camera sensor does not support horizontal flip";
>   	}
>   
> -	if (!supportFlips_)
> +	const struct v4l2_query_ext_ctrl *vflipInfo = subdev_->controlInfo(V4L2_CID_VFLIP);
> +	if (vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {
> +		supportVFlips_ = true;
> +
> +		if (vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT)
> +			flipsAlterBayerOrder_ = true;
> +	} else {
>   		LOG(CameraSensor, Debug)
> -			<< "Camera sensor does not support horizontal/vertical flip";
> +			<< "Camera sensor does not support vertical flip";
> +	}
> +
> +	if (supportHFlips_ && supportVFlips_ &&
> +	    ((vflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT) !=
> +	     (hflipInfo->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT))) {
> +		LOG(CameraSensor, Error)
> +			<< "Flips have different alter bayer order properties."
> +			<< " Disable flipping altogether";

This is theoretically possible, right? Should it be a warning with
an "unsupported" somewhere in the message?


> +		supportHFlips_ = false;
> +		supportVFlips_ = false;
> +		flipsAlterBayerOrder_ = false;
> +	}
>   
>   	/*
>   	 * Make sure the required selection targets are supported.
> @@ -759,11 +778,20 @@ CameraSensorLegacy::getFormat(Span<const unsigned int> mbusCodes,
>   int CameraSensorLegacy::setFormat(V4L2SubdeviceFormat *format, Transform transform)
>   {
>   	/* Configure flips if the sensor supports that. */
> -	if (supportFlips_) {
> +	if (supportHFlips_) {
>   		ControlList flipCtrls(subdev_->controls());
>   
>   		flipCtrls.set(V4L2_CID_HFLIP,
>   			      static_cast<int32_t>(!!(transform & Transform::HFlip)));
> +
> +		int ret = subdev_->setControls(&flipCtrls);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (supportVFlips_) {
> +		ControlList flipCtrls(subdev_->controls());
> +
>   		flipCtrls.set(V4L2_CID_VFLIP,
>   			      static_cast<int32_t>(!!(transform & Transform::VFlip)));
>   
> @@ -936,15 +964,6 @@ int CameraSensorLegacy::sensorInfo(IPACameraSensorInfo *info) const
>   
>   Transform CameraSensorLegacy::computeTransform(Orientation *orientation) const
>   {
> -	/*
> -	 * If we cannot do any flips we cannot change the native camera mounting
> -	 * orientation.
> -	 */
> -	if (!supportFlips_) {
> -		*orientation = mountingOrientation_;
> -		return Transform::Identity;
> -	}
> -
>   	/*
>   	 * Now compute the required transform to obtain 'orientation' starting
>   	 * from the mounting rotation.
> @@ -955,6 +974,19 @@ Transform CameraSensorLegacy::computeTransform(Orientation *orientation) const
>   	 */
>   	Transform transform = *orientation / mountingOrientation_;
>   
> +	/*
> +	 * If we cannot do the required flips, fall back to identity.
> +	 */
> +	if (!supportHFlips_ & !!(transform & Transform::HFlip)) {

I would write

   if (!supportHFlips_ && (transform & Transform::HFlip))


> +		*orientation = mountingOrientation_;
> +		return Transform::Identity;
> +	}
> +
> +	if (!supportVFlips_ & !!(transform & Transform::VFlip)) {

Similarly here.


> +		*orientation = mountingOrientation_;
> +		return Transform::Identity;
> +	}
> +
>   	/*
>   	 * If transform contains any Transpose we cannot do it, so adjust
>   	 * 'orientation' to report the image native orientation and return Identity.