[{"id":27397,"web_url":"https://patchwork.libcamera.org/comment/27397/","msgid":"<8794a76f-01b7-c229-4295-042fc9c12a83@ideasonboard.com>","date":"2023-06-21T06:50:45","subject":"Re: [libcamera-devel] [RFC 3/4] libcamera: Move to use\n\tCameraConfiguration::orientation","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nthank you for the patch.\n\nOn 6/20/23 7:59 PM, Jacopo Mondi via libcamera-devel wrote:\n> Replace the usage of CameraConfiguration::transform with the newly\n> introduced CameraConfiguration::orientation.\n>\n> Rework the CameraSensor::validateTransform(transform) to\n> CameraSensor::computeTransform(orientation), that given the desired\n> image orientation computes the Transform that pipeline handlers should\n> apply to the sensor to obtain it.\n>\n> Introduce in transform.cpp two functions to convert from Transform to\n> Orientation and vice-versa.\n>\n> Port all pipeline handlers to use the newly introduced function.\n>\n> This commit breaks existing applications as it removes the public\n> CameraConfiguration::transform in faviour of\n> CameraConfiguration::orientation.\n>\n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>   include/libcamera/camera.h                    |  2 -\n>   include/libcamera/internal/camera_sensor.h    |  3 +-\n>   include/libcamera/transform.h                 |  4 +\n>   src/libcamera/camera.cpp                      | 15 +--\n>   src/libcamera/camera_sensor.cpp               | 95 +++++++++----------\n>   src/libcamera/pipeline/ipu3/ipu3.cpp          |  6 +-\n>   src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  8 +-\n>   .../pipeline/rpi/common/pipeline_base.cpp     |  9 +-\n>   src/libcamera/pipeline/simple/simple.cpp      |  6 +-\n>   src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  4 +-\n>   src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n>   src/libcamera/transform.cpp                   | 58 +++++++++++\n>   12 files changed, 129 insertions(+), 85 deletions(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 608774ce7768..bffac6bc8922 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -20,7 +20,6 @@\n>   #include <libcamera/controls.h>\n>   #include <libcamera/request.h>\n>   #include <libcamera/stream.h>\n> -#include <libcamera/transform.h>\n>   \n>   namespace libcamera {\n>   \n> @@ -76,7 +75,6 @@ public:\n>   \tbool empty() const;\n>   \tstd::size_t size() const;\n>   \n> -\tTransform transform;\n>   \tOrientation orientation;\n>   \n>   protected:\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 02b4b4d25e6d..1d47a2b1a500 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -14,6 +14,7 @@\n>   #include <libcamera/base/class.h>\n>   #include <libcamera/base/log.h>\n>   \n> +#include <libcamera/camera.h>\n>   #include <libcamera/control_ids.h>\n>   #include <libcamera/controls.h>\n>   #include <libcamera/geometry.h>\n> @@ -71,7 +72,7 @@ public:\n>   \n>   \tCameraLens *focusLens() { return focusLens_.get(); }\n>   \n> -\tTransform validateTransform(Transform *transform) const;\n> +\tTransform computeTransform(CameraConfiguration::Orientation *orientation) const;\n>   \n>   protected:\n>   \tstd::string logPrefix() const override;\n> diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h\n> index 2a6838c78369..573adf18715d 100644\n> --- a/include/libcamera/transform.h\n> +++ b/include/libcamera/transform.h\n> @@ -9,6 +9,8 @@\n>   \n>   #include <string>\n>   \n> +#include <libcamera/camera.h>\n> +\n>   namespace libcamera {\n>   \n>   enum class Transform : int {\n> @@ -69,6 +71,8 @@ constexpr Transform operator~(Transform t)\n>   }\n>   \n>   Transform transformFromRotation(int angle, bool *success = nullptr);\n> +Transform transformFromOrientation(const CameraConfiguration::Orientation &orientation);\n> +CameraConfiguration::Orientation transformToOrientation(const Transform &transform);\n>   \n>   const char *transformToString(Transform t);\n>   \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 5ca05719ebfc..bc1409b5c960 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -197,7 +197,7 @@ LOG_DECLARE_CATEGORY(Camera)\n>    * \\brief Create an empty camera configuration\n>    */\n>   CameraConfiguration::CameraConfiguration()\n> -\t: transform(Transform::Identity), orientation(rotate0), config_({})\n> +\t: orientation(rotate0), config_({})\n>   {\n>   }\n>   \n> @@ -428,19 +428,6 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n>   \treturn status;\n>   }\n>   \n> -/**\n> - * \\var CameraConfiguration::transform\n> - * \\brief User-specified transform to be applied to the image\n> - *\n> - * The transform is a user-specified 2D plane transform that will be applied\n> - * to the camera images by the processing pipeline before being handed to\n> - * the application.\n> - *\n> - * The usual 2D plane transforms are allowed here (horizontal/vertical\n> - * flips, multiple of 90-degree rotations etc.), but the validate() function\n> - * may adjust this field at its discretion if the selection is not supported.\n> - */\n> -\n>   /**\n>    * \\var CameraConfiguration::orientation\n>    * \\brief The desired orientation of the video streams produced by the camera\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index a15a6c89c5c8..5c9f30a62527 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -465,7 +465,7 @@ int CameraSensor::initProperties()\n>   \n>   \t\t/*\n>   \t\t * Cache the Transform associated with the camera mounting\n> -\t\t * rotation for later use in validateTransform().\n> +\t\t * rotation for later use in computeTransform().\n>   \t\t */\n>   \t\tbool success;\n>   \t\trotationTransform_ = transformFromRotation(propertyValue, &success);\n> @@ -477,7 +477,7 @@ int CameraSensor::initProperties()\n>   \t\t}\n>   \n>   \t\t/*\n> -\t\t * Adjust property::Rotation as validateTransform() compensates\n> +\t\t * Adjust property::Rotation as computeTransform() compensates\n>   \t\t * for the mounting rotation. However, as a camera sensor can\n>   \t\t * only compensate rotations by applying H/VFlips, only rotation\n>   \t\t * of 180 degrees are automatically compensated. The other valid\n> @@ -1033,69 +1033,66 @@ void CameraSensor::updateControlInfo()\n>    */\n>   \n>   /**\n> - * \\brief Validate a transform request against the sensor capabilities\n> - * \\param[inout] transform The requested transformation, updated to match\n> - * the sensor capabilities\n> + * \\brief Compute the Transform that gives the requested \\a orientation\n> + * \\param[inout] orientation The desired image orientation\n>    *\n> - * The input \\a transform is the transform that the caller wants, and it is\n> - * adjusted according to the capabilities of the sensor to represent the\n> - * \"nearest\" transform that can actually be delivered.\n> + * This function computes the Transform that the pipeline handler should apply\n> + * to the CameraSensor to obtain the requested \\a orientation.\n>    *\n> - * The returned Transform is the transform applied to the sensor in order to\n> - * produce the input \\a transform, It is also validated against the sensor's\n> - * ability to perform horizontal and vertical flips.\n> + * The intended caller of this function is the validate() implementation of\n> + * pipeline handlers, that pass in the application requested\n> + * CameraConfiguration::orientation and obtain a Transform to apply to the\n> + * camera sensor, likely at configure() time.\n>    *\n> - * For example, if the requested \\a transform is Transform::Identity and the\n> - * sensor rotation is 180 degrees, the output transform will be\n> - * Transform::Rot180 to correct the images so that they appear to have\n> - * Transform::Identity, but only if the sensor can apply horizontal and vertical\n> - * flips.\n> + * If the requested \\a orientation cannot be obtained, the \\a orientation\n> + * parameter is adjusted to report what the current image orientation is and\n> + * Transform::Identity is returned.\n>    *\n> - * \\return A Transform instance that represents which transformation has been\n> - * applied to the camera sensor\n> + * If the requested \\a orientation can be obtained, the function computes a\n> + * Transform and does not adjust \\a orientation.\n> + *\n> + * Pipeline handlers are expected to verify if \\a orientation has been\n> + * adjusted by this function and set the CameraConfiguration::status to\n> + * Adjusted accordingly.\n> + *\n> + * \\return A Transform instance that applied to the CameraSensor produces images\n> + * with \\a orientation\n>    */\n> -Transform CameraSensor::validateTransform(Transform *transform) const\n> +Transform CameraSensor::computeTransform(CameraConfiguration::Orientation *orientation) const\n>   {\n>   \t/*\n> -\t * Combine the requested transform to compensate the sensor mounting\n> -\t * rotation.\n> +\t * We cannot do any flips, we cannot change the native camera mounting\n> +\t * orientation.\n>   \t */\n> -\tTransform combined = *transform * rotationTransform_;\n> +\tif (!supportFlips_) {\n> +\t\t*orientation = transformToOrientation(rotationTransform_);\n> +\t\treturn Transform::Identity;\n> +\t}\n>   \n>   \t/*\n> -\t * We combine the platform and user transform, but must \"adjust away\"\n> -\t * any combined result that includes a transposition, as we can't do\n> -\t * those. In this case, flipping only the transpose bit is helpful to\n> -\t * applications - they either get the transform they requested, or have\n> -\t * to do a simple transpose themselves (they don't have to worry about\n> -\t * the other possible cases).\n> +\t * If the camera is mounted 90 or 270 degrees rotated, there is no\n> +\t * way we can correct it and there's no point in continuing as the\n> +\t * user request cannot be satisfied in full.\n>   \t */\n> -\tif (!!(combined & Transform::Transpose)) {\n> -\t\t/*\n> -\t\t * Flipping the transpose bit in \"transform\" flips it in the\n> -\t\t * combined result too (as it's the last thing that happens),\n> -\t\t * which is of course clearing it.\n> -\t\t */\n> -\t\t*transform ^= Transform::Transpose;\n> -\t\tcombined &= ~Transform::Transpose;\n> +\tif (!!(rotationTransform_ & Transform::Transpose)) {\n> +\t\t*orientation = transformToOrientation(rotationTransform_);\n> +\t\treturn Transform::Identity;\n>   \t}\n>   \n>   \t/*\n> -\t * We also check if the sensor doesn't do h/vflips at all, in which\n> -\t * case we clear them, and the application will have to do everything.\n> +\t * If the user request contains a transform there's no way we can\n\nyou mean \"request contains a transpose...\" not a transform right ?\n> +\t * satisfy it, default it to Identity. We cannot return early as the\n> +\t * camera mounting rotation has to be corrected, and if we get here we\n> +\t * know we can do that (we adjusted property::Rotation already because\n> +\t * of this).\n>   \t */\n> -\tif (!supportFlips_ && !!combined) {\n> -\t\t/*\n> -\t\t * If the sensor can do no transforms, then combined must be\n> -\t\t * changed to the identity. The only user transform that gives\n> -\t\t * rise to this is the inverse of the rotation. (Recall that\n> -\t\t * combined = transform * rotationTransform.)\n> -\t\t */\n> -\t\t*transform = -rotationTransform_;\n> -\t\tcombined = Transform::Identity;\n> -\t}\n> +\tTransform request = transformFromOrientation(*orientation);\n> +\tif (!!(request & Transform::Transpose))\n> +\t\trequest = Transform::Identity;\n> +\n> +\t*orientation = transformToOrientation(request);\n> -\treturn combined;\n> +\treturn request * rotationTransform_;\n\nNice. If rotationTransform_ is 180, this will return Transform 180 - \nwhich will be applied to the sensor, so we get an upright image. The \nproperty is also adjusted already in that case, and orientation too will \nreport rotate0.\n\nAnd if rotationTransform_ is identity (sensor is already upright), this \nwill return Transform identity, so no transform to be applied.\n\nIf request is Rot180 and rotationTransform is identity, It will return \nRot180 which will be applied to sensor.  But the property will still be \nreporting 0, which makes sense.\n\nLooks good to me overall... and API is much easier to grasp now.\n>   }\n>   \n>   std::string CameraSensor::logPrefix() const\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 355cb0cb76b8..ded41e011be2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -187,9 +187,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n>   \t * rotation and store the final combined transform that configure() will\n>   \t * need to apply to the sensor to save us working it out again.\n>   \t */\n> -\tTransform requestedTransform = transform;\n> -\tcombinedTransform_ = data_->cio2_.sensor()->validateTransform(&transform);\n> -\tif (transform != requestedTransform)\n> +\tOrientation requestedOrientation = orientation;\n> +\tcombinedTransform_ = data_->cio2_.sensor()->computeTransform(&orientation);\n> +\tif (orientation != requestedOrientation)\n>   \t\tstatus = Adjusted;\n>   \n>   \t/* Cap the number of entries to the available streams. */\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 91a3c60757e1..81ae84b13a62 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -481,9 +481,9 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>   \t\tstatus = Adjusted;\n>   \t}\n>   \n> -\tTransform requestedTransform = transform;\n> -\tTransform combined = sensor->validateTransform(&transform);\n> -\tif (transform != requestedTransform)\n> +\tOrientation requestedOrientation = orientation;\n> +\tcombinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> +\tif (orientation != requestedOrientation)\n>   \t\tstatus = Adjusted;\n>   \n>   \t/*\n> @@ -595,8 +595,6 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>   \tif (sensorFormat_.size.isNull())\n>   \t\tsensorFormat_.size = sensor->resolution();\n>   \n> -\tcombinedTransform_ = combined;\n> -\n>   \treturn status;\n>   }\n>   \n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index df7482920e75..9d6d816f637a 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -254,9 +254,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>   \t * rotation and store the final combined transform that configure() will\n>   \t * need to apply to the sensor to save us working it out again.\n>   \t */\n> -\tTransform requestedTransform = transform;\n> -\tcombinedTransform_ = data_->sensor_->validateTransform(&transform);\n> -\tif (transform != requestedTransform)\n> +\tOrientation requestedOrientation = orientation;\n> +\tcombinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> +\tif (orientation != requestedOrientation)\n>   \t\tstatus = Adjusted;\n>   \n>   \tstd::vector<CameraData::StreamParams> rawStreams, outStreams;\n> @@ -1193,7 +1193,8 @@ int CameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::Config\n>   \t}\n>   \n>   \t/* Always send the user transform to the IPA. */\n> -\tparams.transform = static_cast<unsigned int>(config->transform);\n> +\tparams.transform =\n> +\t\tstatic_cast<unsigned int>(transformFromOrientation(config->orientation));\n>   \n>   \t/* Ready the IPA - it must know about the sensor resolution. */\n>   \tret = ipa_->configure(sensorInfo_, params, result);\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 050285fd389e..cae67c90bd20 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -888,9 +888,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>   \tif (config_.empty())\n>   \t\treturn Invalid;\n>   \n> -\tTransform requestedTransform = transform;\n> -\tcombinedTransform_ = sensor->validateTransform(&transform);\n> -\tif (transform != requestedTransform)\n> +\tOrientation requestedOrientation = orientation;\n> +\tcombinedTransform_ = sensor->computeTransform(&orientation);\n> +\tif (orientation != requestedOrientation)\n>   \t\tstatus = Adjusted;\n>   \n>   \t/* Cap the number of entries to the available streams. */\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 277465b72164..e1f215f06db2 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -111,8 +111,8 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n>   \tif (config_.empty())\n>   \t\treturn Invalid;\n>   \n> -\tif (transform != Transform::Identity) {\n> -\t\ttransform = Transform::Identity;\n> +\tif (orientation != CameraConfiguration::rotate0) {\n> +\t\torientation = CameraConfiguration::rotate0;\n>   \t\tstatus = Adjusted;\n>   \t}\n>   \n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 204f5ad73f6d..33c165d0cee2 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -128,8 +128,8 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()\n>   \tif (config_.empty())\n>   \t\treturn Invalid;\n>   \n> -\tif (transform != Transform::Identity) {\n> -\t\ttransform = Transform::Identity;\n> +\tif (orientation != CameraConfiguration::rotate0) {\n> +\t\torientation = CameraConfiguration::rotate0;\n>   \t\tstatus = Adjusted;\n>   \t}\n>   \n> diff --git a/src/libcamera/transform.cpp b/src/libcamera/transform.cpp\n> index 4668303d0676..930155b60ff4 100644\n> --- a/src/libcamera/transform.cpp\n> +++ b/src/libcamera/transform.cpp\n> @@ -299,6 +299,64 @@ Transform transformFromRotation(int angle, bool *success)\n>   \treturn Transform::Identity;\n>   }\n>   \n> +/**\n> + * \\brief Return the transform representing \\a orientation\n> + * \\param[in] orientation The orientation to convert\n> + * \\return The transform corresponding to \\a orientation\n> + */\n> +Transform transformFromOrientation(const CameraConfiguration::Orientation &orientation)\n> +{\n> +\tswitch (orientation) {\n> +\tcase CameraConfiguration::rotate0:\n> +\t\treturn Transform::Identity;\n> +\tcase CameraConfiguration::flipRotate0:\n> +\t\treturn Transform::HFlip;\n> +\tcase CameraConfiguration::rotate180:\n> +\t\treturn Transform::Rot180;\n> +\tcase CameraConfiguration::flipRotate180:\n> +\t\treturn Transform::VFlip;\n> +\tcase CameraConfiguration::flipRotate270:\n> +\t\treturn Transform::Transpose;\n> +\tcase CameraConfiguration::rotate90:\n> +\t\treturn Transform::Rot90;\n> +\tcase CameraConfiguration::flipRotate90:\n> +\t\treturn Transform::Rot180Transpose;\n> +\tcase CameraConfiguration::rotate270:\n> +\t\treturn Transform::Rot270;\n> +\t}\n> +\n> +\treturn Transform::Identity;\n> +}\n> +\n> +/**\n> + * \\brief Return the CameraConfiguration::Orientation representing \\a transform\n> + * \\param[in] transform The transform to convert\n> + * \\return The Orientation corresponding to \\a transform\n> + */\n> +CameraConfiguration::Orientation transformToOrientation(const Transform &transform)\n> +{\n> +\tswitch (transform) {\n> +\tcase Transform::Identity:\n> +\t\treturn CameraConfiguration::rotate0;\n> +\tcase Transform::HFlip:\n> +\t\treturn CameraConfiguration::flipRotate0;\n> +\tcase Transform::VFlip:\n> +\t\treturn CameraConfiguration::flipRotate180;\n> +\tcase Transform::Rot180:\n> +\t\treturn CameraConfiguration::rotate180;\n> +\tcase Transform::Transpose:\n> +\t\treturn CameraConfiguration::flipRotate270;\n> +\tcase Transform::Rot270:\n> +\t\treturn CameraConfiguration::rotate270;\n> +\tcase Transform::Rot90:\n> +\t\treturn CameraConfiguration::rotate90;\n> +\tcase Transform::Rot180Transpose:\n> +\t\treturn CameraConfiguration::flipRotate90;\n> +\t}\n> +\n> +\treturn CameraConfiguration::rotate0;\n> +}\n> +\n>   /**\n>    * \\brief Return a character string describing the transform\n>    * \\param[in] t The transform to be described.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5D612BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Jun 2023 06:50:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DFF5E628BD;\n\tWed, 21 Jun 2023 08:50:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C7FC628BC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Jun 2023 08:50:51 +0200 (CEST)","from [192.168.1.108] (unknown [103.86.18.208])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4DC0C10A;\n\tWed, 21 Jun 2023 08:50:15 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687330252;\n\tbh=XkorguEoIcfVFLyRZ1TwpUZ/niVNS5Ojtag4TZ8Jvxg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=qUqOA/YS416CvadlX0YKk+zWv4dqg1QnutsBiMVQphO+xO8yuTYSZZ45KlGjH4X01\n\t5OBnwIWu0ICBKekbNuOhz0nN075VUItUxKmF50djU90QcE1dWWNB155hZuQ9ZetBAa\n\tD33TJJ9YTPPCQsPTyONinPfLjZCO9Hj14lTlV/AvYbd6qeru9T4lnZ6v2mW+IRx83q\n\tgzWKDSLL99YtIqaPx8OynaXSq3mx+5on2QuIYsFiwAVsgxZKtG7RP/a7VF553hYe+z\n\t4j0UlkJLMpZkUd6ZLvXuM4owT8C8bDexhxRIkanlCAVo2/qtLt0mifBetc/I3m3ba8\n\tcf+8TWSveHrhA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1687330216;\n\tbh=XkorguEoIcfVFLyRZ1TwpUZ/niVNS5Ojtag4TZ8Jvxg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=vlHWNUbJICVQbekS/e2wUnquZ1kTyx5I+n4H3ujMmSg9I6OaPylzkQcquyfpsJ+3W\n\twF1mymv61Dup9AwiuQHJpbkSPwakHClz8moysmK/1C5g3FZDtZ22ycnFmrEtiGX2VO\n\tx4NxQljWdqpsXFtSXa1BhKMur5q0yDO9jlTlr8fY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vlHWNUbJ\"; dkim-atps=neutral","Message-ID":"<8794a76f-01b7-c229-4295-042fc9c12a83@ideasonboard.com>","Date":"Wed, 21 Jun 2023 12:20:45 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.7.1","Content-Language":"en-US","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20230620142904.72600-1-jacopo.mondi@ideasonboard.com>\n\t<20230620142904.72600-4-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230620142904.72600-4-jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC 3/4] libcamera: Move to use\n\tCameraConfiguration::orientation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Umang Jain <umang.jain@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27401,"web_url":"https://patchwork.libcamera.org/comment/27401/","msgid":"<fn6urouimx2a5uf7admw572e3bcppnjucgdwgkqa3o5znmrxqy@da3gsy3tsu6l>","date":"2023-06-21T07:44:40","subject":"Re: [libcamera-devel] [RFC 3/4] libcamera: Move to use\n\tCameraConfiguration::orientation","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Umang\n\nOn Wed, Jun 21, 2023 at 12:20:45PM +0530, Umang Jain wrote:\n> Hi Jacopo,\n>\n> thank you for the patch.\n>\n> On 6/20/23 7:59 PM, Jacopo Mondi via libcamera-devel wrote:\n> > Replace the usage of CameraConfiguration::transform with the newly\n> > introduced CameraConfiguration::orientation.\n> >\n> > Rework the CameraSensor::validateTransform(transform) to\n> > CameraSensor::computeTransform(orientation), that given the desired\n> > image orientation computes the Transform that pipeline handlers should\n> > apply to the sensor to obtain it.\n> >\n> > Introduce in transform.cpp two functions to convert from Transform to\n> > Orientation and vice-versa.\n> >\n> > Port all pipeline handlers to use the newly introduced function.\n> >\n> > This commit breaks existing applications as it removes the public\n> > CameraConfiguration::transform in faviour of\n> > CameraConfiguration::orientation.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >   include/libcamera/camera.h                    |  2 -\n> >   include/libcamera/internal/camera_sensor.h    |  3 +-\n> >   include/libcamera/transform.h                 |  4 +\n> >   src/libcamera/camera.cpp                      | 15 +--\n> >   src/libcamera/camera_sensor.cpp               | 95 +++++++++----------\n> >   src/libcamera/pipeline/ipu3/ipu3.cpp          |  6 +-\n> >   src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  8 +-\n> >   .../pipeline/rpi/common/pipeline_base.cpp     |  9 +-\n> >   src/libcamera/pipeline/simple/simple.cpp      |  6 +-\n> >   src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  4 +-\n> >   src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n> >   src/libcamera/transform.cpp                   | 58 +++++++++++\n> >   12 files changed, 129 insertions(+), 85 deletions(-)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 608774ce7768..bffac6bc8922 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -20,7 +20,6 @@\n> >   #include <libcamera/controls.h>\n> >   #include <libcamera/request.h>\n> >   #include <libcamera/stream.h>\n> > -#include <libcamera/transform.h>\n> >   namespace libcamera {\n> > @@ -76,7 +75,6 @@ public:\n> >   \tbool empty() const;\n> >   \tstd::size_t size() const;\n> > -\tTransform transform;\n> >   \tOrientation orientation;\n> >   protected:\n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index 02b4b4d25e6d..1d47a2b1a500 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -14,6 +14,7 @@\n> >   #include <libcamera/base/class.h>\n> >   #include <libcamera/base/log.h>\n> > +#include <libcamera/camera.h>\n> >   #include <libcamera/control_ids.h>\n> >   #include <libcamera/controls.h>\n> >   #include <libcamera/geometry.h>\n> > @@ -71,7 +72,7 @@ public:\n> >   \tCameraLens *focusLens() { return focusLens_.get(); }\n> > -\tTransform validateTransform(Transform *transform) const;\n> > +\tTransform computeTransform(CameraConfiguration::Orientation *orientation) const;\n> >   protected:\n> >   \tstd::string logPrefix() const override;\n> > diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h\n> > index 2a6838c78369..573adf18715d 100644\n> > --- a/include/libcamera/transform.h\n> > +++ b/include/libcamera/transform.h\n> > @@ -9,6 +9,8 @@\n> >   #include <string>\n> > +#include <libcamera/camera.h>\n> > +\n> >   namespace libcamera {\n> >   enum class Transform : int {\n> > @@ -69,6 +71,8 @@ constexpr Transform operator~(Transform t)\n> >   }\n> >   Transform transformFromRotation(int angle, bool *success = nullptr);\n> > +Transform transformFromOrientation(const CameraConfiguration::Orientation &orientation);\n> > +CameraConfiguration::Orientation transformToOrientation(const Transform &transform);\n> >   const char *transformToString(Transform t);\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 5ca05719ebfc..bc1409b5c960 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -197,7 +197,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> >    * \\brief Create an empty camera configuration\n> >    */\n> >   CameraConfiguration::CameraConfiguration()\n> > -\t: transform(Transform::Identity), orientation(rotate0), config_({})\n> > +\t: orientation(rotate0), config_({})\n> >   {\n> >   }\n> > @@ -428,19 +428,6 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> >   \treturn status;\n> >   }\n> > -/**\n> > - * \\var CameraConfiguration::transform\n> > - * \\brief User-specified transform to be applied to the image\n> > - *\n> > - * The transform is a user-specified 2D plane transform that will be applied\n> > - * to the camera images by the processing pipeline before being handed to\n> > - * the application.\n> > - *\n> > - * The usual 2D plane transforms are allowed here (horizontal/vertical\n> > - * flips, multiple of 90-degree rotations etc.), but the validate() function\n> > - * may adjust this field at its discretion if the selection is not supported.\n> > - */\n> > -\n> >   /**\n> >    * \\var CameraConfiguration::orientation\n> >    * \\brief The desired orientation of the video streams produced by the camera\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index a15a6c89c5c8..5c9f30a62527 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -465,7 +465,7 @@ int CameraSensor::initProperties()\n> >   \t\t/*\n> >   \t\t * Cache the Transform associated with the camera mounting\n> > -\t\t * rotation for later use in validateTransform().\n> > +\t\t * rotation for later use in computeTransform().\n> >   \t\t */\n> >   \t\tbool success;\n> >   \t\trotationTransform_ = transformFromRotation(propertyValue, &success);\n> > @@ -477,7 +477,7 @@ int CameraSensor::initProperties()\n> >   \t\t}\n> >   \t\t/*\n> > -\t\t * Adjust property::Rotation as validateTransform() compensates\n> > +\t\t * Adjust property::Rotation as computeTransform() compensates\n> >   \t\t * for the mounting rotation. However, as a camera sensor can\n> >   \t\t * only compensate rotations by applying H/VFlips, only rotation\n> >   \t\t * of 180 degrees are automatically compensated. The other valid\n> > @@ -1033,69 +1033,66 @@ void CameraSensor::updateControlInfo()\n> >    */\n> >   /**\n> > - * \\brief Validate a transform request against the sensor capabilities\n> > - * \\param[inout] transform The requested transformation, updated to match\n> > - * the sensor capabilities\n> > + * \\brief Compute the Transform that gives the requested \\a orientation\n> > + * \\param[inout] orientation The desired image orientation\n> >    *\n> > - * The input \\a transform is the transform that the caller wants, and it is\n> > - * adjusted according to the capabilities of the sensor to represent the\n> > - * \"nearest\" transform that can actually be delivered.\n> > + * This function computes the Transform that the pipeline handler should apply\n> > + * to the CameraSensor to obtain the requested \\a orientation.\n> >    *\n> > - * The returned Transform is the transform applied to the sensor in order to\n> > - * produce the input \\a transform, It is also validated against the sensor's\n> > - * ability to perform horizontal and vertical flips.\n> > + * The intended caller of this function is the validate() implementation of\n> > + * pipeline handlers, that pass in the application requested\n> > + * CameraConfiguration::orientation and obtain a Transform to apply to the\n> > + * camera sensor, likely at configure() time.\n> >    *\n> > - * For example, if the requested \\a transform is Transform::Identity and the\n> > - * sensor rotation is 180 degrees, the output transform will be\n> > - * Transform::Rot180 to correct the images so that they appear to have\n> > - * Transform::Identity, but only if the sensor can apply horizontal and vertical\n> > - * flips.\n> > + * If the requested \\a orientation cannot be obtained, the \\a orientation\n> > + * parameter is adjusted to report what the current image orientation is and\n> > + * Transform::Identity is returned.\n> >    *\n> > - * \\return A Transform instance that represents which transformation has been\n> > - * applied to the camera sensor\n> > + * If the requested \\a orientation can be obtained, the function computes a\n> > + * Transform and does not adjust \\a orientation.\n> > + *\n> > + * Pipeline handlers are expected to verify if \\a orientation has been\n> > + * adjusted by this function and set the CameraConfiguration::status to\n> > + * Adjusted accordingly.\n> > + *\n> > + * \\return A Transform instance that applied to the CameraSensor produces images\n> > + * with \\a orientation\n> >    */\n> > -Transform CameraSensor::validateTransform(Transform *transform) const\n> > +Transform CameraSensor::computeTransform(CameraConfiguration::Orientation *orientation) const\n> >   {\n> >   \t/*\n> > -\t * Combine the requested transform to compensate the sensor mounting\n> > -\t * rotation.\n> > +\t * We cannot do any flips, we cannot change the native camera mounting\n> > +\t * orientation.\n> >   \t */\n> > -\tTransform combined = *transform * rotationTransform_;\n> > +\tif (!supportFlips_) {\n> > +\t\t*orientation = transformToOrientation(rotationTransform_);\n> > +\t\treturn Transform::Identity;\n> > +\t}\n> >   \t/*\n> > -\t * We combine the platform and user transform, but must \"adjust away\"\n> > -\t * any combined result that includes a transposition, as we can't do\n> > -\t * those. In this case, flipping only the transpose bit is helpful to\n> > -\t * applications - they either get the transform they requested, or have\n> > -\t * to do a simple transpose themselves (they don't have to worry about\n> > -\t * the other possible cases).\n> > +\t * If the camera is mounted 90 or 270 degrees rotated, there is no\n> > +\t * way we can correct it and there's no point in continuing as the\n> > +\t * user request cannot be satisfied in full.\n> >   \t */\n> > -\tif (!!(combined & Transform::Transpose)) {\n> > -\t\t/*\n> > -\t\t * Flipping the transpose bit in \"transform\" flips it in the\n> > -\t\t * combined result too (as it's the last thing that happens),\n> > -\t\t * which is of course clearing it.\n> > -\t\t */\n> > -\t\t*transform ^= Transform::Transpose;\n> > -\t\tcombined &= ~Transform::Transpose;\n> > +\tif (!!(rotationTransform_ & Transform::Transpose)) {\n> > +\t\t*orientation = transformToOrientation(rotationTransform_);\n> > +\t\treturn Transform::Identity;\n> >   \t}\n> >   \t/*\n> > -\t * We also check if the sensor doesn't do h/vflips at all, in which\n> > -\t * case we clear them, and the application will have to do everything.\n> > +\t * If the user request contains a transform there's no way we can\n>\n> you mean \"request contains a transpose...\" not a transform right ?\n\nUps, right!\n\n> > +\t * satisfy it, default it to Identity. We cannot return early as the\n> > +\t * camera mounting rotation has to be corrected, and if we get here we\n> > +\t * know we can do that (we adjusted property::Rotation already because\n> > +\t * of this).\n> >   \t */\n> > -\tif (!supportFlips_ && !!combined) {\n> > -\t\t/*\n> > -\t\t * If the sensor can do no transforms, then combined must be\n> > -\t\t * changed to the identity. The only user transform that gives\n> > -\t\t * rise to this is the inverse of the rotation. (Recall that\n> > -\t\t * combined = transform * rotationTransform.)\n> > -\t\t */\n> > -\t\t*transform = -rotationTransform_;\n> > -\t\tcombined = Transform::Identity;\n> > -\t}\n> > +\tTransform request = transformFromOrientation(*orientation);\n> > +\tif (!!(request & Transform::Transpose))\n> > +\t\trequest = Transform::Identity;\n> > +\n> > +\t*orientation = transformToOrientation(request);\n> > -\treturn combined;\n> > +\treturn request * rotationTransform_;\n>\n> Nice. If rotationTransform_ is 180, this will return Transform 180 - which\n> will be applied to the sensor, so we get an upright image. The property is\n> also adjusted already in that case, and orientation too will report rotate0.\n>\n> And if rotationTransform_ is identity (sensor is already upright), this will\n> return Transform identity, so no transform to be applied.\n>\n> If request is Rot180 and rotationTransform is identity, It will return\n> Rot180 which will be applied to sensor.  But the property will still be\n> reporting 0, which makes sense.\n>\n> Looks good to me overall... and API is much easier to grasp now.\n\nThanks!\n\n> >   }\n> >   std::string CameraSensor::logPrefix() const\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 355cb0cb76b8..ded41e011be2 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -187,9 +187,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> >   \t * rotation and store the final combined transform that configure() will\n> >   \t * need to apply to the sensor to save us working it out again.\n> >   \t */\n> > -\tTransform requestedTransform = transform;\n> > -\tcombinedTransform_ = data_->cio2_.sensor()->validateTransform(&transform);\n> > -\tif (transform != requestedTransform)\n> > +\tOrientation requestedOrientation = orientation;\n> > +\tcombinedTransform_ = data_->cio2_.sensor()->computeTransform(&orientation);\n> > +\tif (orientation != requestedOrientation)\n> >   \t\tstatus = Adjusted;\n> >   \t/* Cap the number of entries to the available streams. */\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 91a3c60757e1..81ae84b13a62 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -481,9 +481,9 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> >   \t\tstatus = Adjusted;\n> >   \t}\n> > -\tTransform requestedTransform = transform;\n> > -\tTransform combined = sensor->validateTransform(&transform);\n> > -\tif (transform != requestedTransform)\n> > +\tOrientation requestedOrientation = orientation;\n> > +\tcombinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> > +\tif (orientation != requestedOrientation)\n> >   \t\tstatus = Adjusted;\n> >   \t/*\n> > @@ -595,8 +595,6 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> >   \tif (sensorFormat_.size.isNull())\n> >   \t\tsensorFormat_.size = sensor->resolution();\n> > -\tcombinedTransform_ = combined;\n> > -\n> >   \treturn status;\n> >   }\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index df7482920e75..9d6d816f637a 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -254,9 +254,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> >   \t * rotation and store the final combined transform that configure() will\n> >   \t * need to apply to the sensor to save us working it out again.\n> >   \t */\n> > -\tTransform requestedTransform = transform;\n> > -\tcombinedTransform_ = data_->sensor_->validateTransform(&transform);\n> > -\tif (transform != requestedTransform)\n> > +\tOrientation requestedOrientation = orientation;\n> > +\tcombinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> > +\tif (orientation != requestedOrientation)\n> >   \t\tstatus = Adjusted;\n> >   \tstd::vector<CameraData::StreamParams> rawStreams, outStreams;\n> > @@ -1193,7 +1193,8 @@ int CameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::Config\n> >   \t}\n> >   \t/* Always send the user transform to the IPA. */\n> > -\tparams.transform = static_cast<unsigned int>(config->transform);\n> > +\tparams.transform =\n> > +\t\tstatic_cast<unsigned int>(transformFromOrientation(config->orientation));\n> >   \t/* Ready the IPA - it must know about the sensor resolution. */\n> >   \tret = ipa_->configure(sensorInfo_, params, result);\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 050285fd389e..cae67c90bd20 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -888,9 +888,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >   \tif (config_.empty())\n> >   \t\treturn Invalid;\n> > -\tTransform requestedTransform = transform;\n> > -\tcombinedTransform_ = sensor->validateTransform(&transform);\n> > -\tif (transform != requestedTransform)\n> > +\tOrientation requestedOrientation = orientation;\n> > +\tcombinedTransform_ = sensor->computeTransform(&orientation);\n> > +\tif (orientation != requestedOrientation)\n> >   \t\tstatus = Adjusted;\n> >   \t/* Cap the number of entries to the available streams. */\n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 277465b72164..e1f215f06db2 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -111,8 +111,8 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n> >   \tif (config_.empty())\n> >   \t\treturn Invalid;\n> > -\tif (transform != Transform::Identity) {\n> > -\t\ttransform = Transform::Identity;\n> > +\tif (orientation != CameraConfiguration::rotate0) {\n> > +\t\torientation = CameraConfiguration::rotate0;\n> >   \t\tstatus = Adjusted;\n> >   \t}\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 204f5ad73f6d..33c165d0cee2 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -128,8 +128,8 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()\n> >   \tif (config_.empty())\n> >   \t\treturn Invalid;\n> > -\tif (transform != Transform::Identity) {\n> > -\t\ttransform = Transform::Identity;\n> > +\tif (orientation != CameraConfiguration::rotate0) {\n> > +\t\torientation = CameraConfiguration::rotate0;\n> >   \t\tstatus = Adjusted;\n> >   \t}\n> > diff --git a/src/libcamera/transform.cpp b/src/libcamera/transform.cpp\n> > index 4668303d0676..930155b60ff4 100644\n> > --- a/src/libcamera/transform.cpp\n> > +++ b/src/libcamera/transform.cpp\n> > @@ -299,6 +299,64 @@ Transform transformFromRotation(int angle, bool *success)\n> >   \treturn Transform::Identity;\n> >   }\n> > +/**\n> > + * \\brief Return the transform representing \\a orientation\n> > + * \\param[in] orientation The orientation to convert\n> > + * \\return The transform corresponding to \\a orientation\n> > + */\n> > +Transform transformFromOrientation(const CameraConfiguration::Orientation &orientation)\n> > +{\n> > +\tswitch (orientation) {\n> > +\tcase CameraConfiguration::rotate0:\n> > +\t\treturn Transform::Identity;\n> > +\tcase CameraConfiguration::flipRotate0:\n> > +\t\treturn Transform::HFlip;\n> > +\tcase CameraConfiguration::rotate180:\n> > +\t\treturn Transform::Rot180;\n> > +\tcase CameraConfiguration::flipRotate180:\n> > +\t\treturn Transform::VFlip;\n> > +\tcase CameraConfiguration::flipRotate270:\n> > +\t\treturn Transform::Transpose;\n> > +\tcase CameraConfiguration::rotate90:\n> > +\t\treturn Transform::Rot90;\n> > +\tcase CameraConfiguration::flipRotate90:\n> > +\t\treturn Transform::Rot180Transpose;\n> > +\tcase CameraConfiguration::rotate270:\n> > +\t\treturn Transform::Rot270;\n> > +\t}\n> > +\n> > +\treturn Transform::Identity;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Return the CameraConfiguration::Orientation representing \\a transform\n> > + * \\param[in] transform The transform to convert\n> > + * \\return The Orientation corresponding to \\a transform\n> > + */\n> > +CameraConfiguration::Orientation transformToOrientation(const Transform &transform)\n> > +{\n> > +\tswitch (transform) {\n> > +\tcase Transform::Identity:\n> > +\t\treturn CameraConfiguration::rotate0;\n> > +\tcase Transform::HFlip:\n> > +\t\treturn CameraConfiguration::flipRotate0;\n> > +\tcase Transform::VFlip:\n> > +\t\treturn CameraConfiguration::flipRotate180;\n> > +\tcase Transform::Rot180:\n> > +\t\treturn CameraConfiguration::rotate180;\n> > +\tcase Transform::Transpose:\n> > +\t\treturn CameraConfiguration::flipRotate270;\n> > +\tcase Transform::Rot270:\n> > +\t\treturn CameraConfiguration::rotate270;\n> > +\tcase Transform::Rot90:\n> > +\t\treturn CameraConfiguration::rotate90;\n> > +\tcase Transform::Rot180Transpose:\n> > +\t\treturn CameraConfiguration::flipRotate90;\n> > +\t}\n> > +\n> > +\treturn CameraConfiguration::rotate0;\n> > +}\n> > +\n> >   /**\n> >    * \\brief Return a character string describing the transform\n> >    * \\param[in] t The transform to be described.\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 61304C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Jun 2023 07:44:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 19A7B628C3;\n\tWed, 21 Jun 2023 09:44:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 75C1E628BD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Jun 2023 09:44:43 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:1cf0:b3bc:c785:4625])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 32DF4BC;\n\tWed, 21 Jun 2023 09:44:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687333484;\n\tbh=OzNksEJF3R2jWwXhEXiGKv7RpB+Zf6U5efIu6CFCsVI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=aNWWoqbTV4Tq64ZFLAIXmwJerbbvr8NtWCN70AWv/sF5KvQZEGcMhCivEk19N0H0q\n\teCj901pRGPp065sOU4CeZ7IfSxbziGYI/wKFhCbQHkHZ0X+vWRE1KJ6v4cnTr+qT/4\n\tlignNJ7GUrb1c83h0o1HURweQn4XO9tgzmh/YYAN9cx3QrQ+14e4AA+k/ki1cAEwzx\n\tPQbKYXN9/hmFs+vVr/a8l5hT3D8Msld0lLUJAVRI1i+yjKUKRFV6oS5pI0UsIp0mpn\n\tVshg3Ls1JG69G0MtHzsU7r2IIVt/BWOZhvrOHVlRA49mqbJOLl8FMNapwYdk4fiwKw\n\tlxAGeciyxJbNA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1687333448;\n\tbh=OzNksEJF3R2jWwXhEXiGKv7RpB+Zf6U5efIu6CFCsVI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AC85EJ0gVTrIBsVQz+BPMen47QumSiRZBP95VxegCKG1NAODZKva6R7XOlS9wCV7p\n\tvUKKIpSHB6QMrRCUCuc5rOw+RrxP9s5stTKXCPizMyvnyAYZU3/wfsRTxNQZPOaGI6\n\tR3RKhtkMg7b8IU8y585VIiy345ABRvjt0kRLGvyk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AC85EJ0g\"; dkim-atps=neutral","Date":"Wed, 21 Jun 2023 09:44:40 +0200","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<fn6urouimx2a5uf7admw572e3bcppnjucgdwgkqa3o5znmrxqy@da3gsy3tsu6l>","References":"<20230620142904.72600-1-jacopo.mondi@ideasonboard.com>\n\t<20230620142904.72600-4-jacopo.mondi@ideasonboard.com>\n\t<8794a76f-01b7-c229-4295-042fc9c12a83@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<8794a76f-01b7-c229-4295-042fc9c12a83@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC 3/4] libcamera: Move to use\n\tCameraConfiguration::orientation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27405,"web_url":"https://patchwork.libcamera.org/comment/27405/","msgid":"<b81fb282-0c90-f8a4-0a26-35f78f4156be@collabora.com>","date":"2023-06-21T09:30:09","subject":"Re: [libcamera-devel] [RFC 3/4] libcamera: Move to use\n\tCameraConfiguration::orientation","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Hi Jacobo,\n\nOn 20.06.23 16:29, Jacopo Mondi wrote:\n> Replace the usage of CameraConfiguration::transform with the newly\n> introduced CameraConfiguration::orientation.\n>\n> Rework the CameraSensor::validateTransform(transform) to\n> CameraSensor::computeTransform(orientation), that given the desired\n> image orientation computes the Transform that pipeline handlers should\n> apply to the sensor to obtain it.\n>\n> Introduce in transform.cpp two functions to convert from Transform to\n> Orientation and vice-versa.\n>\n> Port all pipeline handlers to use the newly introduced function.\n>\n> This commit breaks existing applications as it removes the public\n> CameraConfiguration::transform in faviour of\n> CameraConfiguration::orientation.\n\nI'm pondering how make this breaking change less painful for apps - \nespecially Pipewire. Some distros like Fedora have a frozen libcamera \nversion but update PW, so it would be nice if there was a macro to check \nfor the new type at build time.\n\nAnother issue is the question when to remove transform.h from the public \nheaders (missing in this version) and the CameraConfiguration. Doing so \nright now would require users to update straight away - acceptable, \ngiven that libcamera does not have a stable API, but keeping it around \nfor a release or two with a build-time deprecation warning maybe \nwouldn't hurt either?\n\n>\n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>   include/libcamera/camera.h                    |  2 -\n>   include/libcamera/internal/camera_sensor.h    |  3 +-\n>   include/libcamera/transform.h                 |  4 +\n>   src/libcamera/camera.cpp                      | 15 +--\n>   src/libcamera/camera_sensor.cpp               | 95 +++++++++----------\n>   src/libcamera/pipeline/ipu3/ipu3.cpp          |  6 +-\n>   src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  8 +-\n>   .../pipeline/rpi/common/pipeline_base.cpp     |  9 +-\n>   src/libcamera/pipeline/simple/simple.cpp      |  6 +-\n>   src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  4 +-\n>   src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n>   src/libcamera/transform.cpp                   | 58 +++++++++++\n>   12 files changed, 129 insertions(+), 85 deletions(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 608774ce7768..bffac6bc8922 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -20,7 +20,6 @@\n>   #include <libcamera/controls.h>\n>   #include <libcamera/request.h>\n>   #include <libcamera/stream.h>\n> -#include <libcamera/transform.h>\n>   \n>   namespace libcamera {\n>   \n> @@ -76,7 +75,6 @@ public:\n>   \tbool empty() const;\n>   \tstd::size_t size() const;\n>   \n> -\tTransform transform;\n>   \tOrientation orientation;\n>   \n>   protected:\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 02b4b4d25e6d..1d47a2b1a500 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -14,6 +14,7 @@\n>   #include <libcamera/base/class.h>\n>   #include <libcamera/base/log.h>\n>   \n> +#include <libcamera/camera.h>\n>   #include <libcamera/control_ids.h>\n>   #include <libcamera/controls.h>\n>   #include <libcamera/geometry.h>\n> @@ -71,7 +72,7 @@ public:\n>   \n>   \tCameraLens *focusLens() { return focusLens_.get(); }\n>   \n> -\tTransform validateTransform(Transform *transform) const;\n> +\tTransform computeTransform(CameraConfiguration::Orientation *orientation) const;\n>   \n>   protected:\n>   \tstd::string logPrefix() const override;\n> diff --git a/include/libcamera/transform.h b/include/libcamera/transform.h\n> index 2a6838c78369..573adf18715d 100644\n> --- a/include/libcamera/transform.h\n> +++ b/include/libcamera/transform.h\n> @@ -9,6 +9,8 @@\n>   \n>   #include <string>\n>   \n> +#include <libcamera/camera.h>\n> +\n>   namespace libcamera {\n>   \n>   enum class Transform : int {\n> @@ -69,6 +71,8 @@ constexpr Transform operator~(Transform t)\n>   }\n>   \n>   Transform transformFromRotation(int angle, bool *success = nullptr);\n> +Transform transformFromOrientation(const CameraConfiguration::Orientation &orientation);\n> +CameraConfiguration::Orientation transformToOrientation(const Transform &transform);\n>   \n>   const char *transformToString(Transform t);\n>   \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 5ca05719ebfc..bc1409b5c960 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -197,7 +197,7 @@ LOG_DECLARE_CATEGORY(Camera)\n>    * \\brief Create an empty camera configuration\n>    */\n>   CameraConfiguration::CameraConfiguration()\n> -\t: transform(Transform::Identity), orientation(rotate0), config_({})\n> +\t: orientation(rotate0), config_({})\n>   {\n>   }\n>   \n> @@ -428,19 +428,6 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n>   \treturn status;\n>   }\n>   \n> -/**\n> - * \\var CameraConfiguration::transform\n> - * \\brief User-specified transform to be applied to the image\n> - *\n> - * The transform is a user-specified 2D plane transform that will be applied\n> - * to the camera images by the processing pipeline before being handed to\n> - * the application.\n> - *\n> - * The usual 2D plane transforms are allowed here (horizontal/vertical\n> - * flips, multiple of 90-degree rotations etc.), but the validate() function\n> - * may adjust this field at its discretion if the selection is not supported.\n> - */\n> -\n>   /**\n>    * \\var CameraConfiguration::orientation\n>    * \\brief The desired orientation of the video streams produced by the camera\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index a15a6c89c5c8..5c9f30a62527 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -465,7 +465,7 @@ int CameraSensor::initProperties()\n>   \n>   \t\t/*\n>   \t\t * Cache the Transform associated with the camera mounting\n> -\t\t * rotation for later use in validateTransform().\n> +\t\t * rotation for later use in computeTransform().\n>   \t\t */\n>   \t\tbool success;\n>   \t\trotationTransform_ = transformFromRotation(propertyValue, &success);\n> @@ -477,7 +477,7 @@ int CameraSensor::initProperties()\n>   \t\t}\n>   \n>   \t\t/*\n> -\t\t * Adjust property::Rotation as validateTransform() compensates\n> +\t\t * Adjust property::Rotation as computeTransform() compensates\n>   \t\t * for the mounting rotation. However, as a camera sensor can\n>   \t\t * only compensate rotations by applying H/VFlips, only rotation\n>   \t\t * of 180 degrees are automatically compensated. The other valid\n> @@ -1033,69 +1033,66 @@ void CameraSensor::updateControlInfo()\n>    */\n>   \n>   /**\n> - * \\brief Validate a transform request against the sensor capabilities\n> - * \\param[inout] transform The requested transformation, updated to match\n> - * the sensor capabilities\n> + * \\brief Compute the Transform that gives the requested \\a orientation\n> + * \\param[inout] orientation The desired image orientation\n>    *\n> - * The input \\a transform is the transform that the caller wants, and it is\n> - * adjusted according to the capabilities of the sensor to represent the\n> - * \"nearest\" transform that can actually be delivered.\n> + * This function computes the Transform that the pipeline handler should apply\n> + * to the CameraSensor to obtain the requested \\a orientation.\n>    *\n> - * The returned Transform is the transform applied to the sensor in order to\n> - * produce the input \\a transform, It is also validated against the sensor's\n> - * ability to perform horizontal and vertical flips.\n> + * The intended caller of this function is the validate() implementation of\n> + * pipeline handlers, that pass in the application requested\n> + * CameraConfiguration::orientation and obtain a Transform to apply to the\n> + * camera sensor, likely at configure() time.\n>    *\n> - * For example, if the requested \\a transform is Transform::Identity and the\n> - * sensor rotation is 180 degrees, the output transform will be\n> - * Transform::Rot180 to correct the images so that they appear to have\n> - * Transform::Identity, but only if the sensor can apply horizontal and vertical\n> - * flips.\n> + * If the requested \\a orientation cannot be obtained, the \\a orientation\n> + * parameter is adjusted to report what the current image orientation is and\n> + * Transform::Identity is returned.\n>    *\n> - * \\return A Transform instance that represents which transformation has been\n> - * applied to the camera sensor\n> + * If the requested \\a orientation can be obtained, the function computes a\n> + * Transform and does not adjust \\a orientation.\n> + *\n> + * Pipeline handlers are expected to verify if \\a orientation has been\n> + * adjusted by this function and set the CameraConfiguration::status to\n> + * Adjusted accordingly.\n> + *\n> + * \\return A Transform instance that applied to the CameraSensor produces images\n> + * with \\a orientation\n>    */\n> -Transform CameraSensor::validateTransform(Transform *transform) const\n> +Transform CameraSensor::computeTransform(CameraConfiguration::Orientation *orientation) const\n>   {\n>   \t/*\n> -\t * Combine the requested transform to compensate the sensor mounting\n> -\t * rotation.\n> +\t * We cannot do any flips, we cannot change the native camera mounting\n> +\t * orientation.\n>   \t */\n> -\tTransform combined = *transform * rotationTransform_;\n> +\tif (!supportFlips_) {\n> +\t\t*orientation = transformToOrientation(rotationTransform_);\n> +\t\treturn Transform::Identity;\n> +\t}\n>   \n>   \t/*\n> -\t * We combine the platform and user transform, but must \"adjust away\"\n> -\t * any combined result that includes a transposition, as we can't do\n> -\t * those. In this case, flipping only the transpose bit is helpful to\n> -\t * applications - they either get the transform they requested, or have\n> -\t * to do a simple transpose themselves (they don't have to worry about\n> -\t * the other possible cases).\n> +\t * If the camera is mounted 90 or 270 degrees rotated, there is no\n> +\t * way we can correct it and there's no point in continuing as the\n> +\t * user request cannot be satisfied in full.\n>   \t */\n> -\tif (!!(combined & Transform::Transpose)) {\n> -\t\t/*\n> -\t\t * Flipping the transpose bit in \"transform\" flips it in the\n> -\t\t * combined result too (as it's the last thing that happens),\n> -\t\t * which is of course clearing it.\n> -\t\t */\n> -\t\t*transform ^= Transform::Transpose;\n> -\t\tcombined &= ~Transform::Transpose;\n> +\tif (!!(rotationTransform_ & Transform::Transpose)) {\n> +\t\t*orientation = transformToOrientation(rotationTransform_);\n> +\t\treturn Transform::Identity;\n>   \t}\n>   \n>   \t/*\n> -\t * We also check if the sensor doesn't do h/vflips at all, in which\n> -\t * case we clear them, and the application will have to do everything.\n> +\t * If the user request contains a transform there's no way we can\n> +\t * satisfy it, default it to Identity. We cannot return early as the\n> +\t * camera mounting rotation has to be corrected, and if we get here we\n> +\t * know we can do that (we adjusted property::Rotation already because\n> +\t * of this).\n>   \t */\n> -\tif (!supportFlips_ && !!combined) {\n> -\t\t/*\n> -\t\t * If the sensor can do no transforms, then combined must be\n> -\t\t * changed to the identity. The only user transform that gives\n> -\t\t * rise to this is the inverse of the rotation. (Recall that\n> -\t\t * combined = transform * rotationTransform.)\n> -\t\t */\n> -\t\t*transform = -rotationTransform_;\n> -\t\tcombined = Transform::Identity;\n> -\t}\n> +\tTransform request = transformFromOrientation(*orientation);\n> +\tif (!!(request & Transform::Transpose))\n> +\t\trequest = Transform::Identity;\n> +\n> +\t*orientation = transformToOrientation(request);\n>   \n> -\treturn combined;\n> +\treturn request * rotationTransform_;\n>   }\n>   \n>   std::string CameraSensor::logPrefix() const\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 355cb0cb76b8..ded41e011be2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -187,9 +187,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n>   \t * rotation and store the final combined transform that configure() will\n>   \t * need to apply to the sensor to save us working it out again.\n>   \t */\n> -\tTransform requestedTransform = transform;\n> -\tcombinedTransform_ = data_->cio2_.sensor()->validateTransform(&transform);\n> -\tif (transform != requestedTransform)\n> +\tOrientation requestedOrientation = orientation;\n> +\tcombinedTransform_ = data_->cio2_.sensor()->computeTransform(&orientation);\n> +\tif (orientation != requestedOrientation)\n>   \t\tstatus = Adjusted;\n>   \n>   \t/* Cap the number of entries to the available streams. */\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 91a3c60757e1..81ae84b13a62 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -481,9 +481,9 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>   \t\tstatus = Adjusted;\n>   \t}\n>   \n> -\tTransform requestedTransform = transform;\n> -\tTransform combined = sensor->validateTransform(&transform);\n> -\tif (transform != requestedTransform)\n> +\tOrientation requestedOrientation = orientation;\n> +\tcombinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> +\tif (orientation != requestedOrientation)\n>   \t\tstatus = Adjusted;\n>   \n>   \t/*\n> @@ -595,8 +595,6 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>   \tif (sensorFormat_.size.isNull())\n>   \t\tsensorFormat_.size = sensor->resolution();\n>   \n> -\tcombinedTransform_ = combined;\n> -\n>   \treturn status;\n>   }\n>   \n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index df7482920e75..9d6d816f637a 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -254,9 +254,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>   \t * rotation and store the final combined transform that configure() will\n>   \t * need to apply to the sensor to save us working it out again.\n>   \t */\n> -\tTransform requestedTransform = transform;\n> -\tcombinedTransform_ = data_->sensor_->validateTransform(&transform);\n> -\tif (transform != requestedTransform)\n> +\tOrientation requestedOrientation = orientation;\n> +\tcombinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> +\tif (orientation != requestedOrientation)\n>   \t\tstatus = Adjusted;\n>   \n>   \tstd::vector<CameraData::StreamParams> rawStreams, outStreams;\n> @@ -1193,7 +1193,8 @@ int CameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::Config\n>   \t}\n>   \n>   \t/* Always send the user transform to the IPA. */\n> -\tparams.transform = static_cast<unsigned int>(config->transform);\n> +\tparams.transform =\n> +\t\tstatic_cast<unsigned int>(transformFromOrientation(config->orientation));\n>   \n>   \t/* Ready the IPA - it must know about the sensor resolution. */\n>   \tret = ipa_->configure(sensorInfo_, params, result);\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 050285fd389e..cae67c90bd20 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -888,9 +888,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>   \tif (config_.empty())\n>   \t\treturn Invalid;\n>   \n> -\tTransform requestedTransform = transform;\n> -\tcombinedTransform_ = sensor->validateTransform(&transform);\n> -\tif (transform != requestedTransform)\n> +\tOrientation requestedOrientation = orientation;\n> +\tcombinedTransform_ = sensor->computeTransform(&orientation);\n> +\tif (orientation != requestedOrientation)\n>   \t\tstatus = Adjusted;\n>   \n>   \t/* Cap the number of entries to the available streams. */\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 277465b72164..e1f215f06db2 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -111,8 +111,8 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n>   \tif (config_.empty())\n>   \t\treturn Invalid;\n>   \n> -\tif (transform != Transform::Identity) {\n> -\t\ttransform = Transform::Identity;\n> +\tif (orientation != CameraConfiguration::rotate0) {\n> +\t\torientation = CameraConfiguration::rotate0;\n>   \t\tstatus = Adjusted;\n>   \t}\n>   \n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 204f5ad73f6d..33c165d0cee2 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -128,8 +128,8 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()\n>   \tif (config_.empty())\n>   \t\treturn Invalid;\n>   \n> -\tif (transform != Transform::Identity) {\n> -\t\ttransform = Transform::Identity;\n> +\tif (orientation != CameraConfiguration::rotate0) {\n> +\t\torientation = CameraConfiguration::rotate0;\n>   \t\tstatus = Adjusted;\n>   \t}\n>   \n> diff --git a/src/libcamera/transform.cpp b/src/libcamera/transform.cpp\n> index 4668303d0676..930155b60ff4 100644\n> --- a/src/libcamera/transform.cpp\n> +++ b/src/libcamera/transform.cpp\n> @@ -299,6 +299,64 @@ Transform transformFromRotation(int angle, bool *success)\n>   \treturn Transform::Identity;\n>   }\n>   \n> +/**\n> + * \\brief Return the transform representing \\a orientation\n> + * \\param[in] orientation The orientation to convert\n> + * \\return The transform corresponding to \\a orientation\n> + */\n> +Transform transformFromOrientation(const CameraConfiguration::Orientation &orientation)\n> +{\n> +\tswitch (orientation) {\n> +\tcase CameraConfiguration::rotate0:\n> +\t\treturn Transform::Identity;\n> +\tcase CameraConfiguration::flipRotate0:\n> +\t\treturn Transform::HFlip;\n> +\tcase CameraConfiguration::rotate180:\n> +\t\treturn Transform::Rot180;\n> +\tcase CameraConfiguration::flipRotate180:\n> +\t\treturn Transform::VFlip;\n> +\tcase CameraConfiguration::flipRotate270:\n> +\t\treturn Transform::Transpose;\n> +\tcase CameraConfiguration::rotate90:\n> +\t\treturn Transform::Rot90;\n> +\tcase CameraConfiguration::flipRotate90:\n> +\t\treturn Transform::Rot180Transpose;\n> +\tcase CameraConfiguration::rotate270:\n> +\t\treturn Transform::Rot270;\n> +\t}\n> +\n> +\treturn Transform::Identity;\n> +}\n> +\n> +/**\n> + * \\brief Return the CameraConfiguration::Orientation representing \\a transform\n> + * \\param[in] transform The transform to convert\n> + * \\return The Orientation corresponding to \\a transform\n> + */\n> +CameraConfiguration::Orientation transformToOrientation(const Transform &transform)\n> +{\n> +\tswitch (transform) {\n> +\tcase Transform::Identity:\n> +\t\treturn CameraConfiguration::rotate0;\n> +\tcase Transform::HFlip:\n> +\t\treturn CameraConfiguration::flipRotate0;\n> +\tcase Transform::VFlip:\n> +\t\treturn CameraConfiguration::flipRotate180;\n> +\tcase Transform::Rot180:\n> +\t\treturn CameraConfiguration::rotate180;\n> +\tcase Transform::Transpose:\n> +\t\treturn CameraConfiguration::flipRotate270;\n> +\tcase Transform::Rot270:\n> +\t\treturn CameraConfiguration::rotate270;\n> +\tcase Transform::Rot90:\n> +\t\treturn CameraConfiguration::rotate90;\n> +\tcase Transform::Rot180Transpose:\n> +\t\treturn CameraConfiguration::flipRotate90;\n> +\t}\n> +\n> +\treturn CameraConfiguration::rotate0;\n> +}\n> +\n>   /**\n>    * \\brief Return a character string describing the transform\n>    * \\param[in] t The transform to be described.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8BF53C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Jun 2023 09:30:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0FB40628BD;\n\tWed, 21 Jun 2023 11:30:15 +0200 (CEST)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[46.235.227.172])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 755E161E3C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Jun 2023 11:30:13 +0200 (CEST)","from [IPV6:2001:4091:a245:842d:2b55:cc50:7b9e:92a5] (unknown\n\t[IPv6:2001:4091:a245:842d:2b55:cc50:7b9e:92a5])\n\t(using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: rmader)\n\tby madras.collabora.co.uk (Postfix) with ESMTPSA id C80E86606F8E;\n\tWed, 21 Jun 2023 10:30:12 +0100 (BST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687339815;\n\tbh=rrr5QuYAr744C3gb9EJrKoeHu7KM7kACfKKh748K3x8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=B5orKiofcQQrqcSI4ECEwmlGgr6eeS/TYX9gOfguCxhA+gkrB3q3MouQrvbHFKUu/\n\tyaLr0mKWg0uY+9D37ZUxYlPwz0lfTOadYfnfVltn31Bz1lUSBKU7cco4kp/20ayTRZ\n\tjRtfl6OxS8th2tu47d+B8fPeQ9MDnACDNwtoe0YonEM/A5iv33dSFB/5kfXWNdlstT\n\tPpdBwiOl5HRqnCXa7yF67uFX9jtzZwQd94OQycOyfilJV7p/23dHr0LLvkn5yqBhls\n\td8RI1heQYpoON+5kpfSN9Ma4Bq1eyisKvLk80BThoi8mpoepzV0+CYAbdXX4Bx8hFL\n\tKBLjgVJueeR3w==","v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1687339813;\n\tbh=rrr5QuYAr744C3gb9EJrKoeHu7KM7kACfKKh748K3x8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=YgJMPRxr/VgnVg/F0My+FsI6f5xLXOdW6wxg3lVbcFR0He9PBHkEVfkYXvjyAxueW\n\tSHENVU/WFlU2WvTDv7KdXl3flTGV+rEG2yZa7YImVOnzz2108iX1b2SX7gOEPugb0b\n\tSrML1k1v+Ag1I0giKQTNFVw/Pe+3ajg+GI/xSKXtmbTXNznsnE1SZOIZTRYb9F3ksj\n\tY9aPMg6JLArbxx/IreV7bhjLqYdmowdPDjxaC8UD8vJfKGjZ5T6dWuJNpJUMyxFXjx\n\tYG/pKTOIcaQ+pqJKTTjlYYLa7i12U4hJCzlZvyYVwEMuFqCYKzYeiC3yiP0xxoe3pk\n\t3b8KazdLDrXsA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=collabora.com\n\theader.i=@collabora.com\n\theader.b=\"YgJMPRxr\"; dkim-atps=neutral","Message-ID":"<b81fb282-0c90-f8a4-0a26-35f78f4156be@collabora.com>","Date":"Wed, 21 Jun 2023 11:30:09 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.12.0","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20230620142904.72600-1-jacopo.mondi@ideasonboard.com>\n\t<20230620142904.72600-4-jacopo.mondi@ideasonboard.com>","Content-Language":"en-US, de-DE","In-Reply-To":"<20230620142904.72600-4-jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [RFC 3/4] libcamera: Move to use\n\tCameraConfiguration::orientation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Robert Mader via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Robert Mader <robert.mader@collabora.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]