Patch Detail
Show a patch.
GET /api/patches/18259/?format=api
{ "id": 18259, "url": "https://patchwork.libcamera.org/api/patches/18259/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18259/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20230206195809.62203-1-jacopo.mondi@ideasonboard.com>", "date": "2023-02-06T19:58:09", "name": "[libcamera-devel] libcamera: Remove transform from V4L2SubdeviceFormat", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "1b9d8c1719c7b50cb90c85f21f013152e61c0952", "submitter": { "id": 143, "url": "https://patchwork.libcamera.org/api/people/143/?format=api", "name": "Jacopo Mondi", "email": "jacopo.mondi@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18259/mbox/", "series": [ { "id": 3741, "url": "https://patchwork.libcamera.org/api/series/3741/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3741", "date": "2023-02-06T19:58:09", "name": "[libcamera-devel] libcamera: Remove transform from V4L2SubdeviceFormat", "version": 1, "mbox": "https://patchwork.libcamera.org/series/3741/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18259/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18259/checks/", "tags": {}, "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 63CFDBDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 6 Feb 2023 19:58:30 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A5D20625F1;\n\tMon, 6 Feb 2023 20:58:29 +0100 (CET)", "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 89B6C603B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 6 Feb 2023 20:58:27 +0100 (CET)", "from uno.homenet.telecomitalia.it\n\t(host-79-35-57-126.retail.telecomitalia.it [79.35.57.126])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9983B88E;\n\tMon, 6 Feb 2023 20:58:26 +0100 (CET)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675713509;\n\tbh=dCmoWo0hzh66u5ddr99OgHe1LYM7z1T2EJCt9/z7TF0=;\n\th=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post:\n\tList-Help:List-Subscribe:From:Reply-To:Cc:From;\n\tb=Xyep+ejw87+dnQTMNicnWN9s5csH7PK31+v/Sx0a4YcaablmdFnQulsVXjwy9ghte\n\tNLIx4pwXeFOgGkLtCbk97/Tln4U1ZY7V8FVmEFXfuRxMcqNsbaQ4R2Ag6RejEi4szI\n\tQK8PgmatXfk3MgHpEwwWFsc66fGBDPLatkGmwVDBX7gRao7Okm9PbA2rGFJPbQXHAf\n\t+aFSK9wOh4mCHQWvuEMZYqJ9tqFD/PSPdns/fv7f/bG64SYfBKQd1y/ihiF4J063k5\n\tFMvwWfnnNQmc5IDLFxs6EduJ2lCzniHYMSWHp74B5dQ3HPRNW7aZjh1gU8iMxnmkPr\n\t1f90sAhn07tAw==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675713507;\n\tbh=dCmoWo0hzh66u5ddr99OgHe1LYM7z1T2EJCt9/z7TF0=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=G4dxhSK+3kP4GsZAAptHHOFcPzp8h49BwdKPAo+UhNp0jJ0033lBPy2Nev1geBn0K\n\tmsHGbr+5XSUWzJN5EmIzAPUYhGxgUiCUqbKGc27Pjfp0vtYT1RpTjdpbzA02B33c2S\n\tzhxN1CD/nfHRmlsYvfH1Ldkd64usfwyf+poZ898M=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"G4dxhSK+\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 6 Feb 2023 20:58:09 +0100", "Message-Id": "<20230206195809.62203-1-jacopo.mondi@ideasonboard.com>", "X-Mailer": "git-send-email 2.39.0", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH] libcamera: Remove transform from\n\tV4L2SubdeviceFormat", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Commit 6f6e1bf704fe (\"libcamera: camera_sensor: Apply flips at\nsetFormat()\") extended the CameraSensor::setFormat() function\nto apply vertical/horizontal flips on the sensor based on the\nsupplied Transform. To pass the Transform to the function the\nV4L2SubdeviceFormat structure has been augmented with a Transform\nmember.\n\nHowever as the newly added Transform is not used at all in the\nV4L2Subdevice class, it should not be part of V4L2SubdeviceFormat.\n\nFix that by removing the transform field from V4L2SubdeviceFormat\nand pass it as an explicit parameter to CameraSensor::setFormat().\n\nFixes: 6f6e1bf704fe (\"libcamera: camera_sensor: Apply flips at setFormat())\nSigned-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n---\nCompiled tested only:\nhttps://buildbot.libcamera.org/#/changes/4232\n---\n include/libcamera/internal/camera_sensor.h | 4 +++-\n include/libcamera/internal/v4l2_subdevice.h | 2 --\n src/libcamera/camera_sensor.cpp | 11 ++++-------\n src/libcamera/pipeline/ipu3/cio2.cpp | 3 +--\n src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++-----\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 ++++--\n src/libcamera/v4l2_subdevice.cpp | 7 -------\n 7 files changed, 17 insertions(+), 26 deletions(-)\n\n--\n2.39.0", "diff": "diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\nindex bea52badaff7..77514de7a879 100644\n--- a/include/libcamera/internal/camera_sensor.h\n+++ b/include/libcamera/internal/camera_sensor.h\n@@ -17,6 +17,7 @@\n #include <libcamera/control_ids.h>\n #include <libcamera/controls.h>\n #include <libcamera/geometry.h>\n+#include <libcamera/transform.h>\n\n #include <libcamera/ipa/core_ipa_interface.h>\n\n@@ -55,7 +56,8 @@ public:\n\n \tV4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,\n \t\t\t\t const Size &size) const;\n-\tint setFormat(V4L2SubdeviceFormat *format);\n+\tint setFormat(V4L2SubdeviceFormat *format,\n+\t\t const Transform &transform = Transform::Identity);\n\n \tconst ControlInfoMap &controls() const;\n \tControlList getControls(const std::vector<uint32_t> &ids);\ndiff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\nindex 576faf971a05..69862de0585a 100644\n--- a/include/libcamera/internal/v4l2_subdevice.h\n+++ b/include/libcamera/internal/v4l2_subdevice.h\n@@ -20,7 +20,6 @@\n\n #include <libcamera/color_space.h>\n #include <libcamera/geometry.h>\n-#include <libcamera/transform.h>\n\n #include \"libcamera/internal/formats.h\"\n #include \"libcamera/internal/media_object.h\"\n@@ -45,7 +44,6 @@ struct V4L2SubdeviceFormat {\n \tuint32_t mbus_code;\n \tSize size;\n \tstd::optional<ColorSpace> colorSpace;\n-\tTransform transform = Transform::Identity;\n\n \tconst std::string toString() const;\n \tuint8_t bitsPerPixel() const;\ndiff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\nindex 274ed419ddfd..903edbdcf233 100644\n--- a/src/libcamera/camera_sensor.cpp\n+++ b/src/libcamera/camera_sensor.cpp\n@@ -16,7 +16,6 @@\n #include <string.h>\n\n #include <libcamera/property_ids.h>\n-#include <libcamera/transform.h>\n\n #include <libcamera/base/utils.h>\n\n@@ -751,7 +750,6 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n \t\t.mbus_code = bestCode,\n \t\t.size = *bestSize,\n \t\t.colorSpace = ColorSpace::Raw,\n-\t\t.transform = Transform::Identity,\n \t};\n\n \treturn format;\n@@ -760,6 +758,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n /**\n * \\brief Set the sensor output format\n * \\param[in] format The desired sensor output format\n+ * \\param[in] transform The transform to be applied on the sensor. Defaults to Identity.\n *\n * If flips are writable they are configured according to the desired Transform.\n * Transform::Identity always corresponds to H/V flip being disabled if the\n@@ -770,18 +769,16 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu\n *\n * \\return 0 on success or a negative error code otherwise\n */\n-int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n+int CameraSensor::setFormat(V4L2SubdeviceFormat *format, const Transform &transform)\n {\n \t/* Configure flips if the sensor supports that. */\n \tif (supportFlips_) {\n \t\tControlList flipCtrls(subdev_->controls());\n\n \t\tflipCtrls.set(V4L2_CID_HFLIP,\n-\t\t\t static_cast<int32_t>(!!(format->transform &\n-\t\t\t\t\t\t Transform::HFlip)));\n+\t\t\t static_cast<int32_t>(!!(transform & Transform::HFlip)));\n \t\tflipCtrls.set(V4L2_CID_VFLIP,\n-\t\t\t static_cast<int32_t>(!!(format->transform &\n-\t\t\t\t\t\t Transform::VFlip)));\n+\t\t\t static_cast<int32_t>(!!(transform & Transform::VFlip)));\n\n \t\tint ret = subdev_->setControls(&flipCtrls);\n \t\tif (ret)\ndiff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\nindex a819884f762d..7400cb0b644c 100644\n--- a/src/libcamera/pipeline/ipu3/cio2.cpp\n+++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n@@ -194,8 +194,7 @@ int CIO2Device::configure(const Size &size, const Transform &transform,\n \t */\n \tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToPixelFormat);\n \tsensorFormat = getSensorFormat(mbusCodes, size);\n-\tsensorFormat.transform = transform;\n-\tret = sensor_->setFormat(&sensorFormat);\n+\tret = sensor_->setFormat(&sensorFormat, transform);\n \tif (ret)\n \t\treturn ret;\n\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 77e860ab0e72..da408c89bc3f 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -832,13 +832,13 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n \t\t}\n \t}\n\n-\t/* First calculate the best sensor mode we can use based on the user request. */\n+\t/*\n+\t * First calculate the best sensor mode we can use based on the user\n+\t * and apply any cached transform.\n+\t */\n \tV4L2SubdeviceFormat sensorFormat = findBestFormat(data->sensorFormats_, rawStream ? sensorSize : maxSize, bitDepth);\n-\t/* Apply any cached transform. */\n \tconst RPiCameraConfiguration *rpiConfig = static_cast<const RPiCameraConfiguration *>(config);\n-\tsensorFormat.transform = rpiConfig->combinedTransform_;\n-\t/* Finally apply the format on the sensor. */\n-\tret = data->sensor_->setFormat(&sensorFormat);\n+\tret = data->sensor_->setFormat(&sensorFormat, rpiConfig->combinedTransform_);\n \tif (ret)\n \t\treturn ret;\n\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 5f22a29d02c6..8a30fe061d04 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -125,6 +125,7 @@ public:\n \tStatus validate() override;\n\n \tconst V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; }\n+\tconst Transform &combinedTransform() { return combinedTransform_; }\n\n private:\n \tbool fitsAllPaths(const StreamConfiguration &cfg);\n@@ -138,6 +139,7 @@ private:\n \tconst RkISP1CameraData *data_;\n\n \tV4L2SubdeviceFormat sensorFormat_;\n+\tTransform combinedTransform_;\n };\n\n class PipelineHandlerRkISP1 : public PipelineHandler\n@@ -591,7 +593,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \tif (sensorFormat_.size.isNull())\n \t\tsensorFormat_.size = sensor->resolution();\n\n-\tsensorFormat_.transform = combined;\n+\tcombinedTransform_ = combined;\n\n \treturn status;\n }\n@@ -720,7 +722,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n \tV4L2SubdeviceFormat format = config->sensorFormat();\n \tLOG(RkISP1, Debug) << \"Configuring sensor with \" << format;\n\n-\tret = sensor->setFormat(&format);\n+\tret = sensor->setFormat(&format, config->combinedTransform());\n \tif (ret < 0)\n \t\treturn ret;\n\ndiff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\nindex 38ff8b0c605b..15e8206a915c 100644\n--- a/src/libcamera/v4l2_subdevice.cpp\n+++ b/src/libcamera/v4l2_subdevice.cpp\n@@ -216,13 +216,6 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = {\n * resulting color space is acceptable.\n */\n\n-/**\n- * \\var V4L2SubdeviceFormat::transform\n- * \\brief The transform (vertical/horizontal flips) to be applied on the subdev\n- *\n- * Default initialized to Identity (no transform).\n- */\n-\n /**\n * \\brief Assemble and return a string describing the format\n * \\return A string describing the V4L2SubdeviceFormat\n", "prefixes": [ "libcamera-devel" ] }