[libcamera-devel] [PATCH v5 6/8] libcamera: raspberrypi: Set camera flips correctly from user transform

[David Plowman]

The Raspberry Pi pipeline handler allows all transforms except those
involving a transpose. The user transform is combined with any
inherent rotation of the camera, and the camera's H and V flip bits
are set accordingly.

Note that the validate() method has to work out what the final Bayer
order of any raw streams will be, before configure() actually applies
the transform to the sensor.

Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
---
 .../pipeline/raspberrypi/raspberrypi.cpp      | 82 ++++++++++++++++---
 1 file changed, 71 insertions(+), 11 deletions(-)

diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index c554532..333aa94 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -294,7 +294,7 @@ public:
 	void frameStarted(uint32_t sequence);
 
 	int loadIPA();
-	int configureIPA();
+	int configureIPA(CameraConfiguration *config);
 
 	void queueFrameAction(unsigned int frame, const IPAOperationData &action);
 
@@ -400,8 +400,34 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
 	if (config_.empty())
 		return Invalid;
 
-	if (transform != Transform::Identity) {
-		transform = Transform::Identity;
+	/*
+	 * What if the platform has a non-90 degree rotation? We can't even
+	 * "adjust" the configuration and carry on. Alternatively, raising an
+	 * error means the platform can never run. Let's just print a warning
+	 * and continue regardless; the rotation is effectively set to zero.
+	 */
+	int32_t rotation = data_->sensor_->properties().get(properties::Rotation);
+	bool success;
+	Transform combined = transform * transformFromRotation(rotation, &success);
+	if (!success)
+		LOG(RPI, Warning) << "Invalid rotation of " << rotation
+				  << " degrees - ignoring";
+
+	/*
+	 * We combine the platform and user transform, but must "adjust away"
+	 * any combined result that includes a transform, as we can't do those.
+	 * In this case, flipping only the transpose bit is helpful to
+	 * applications - they either get the transform they requested, or have
+	 * to do a simple transpose themselves (they don't have to worry about
+	 * the other possible cases).
+	 */
+	if (!!(combined & Transform::Transpose)) {
+		/*
+		 * Flipping the transpose bit in "transform" flips it in
+		 * combined result too (as it's the last thing that happens).
+		 */
+		transform ^= Transform::Transpose;
+		combined ^= Transform::Transpose;
 		status = Adjusted;
 	}
 
@@ -414,13 +440,42 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
 			 * Calculate the best sensor mode we can use based on
 			 * the user request.
 			 */
-			V4L2VideoDevice::Formats fmts = data_->unicam_[Unicam::Image].dev()->formats();
+			V4L2VideoDevice *dev = data_->unicam_[Unicam::Image].dev();
+			V4L2VideoDevice::Formats fmts = dev->formats();
 			V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
-			int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&sensorFormat);
+			int ret = dev->tryFormat(&sensorFormat);
 			if (ret)
 				return Invalid;
 
-			PixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();
+			/*
+			 * Some sensors change their Bayer order when they are
+			 * h-flipped or v-flipped, according to the transform.
+			 * If the controls own up to "modifying the layout" we
+			 * will assume that's what is going on and advertise
+			 * the transformed Bayer order in the stream.
+			 */
+			V4L2PixelFormat fourcc = sensorFormat.fourcc;
+
+			/*
+			 * The camera may already be transformed, so we must
+			 * only transform the fourcc if the new transform is
+			 * different.
+			 */
+			ControlList ctrls = dev->getControls({ V4L2_CID_HFLIP, V4L2_CID_VFLIP });
+
+			const struct v4l2_query_ext_ctrl *hflipCtrl = dev->queryControl(V4L2_CID_HFLIP);
+			if (hflipCtrl &&
+			    (hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT) &&
+			    ctrls.get(V4L2_CID_HFLIP).get<int32_t>() != !!(combined & Transform::HFlip))
+				fourcc = fourcc.transform(Transform::HFlip);
+
+			const struct v4l2_query_ext_ctrl *vflipCtrl = dev->queryControl(V4L2_CID_VFLIP);
+			if (vflipCtrl &&
+			    (vflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT) &&
+			    ctrls.get(V4L2_CID_VFLIP).get<int32_t>() != !!(combined & Transform::VFlip))
+				fourcc = fourcc.transform(Transform::VFlip);
+
+			PixelFormat sensorPixFormat = fourcc.toPixelFormat();
 			if (cfg.size != sensorFormat.size ||
 			    cfg.pixelFormat != sensorPixFormat) {
 				cfg.size = sensorFormat.size;
@@ -756,7 +811,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
 	crop.y = (sensorFormat.size.height - crop.height) >> 1;
 	data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);
 
-	ret = data->configureIPA();
+	ret = data->configureIPA(config);
 	if (ret)
 		LOG(RPI, Error) << "Failed to configure the IPA: " << ret;
 
@@ -1112,7 +1167,7 @@ int RPiCameraData::loadIPA()
 	return ipa_->init(settings);
 }
 
-int RPiCameraData::configureIPA()
+int RPiCameraData::configureIPA(CameraConfiguration *config)
 {
 	std::map<unsigned int, IPAStream> streamConfig;
 	std::map<unsigned int, const ControlInfoMap &> entityControls;
@@ -1170,11 +1225,16 @@ int RPiCameraData::configureIPA()
 			sensorMetadata_ = result.data[2];
 		}
 
-		/* Configure the H/V flip controls based on the sensor rotation. */
+		/* 
+		 * Configure the H/V flip controls based on the sensor rotation
+		 * and user transform.
+		 */
 		ControlList ctrls(unicam_[Unicam::Image].dev()->controls());
 		int32_t rotation = sensor_->properties().get(properties::Rotation);
-		ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation));
-		ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation));
+		/* The rotation angle was already checked in validate(). */
+		Transform combined = config->transform * transformFromRotation(rotation);
+		ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!(combined & Transform::HFlip)));
+		ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!(combined & Transform::VFlip)));
 		unicam_[Unicam::Image].dev()->setControls(&ctrls);
 	}
 
-- 
2.20.1