Patch Detail
Show a patch.
GET /api/1.1/patches/18113/?format=api
{ "id": 18113, "url": "https://patchwork.libcamera.org/api/1.1/patches/18113/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18113/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20230114194712.23272-3-jacopo.mondi@ideasonboard.com>", "date": "2023-01-14T19:47:09", "name": "[libcamera-devel,v2,2/5] libcamera: camera_sensor: Validate Transform", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "905b00a2b91affd96370a642872fd4f8e63f1bdf", "submitter": { "id": 143, "url": "https://patchwork.libcamera.org/api/1.1/people/143/?format=api", "name": "Jacopo Mondi", "email": "jacopo.mondi@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18113/mbox/", "series": [ { "id": 3711, "url": "https://patchwork.libcamera.org/api/1.1/series/3711/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3711", "date": "2023-01-14T19:47:07", "name": "libcamera: camera_sensor: Centralize flips handling", "version": 2, "mbox": "https://patchwork.libcamera.org/series/3711/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18113/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18113/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 1FCD5C3240\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 14 Jan 2023 19:47:31 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BB986625EC;\n\tSat, 14 Jan 2023 20:47:28 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 81F05625D0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 14 Jan 2023 20:47:25 +0100 (CET)", "from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EAAA84D4;\n\tSat, 14 Jan 2023 20:47:24 +0100 (CET)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1673725648;\n\tbh=CtRTJpV4d+DsfdLbYm0nj4IZEqeFkpUvjLsEMsL4KgY=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=iMbaIN0fkKSXlkG+v7pdrcqms6fkut9pJucy+eJ/3wnVYuVnrVN+Lbu20G/0XmirI\n\tKnBMVvEg7st44nOJpZnv/iHOwh9Ida2mHtf5hOnzSkwwPDlH+jF6FB4vLgHB45TC+s\n\try1jXCVGBMR1CaoC2E4rogaTLwaf8KJdzhQTS8I0AutZCvd187Fko7ZyL1XnGUUlu0\n\t4kK+hdmT2bEHGcJXWiS+PCrYz0wJEnAQPWYpQ2AuT7RN6y3xjM88R1Ven3Unr0/uwz\n\toDyeH9NS3QkzuyWjDNivLvuwI8V3fuSBihj3UaJt9jAe17w7ONmvBcQip/SyWPim6e\n\t5XhKTT6ZaZfag==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1673725645;\n\tbh=CtRTJpV4d+DsfdLbYm0nj4IZEqeFkpUvjLsEMsL4KgY=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=AZrHpNhi/Dv2tGiAl4F5CxzbmYXDqu1CaJkEZ9XB7S/hjosa6QFMmbOWQ4xn0Slo6\n\terATmIngOulW+8XKKzr0edtDc0pyUJQFh0W0sn9lEJO0p5pv0TwZz/UfO6OY898QMP\n\tyFXSBFt+GzDofWgikx3uPSgjR5tWb3WL9xjbMs6o=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AZrHpNhi\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 14 Jan 2023 20:47:09 +0100", "Message-Id": "<20230114194712.23272-3-jacopo.mondi@ideasonboard.com>", "X-Mailer": "git-send-email 2.39.0", "In-Reply-To": "<20230114194712.23272-1-jacopo.mondi@ideasonboard.com>", "References": "<20230114194712.23272-1-jacopo.mondi@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 2/5] libcamera: camera_sensor: Validate\n\tTransform", "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": "The two pipeline handlers that currently support Transform (IPU3 and\nRkISP1) implement it by operating H/V flips on the image sensor.\n\nCentralize the code that validates a Transform request against the\nsensor rotation capabilities in the CameraSensor class.\n\nThe implementation in the IPU3 pipeline handler was copied from the\nRaspberryPi implementation, and is now centralized in CameraSensor to\nmake it easier for other platforms.\n\nThe CameraSensor::validateTransform() implementation comes directly from\nthe RaspberryPi pipeline handler, no functional changes intended.\n\nSigned-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n---\n include/libcamera/internal/camera_sensor.h | 4 ++\n src/libcamera/camera_sensor.cpp | 65 +++++++++++++++++++\n src/libcamera/pipeline/ipu3/ipu3.cpp | 45 ++-----------\n .../pipeline/raspberrypi/raspberrypi.cpp | 59 ++---------------\n 4 files changed, 82 insertions(+), 91 deletions(-)", "diff": "diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\nindex 878f3c28a3c9..bea52badaff7 100644\n--- a/include/libcamera/internal/camera_sensor.h\n+++ b/include/libcamera/internal/camera_sensor.h\n@@ -29,6 +29,8 @@ class BayerFormat;\n class CameraLens;\n class MediaEntity;\n \n+enum class Transform;\n+\n struct CameraSensorProperties;\n \n class CameraSensor : protected Loggable\n@@ -68,6 +70,8 @@ public:\n \n \tCameraLens *focusLens() { return focusLens_.get(); }\n \n+\tTransform validateTransform(Transform *transform) const;\n+\n protected:\n \tstd::string logPrefix() const override;\n \ndiff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\nindex 83ac075a4265..3518d3e3b02a 100644\n--- a/src/libcamera/camera_sensor.cpp\n+++ b/src/libcamera/camera_sensor.cpp\n@@ -962,6 +962,71 @@ void CameraSensor::updateControlInfo()\n * connected to the sensor\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+ *\n+ * The requested \\a transform is adjusted to compensate for the sensor's\n+ * mounting rotation and validated agains the sensor capabilities.\n+ *\n+ * For example, if the requested \\a transform is Transform::Identity and the\n+ * sensor rotation is 180 degrees, the resulting transform returned by the\n+ * function is Transform::Rot180 to automatically correct the image, but only if\n+ * the sensor can actually apply horizontal and vertical flips.\n+ *\n+ * \\return A Transform instance that represents which transformation has been\n+ * applied to the camera sensor\n+ */\n+Transform CameraSensor::validateTransform(Transform *transform) const\n+{\n+\t/* Adjust the requested transform to compensate the sensor rotation. */\n+\tint32_t rotation = properties().get(properties::Rotation).value_or(0);\n+\tbool success;\n+\n+\tTransform rotationTransform = transformFromRotation(rotation, &success);\n+\tif (!success)\n+\t\tLOG(CameraSensor, Warning) << \"Invalid rotation of \" << rotation\n+\t\t\t\t\t << \" degrees - ignoring\";\n+\n+\tTransform combined = *transform * rotationTransform;\n+\n+\t/*\n+\t * We combine the platform and user transform, but must \"adjust away\"\n+\t * any combined result that includes a transform, as we can't do those.\n+\t * 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 */\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+\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 */\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 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+\n+\treturn combined;\n+}\n+\n std::string CameraSensor::logPrefix() const\n {\n \treturn \"'\" + entity_->name() + \"'\";\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex e4d79ea44aed..a424ac914859 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -184,48 +184,15 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n \tif (config_.empty())\n \t\treturn Invalid;\n \n-\tTransform combined = transform * data_->rotationTransform_;\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 */\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\ttransform ^= Transform::Transpose;\n-\t\tcombined &= ~Transform::Transpose;\n-\t\tstatus = Adjusted;\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 * Validate the requested transform against the sensor capabilities and\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-\tif (!data_->supportsFlips_ && !!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\ttransform = -data_->rotationTransform_;\n-\t\tcombined = Transform::Identity;\n+\tTransform requestedTransform = transform;\n+\tcombinedTransform_ = data_->cio2_.sensor()->validateTransform(&transform);\n+\tif (transform != requestedTransform)\n \t\tstatus = Adjusted;\n-\t}\n-\n-\t/*\n-\t * Store the final combined transform that configure() will need to\n-\t * apply to the sensor to save us working it out again.\n-\t */\n-\tcombinedTransform_ = combined;\n \n \t/* Cap the number of entries to the available streams. */\n \tif (config_.size() > kMaxStreams) {\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 8569df17976a..c086a69a913d 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -367,59 +367,14 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n \tstatus = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace);\n \n \t/*\n-\t * What if the platform has a non-90 degree rotation? We can't even\n-\t * \"adjust\" the configuration and carry on. Alternatively, raising an\n-\t * error means the platform can never run. Let's just print a warning\n-\t * and continue regardless; the rotation is effectively set to zero.\n+\t * Validate the requested transform against the sensor capabilities and\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-\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation).value_or(0);\n-\tbool success;\n-\tTransform rotationTransform = transformFromRotation(rotation, &success);\n-\tif (!success)\n-\t\tLOG(RPI, Warning) << \"Invalid rotation of \" << rotation\n-\t\t\t\t << \" degrees - ignoring\";\n-\tTransform combined = transform * rotationTransform;\n-\n-\t/*\n-\t * We combine the platform and user transform, but must \"adjust away\"\n-\t * any combined result that includes a transform, as we can't do those.\n-\t * 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 */\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\ttransform ^= Transform::Transpose;\n-\t\tcombined &= ~Transform::Transpose;\n-\t\tstatus = Adjusted;\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 */\n-\tif (!data_->supportsFlips_ && !!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 the inverse of the rotation. (Recall that\n-\t\t * combined = transform * rotationTransform.)\n-\t\t */\n-\t\ttransform = -rotationTransform;\n-\t\tcombined = Transform::Identity;\n+\tTransform requestedTransform = transform;\n+\tcombinedTransform_ = data_->sensor_->validateTransform(&transform);\n+\tif (transform != requestedTransform)\n \t\tstatus = Adjusted;\n-\t}\n-\n-\t/*\n-\t * Store the final combined transform that configure() will need to\n-\t * apply to the sensor to save us working it out again.\n-\t */\n-\tcombinedTransform_ = combined;\n \n \tunsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n \tstd::pair<int, Size> outSize[2];\n@@ -454,7 +409,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n \t\t\tif (data_->flipsAlterBayerOrder_) {\n \t\t\t\tBayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc);\n \t\t\t\tbayer.order = data_->nativeBayerOrder_;\n-\t\t\t\tbayer = bayer.transform(combined);\n+\t\t\t\tbayer = bayer.transform(combinedTransform_);\n \t\t\t\tfourcc = bayer.toV4L2PixelFormat();\n \t\t\t}\n \n", "prefixes": [ "libcamera-devel", "v2", "2/5" ] }