Patch Detail
Show a patch.
GET /api/patches/19607/?format=api
{ "id": 19607, "url": "https://patchwork.libcamera.org/api/patches/19607/?format=api", "web_url": "https://patchwork.libcamera.org/patch/19607/", "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": "<20240301212121.9072-17-laurent.pinchart@ideasonboard.com>", "date": "2024-03-01T21:21:05", "name": "[PATCH/RFC,16/32] libcamera: camera_sensor: Reorder functions", "commit_ref": null, "pull_url": null, "state": "rfc", "archived": false, "hash": "d317a1bae8f2311357d46b81752d7d6088a8f349", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/19607/mbox/", "series": [ { "id": 4197, "url": "https://patchwork.libcamera.org/api/series/4197/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4197", "date": "2024-03-01T21:20:49", "name": "libcamera: Support the upstream Unicam driver", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4197/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/19607/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/19607/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 C2F4FBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 1 Mar 2024 21:21:46 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7DEDE62B3D;\n\tFri, 1 Mar 2024 22:21:46 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C8BF62973\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 1 Mar 2024 22:21:43 +0100 (CET)", "from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A8FC63263;\n\tFri, 1 Mar 2024 22:21:28 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fWjHfjhc\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709328088;\n\tbh=l9IhGEVXAL09vcS7KoG1j437iANs+m7qNSq4ZH4nje0=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=fWjHfjhcBhWTQMoOawawhrT7w2n/uWxkhk7/2fSLrnt9bF8s0jXkMdc3dhPRZstUZ\n\t+dnZZeJM26XK7yUcQY/EzYDmKLQNRx70QuSMqytwc4MnaX8B8CBtCbf8KPSrKivgMn\n\tVA3cLB804cOi3HU0Mrrvmp3FjHB0hRGLXtY8m3xA=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[PATCH/RFC 16/32] libcamera: camera_sensor: Reorder functions", "Date": "Fri, 1 Mar 2024 23:21:05 +0200", "Message-ID": "<20240301212121.9072-17-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.43.0", "In-Reply-To": "<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>", "References": "<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "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>", "Cc": "Sakari Ailus <sakari.ailus@iki.fi>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The CameraSensor class has grown a lot since its creation, with many\nfunctions added for different types of purposes. They are not grouped by\ncategories in the class definition, generating confusion when reading\nthe header file. Improve readability by sorting functions by category:\n\n- Getters for static data (model, entity, focus lens, ...)\n- Format and sensor configuration accessors\n- Properties and controls (including test pattern mode)\n\nUpdate the .cpp file accordingly.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/internal/camera_sensor.h | 29 +-\n src/libcamera/sensor/camera_sensor.cpp | 328 ++++++++++-----------\n 2 files changed, 179 insertions(+), 178 deletions(-)", "diff": "diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\nindex b2f077b9cc75..750d6d729cac 100644\n--- a/include/libcamera/internal/camera_sensor.h\n+++ b/include/libcamera/internal/camera_sensor.h\n@@ -46,15 +46,15 @@ public:\n \n \tconst std::string &model() const { return model_; }\n \tconst std::string &id() const { return id_; }\n+\n \tconst MediaEntity *entity() const { return entity_; }\n+\tV4L2Subdevice *device() { return subdev_.get(); }\n+\n+\tCameraLens *focusLens() { return focusLens_.get(); }\n+\n \tconst std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }\n \tstd::vector<Size> sizes(unsigned int mbusCode) const;\n \tSize resolution() const;\n-\tconst std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const\n-\t{\n-\t\treturn testPatternModes_;\n-\t}\n-\tint setTestPatternMode(controls::draft::TestPatternModeEnum mode);\n \n \tV4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,\n \t\t\t\t const Size &size) const;\n@@ -66,18 +66,19 @@ public:\n \t\t\t Transform transform = Transform::Identity,\n \t\t\t V4L2SubdeviceFormat *sensorFormat = nullptr);\n \n+\tconst ControlList &properties() const { return properties_; }\n+\tint sensorInfo(IPACameraSensorInfo *info) const;\n+\tTransform computeTransform(Orientation *orientation) const;\n+\n \tconst ControlInfoMap &controls() const;\n \tControlList getControls(const std::vector<uint32_t> &ids);\n \tint setControls(ControlList *ctrls);\n \n-\tV4L2Subdevice *device() { return subdev_.get(); }\n-\n-\tconst ControlList &properties() const { return properties_; }\n-\tint sensorInfo(IPACameraSensorInfo *info) const;\n-\n-\tCameraLens *focusLens() { return focusLens_.get(); }\n-\n-\tTransform computeTransform(Orientation *orientation) const;\n+\tconst std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const\n+\t{\n+\t\treturn testPatternModes_;\n+\t}\n+\tint setTestPatternMode(controls::draft::TestPatternModeEnum mode);\n \n protected:\n \tstd::string logPrefix() const override;\n@@ -91,8 +92,8 @@ private:\n \tvoid initStaticProperties();\n \tvoid initTestPatternModes();\n \tint initProperties();\n-\tint applyTestPatternMode(controls::draft::TestPatternModeEnum mode);\n \tint discoverAncillaryDevices();\n+\tint applyTestPatternMode(controls::draft::TestPatternModeEnum mode);\n \n \tconst MediaEntity *entity_;\n \tstd::unique_ptr<V4L2Subdevice> subdev_;\ndiff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\nindex 545f89d036df..86ad9a85371c 100644\n--- a/src/libcamera/sensor/camera_sensor.cpp\n+++ b/src/libcamera/sensor/camera_sensor.cpp\n@@ -215,6 +215,30 @@ int CameraSensor::init()\n \treturn applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n }\n \n+int CameraSensor::generateId()\n+{\n+\tconst std::string devPath = subdev_->devicePath();\n+\n+\t/* Try to get ID from firmware description. */\n+\tid_ = sysfs::firmwareNodePath(devPath);\n+\tif (!id_.empty())\n+\t\treturn 0;\n+\n+\t/*\n+\t * Virtual sensors not described in firmware\n+\t *\n+\t * Verify it's a platform device and construct ID from the device path\n+\t * and model of sensor.\n+\t */\n+\tif (devPath.find(\"/sys/devices/platform/\", 0) == 0) {\n+\t\tid_ = devPath.substr(strlen(\"/sys/devices/\")) + \" \" + model();\n+\t\treturn 0;\n+\t}\n+\n+\tLOG(CameraSensor, Error) << \"Can't generate sensor ID\";\n+\treturn -EINVAL;\n+}\n+\n int CameraSensor::validateSensorDriver()\n {\n \tint err = 0;\n@@ -580,6 +604,21 @@ int CameraSensor::discoverAncillaryDevices()\n * \\return The sensor media entity\n */\n \n+/**\n+ * \\fn CameraSensor::device()\n+ * \\brief Retrieve the camera sensor device\n+ * \\todo Remove this function by integrating DelayedControl with CameraSensor\n+ * \\return The camera sensor device\n+ */\n+\n+/**\n+ * \\fn CameraSensor::focusLens()\n+ * \\brief Retrieve the focus lens controller\n+ *\n+ * \\return The focus lens controller. nullptr if no focus lens controller is\n+ * connected to the sensor\n+ */\n+\n /**\n * \\fn CameraSensor::mbusCodes()\n * \\brief Retrieve the media bus codes supported by the camera sensor\n@@ -632,64 +671,6 @@ Size CameraSensor::resolution() const\n \treturn std::min(sizes_.back(), activeArea_.size());\n }\n \n-/**\n- * \\fn CameraSensor::testPatternModes()\n- * \\brief Retrieve all the supported test pattern modes of the camera sensor\n- * The test pattern mode values correspond to the controls::TestPattern control.\n- *\n- * \\return The list of test pattern modes\n- */\n-\n-/**\n- * \\brief Set the test pattern mode for the camera sensor\n- * \\param[in] mode The test pattern mode\n- *\n- * The new \\a mode is applied to the sensor if it differs from the active test\n- * pattern mode. Otherwise, this function is a no-op. Setting the same test\n- * pattern mode for every frame thus incurs no performance penalty.\n- */\n-int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode)\n-{\n-\tif (testPatternMode_ == mode)\n-\t\treturn 0;\n-\n-\tif (testPatternModes_.empty()) {\n-\t\tLOG(CameraSensor, Error)\n-\t\t\t<< \"Camera sensor does not support test pattern modes.\";\n-\t\treturn -EINVAL;\n-\t}\n-\n-\treturn applyTestPatternMode(mode);\n-}\n-\n-int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode)\n-{\n-\tif (testPatternModes_.empty())\n-\t\treturn 0;\n-\n-\tauto it = std::find(testPatternModes_.begin(), testPatternModes_.end(),\n-\t\t\t mode);\n-\tif (it == testPatternModes_.end()) {\n-\t\tLOG(CameraSensor, Error) << \"Unsupported test pattern mode \"\n-\t\t\t\t\t << mode;\n-\t\treturn -EINVAL;\n-\t}\n-\n-\tLOG(CameraSensor, Debug) << \"Apply test pattern mode \" << mode;\n-\n-\tint32_t index = staticProps_->testPatternModes.at(mode);\n-\tControlList ctrls{ controls() };\n-\tctrls.set(V4L2_CID_TEST_PATTERN, index);\n-\n-\tint ret = setControls(&ctrls);\n-\tif (ret)\n-\t\treturn ret;\n-\n-\ttestPatternMode_ = mode;\n-\n-\treturn 0;\n-}\n-\n /**\n * \\brief Retrieve the best sensor format for a desired output\n * \\param[in] mbusCodes The list of acceptable media bus codes\n@@ -919,80 +900,6 @@ int CameraSensor::applyConfiguration(const SensorConfiguration &config,\n \treturn 0;\n }\n \n-/**\n- * \\brief Retrieve the supported V4L2 controls and their information\n- *\n- * Control information is updated automatically to reflect the current sensor\n- * configuration when the setFormat() function is called, without invalidating\n- * any iterator on the ControlInfoMap.\n- *\n- * \\return A map of the V4L2 controls supported by the sensor\n- */\n-const ControlInfoMap &CameraSensor::controls() const\n-{\n-\treturn subdev_->controls();\n-}\n-\n-/**\n- * \\brief Read V4L2 controls from the sensor\n- * \\param[in] ids The list of controls to read, specified by their ID\n- *\n- * This function reads the value of all controls contained in \\a ids, and\n- * returns their values as a ControlList. The control identifiers are defined by\n- * the V4L2 specification (V4L2_CID_*).\n- *\n- * If any control in \\a ids is not supported by the device, is disabled (i.e.\n- * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs\n- * during validation of the requested controls, no control is read and this\n- * function returns an empty control list.\n- *\n- * \\sa V4L2Device::getControls()\n- *\n- * \\return The control values in a ControlList on success, or an empty list on\n- * error\n- */\n-ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids)\n-{\n-\treturn subdev_->getControls(ids);\n-}\n-\n-/**\n- * \\brief Write V4L2 controls to the sensor\n- * \\param[in] ctrls The list of controls to write\n- *\n- * This function writes the value of all controls contained in \\a ctrls, and\n- * stores the values actually applied to the device in the corresponding \\a\n- * ctrls entry. The control identifiers are defined by the V4L2 specification\n- * (V4L2_CID_*).\n- *\n- * If any control in \\a ctrls is not supported by the device, is disabled (i.e.\n- * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other\n- * error occurs during validation of the requested controls, no control is\n- * written and this function returns -EINVAL.\n- *\n- * If an error occurs while writing the controls, the index of the first\n- * control that couldn't be written is returned. All controls below that index\n- * are written and their values are updated in \\a ctrls, while all other\n- * controls are not written and their values are not changed.\n- *\n- * \\sa V4L2Device::setControls()\n- *\n- * \\return 0 on success or an error code otherwise\n- * \\retval -EINVAL One of the control is not supported or not accessible\n- * \\retval i The index of the control that failed\n- */\n-int CameraSensor::setControls(ControlList *ctrls)\n-{\n-\treturn subdev_->setControls(ctrls);\n-}\n-\n-/**\n- * \\fn CameraSensor::device()\n- * \\brief Retrieve the camera sensor device\n- * \\todo Remove this function by integrating DelayedControl with CameraSensor\n- * \\return The camera sensor device\n- */\n-\n /**\n * \\fn CameraSensor::properties()\n * \\brief Retrieve the camera sensor properties\n@@ -1088,14 +995,6 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n \treturn 0;\n }\n \n-/**\n- * \\fn CameraSensor::focusLens()\n- * \\brief Retrieve the focus lens controller\n- *\n- * \\return The focus lens controller. nullptr if no focus lens controller is\n- * connected to the sensor\n- */\n-\n /**\n * \\brief Compute the Transform that gives the requested \\a orientation\n * \\param[inout] orientation The desired image orientation\n@@ -1155,33 +1054,134 @@ Transform CameraSensor::computeTransform(Orientation *orientation) const\n \treturn transform;\n }\n \n+/**\n+ * \\brief Retrieve the supported V4L2 controls and their information\n+ *\n+ * Control information is updated automatically to reflect the current sensor\n+ * configuration when the setFormat() function is called, without invalidating\n+ * any iterator on the ControlInfoMap.\n+ *\n+ * \\return A map of the V4L2 controls supported by the sensor\n+ */\n+const ControlInfoMap &CameraSensor::controls() const\n+{\n+\treturn subdev_->controls();\n+}\n+\n+/**\n+ * \\brief Read V4L2 controls from the sensor\n+ * \\param[in] ids The list of controls to read, specified by their ID\n+ *\n+ * This function reads the value of all controls contained in \\a ids, and\n+ * returns their values as a ControlList. The control identifiers are defined by\n+ * the V4L2 specification (V4L2_CID_*).\n+ *\n+ * If any control in \\a ids is not supported by the device, is disabled (i.e.\n+ * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs\n+ * during validation of the requested controls, no control is read and this\n+ * function returns an empty control list.\n+ *\n+ * \\sa V4L2Device::getControls()\n+ *\n+ * \\return The control values in a ControlList on success, or an empty list on\n+ * error\n+ */\n+ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids)\n+{\n+\treturn subdev_->getControls(ids);\n+}\n+\n+/**\n+ * \\brief Write V4L2 controls to the sensor\n+ * \\param[in] ctrls The list of controls to write\n+ *\n+ * This function writes the value of all controls contained in \\a ctrls, and\n+ * stores the values actually applied to the device in the corresponding \\a\n+ * ctrls entry. The control identifiers are defined by the V4L2 specification\n+ * (V4L2_CID_*).\n+ *\n+ * If any control in \\a ctrls is not supported by the device, is disabled (i.e.\n+ * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other\n+ * error occurs during validation of the requested controls, no control is\n+ * written and this function returns -EINVAL.\n+ *\n+ * If an error occurs while writing the controls, the index of the first\n+ * control that couldn't be written is returned. All controls below that index\n+ * are written and their values are updated in \\a ctrls, while all other\n+ * controls are not written and their values are not changed.\n+ *\n+ * \\sa V4L2Device::setControls()\n+ *\n+ * \\return 0 on success or an error code otherwise\n+ * \\retval -EINVAL One of the control is not supported or not accessible\n+ * \\retval i The index of the control that failed\n+ */\n+int CameraSensor::setControls(ControlList *ctrls)\n+{\n+\treturn subdev_->setControls(ctrls);\n+}\n+\n+/**\n+ * \\fn CameraSensor::testPatternModes()\n+ * \\brief Retrieve all the supported test pattern modes of the camera sensor\n+ * The test pattern mode values correspond to the controls::TestPattern control.\n+ *\n+ * \\return The list of test pattern modes\n+ */\n+\n+/**\n+ * \\brief Set the test pattern mode for the camera sensor\n+ * \\param[in] mode The test pattern mode\n+ *\n+ * The new \\a mode is applied to the sensor if it differs from the active test\n+ * pattern mode. Otherwise, this function is a no-op. Setting the same test\n+ * pattern mode for every frame thus incurs no performance penalty.\n+ */\n+int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode)\n+{\n+\tif (testPatternMode_ == mode)\n+\t\treturn 0;\n+\n+\tif (testPatternModes_.empty()) {\n+\t\tLOG(CameraSensor, Error)\n+\t\t\t<< \"Camera sensor does not support test pattern modes.\";\n+\t\treturn -EINVAL;\n+\t}\n+\n+\treturn applyTestPatternMode(mode);\n+}\n+\n+int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode)\n+{\n+\tif (testPatternModes_.empty())\n+\t\treturn 0;\n+\n+\tauto it = std::find(testPatternModes_.begin(), testPatternModes_.end(),\n+\t\t\t mode);\n+\tif (it == testPatternModes_.end()) {\n+\t\tLOG(CameraSensor, Error) << \"Unsupported test pattern mode \"\n+\t\t\t\t\t << mode;\n+\t\treturn -EINVAL;\n+\t}\n+\n+\tLOG(CameraSensor, Debug) << \"Apply test pattern mode \" << mode;\n+\n+\tint32_t index = staticProps_->testPatternModes.at(mode);\n+\tControlList ctrls{ controls() };\n+\tctrls.set(V4L2_CID_TEST_PATTERN, index);\n+\n+\tint ret = setControls(&ctrls);\n+\tif (ret)\n+\t\treturn ret;\n+\n+\ttestPatternMode_ = mode;\n+\n+\treturn 0;\n+}\n+\n std::string CameraSensor::logPrefix() const\n {\n \treturn \"'\" + entity_->name() + \"'\";\n }\n \n-int CameraSensor::generateId()\n-{\n-\tconst std::string devPath = subdev_->devicePath();\n-\n-\t/* Try to get ID from firmware description. */\n-\tid_ = sysfs::firmwareNodePath(devPath);\n-\tif (!id_.empty())\n-\t\treturn 0;\n-\n-\t/*\n-\t * Virtual sensors not described in firmware\n-\t *\n-\t * Verify it's a platform device and construct ID from the device path\n-\t * and model of sensor.\n-\t */\n-\tif (devPath.find(\"/sys/devices/platform/\", 0) == 0) {\n-\t\tid_ = devPath.substr(strlen(\"/sys/devices/\")) + \" \" + model();\n-\t\treturn 0;\n-\t}\n-\n-\tLOG(CameraSensor, Error) << \"Can't generate sensor ID\";\n-\treturn -EINVAL;\n-}\n-\n } /* namespace libcamera */\n", "prefixes": [ "PATCH/RFC", "16/32" ] }