Patch Detail
Show a patch.
GET /api/1.1/patches/17514/?format=api
{ "id": 17514, "url": "https://patchwork.libcamera.org/api/1.1/patches/17514/?format=api", "web_url": "https://patchwork.libcamera.org/patch/17514/", "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": "<20221003212128.32429-5-laurent.pinchart@ideasonboard.com>", "date": "2022-10-03T21:21:24", "name": "[libcamera-devel,4/8] ipa: camera_sensor_helper: Implement factories through class templates", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "1f7ef2481c7802a1fa9d3268f2873a6078374d62", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/17514/mbox/", "series": [ { "id": 3528, "url": "https://patchwork.libcamera.org/api/1.1/series/3528/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3528", "date": "2022-10-03T21:21:20", "name": "libcamera: Use class templates for auto-registration", "version": 1, "mbox": "https://patchwork.libcamera.org/series/3528/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/17514/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/17514/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 91D31C3286\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 3 Oct 2022 21:21:39 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 32F7160A74;\n\tMon, 3 Oct 2022 23:21:39 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D905604FE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 3 Oct 2022 23:21:37 +0200 (CEST)", "from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 286E3519\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 3 Oct 2022 23:21:37 +0200 (CEST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1664832099;\n\tbh=O7o9URYdWuQuB+HNYJT6a9yPYDRG53BY52xPUmesJIw=;\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:\n\tFrom;\n\tb=DDU+GDcNuTTkige29Y3OTmNgopt+Y9PIyfonUT5KYGjyDuW8SXkYXSMUW7fBF7K+N\n\tngupPrLQCxhVN0afdWy3+IagX5VLJbl1wGLz1JVD/dUo0IGtsj7qnsNAIOXzWvC2it\n\t2V1H2OfiVZnNnGLuy2WHEYOd7nFGD1sYUOhwVxgAi2F6je0yl4Q/N2yBCfb0Zw4lxA\n\tMWc+HtQTfgtkDh+10AqA15mGVy92bEoe3E4D+R4DhpE6EpbE3C1efFcUpgNqaZBANa\n\tzjHbesVgQYqw06EQlMER5tLteAODaOEDvRU7dw/Ivz6OGZSnlwoASaSQ1U1NIzgtWi\n\tjDTvUurclVDnw==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1664832097;\n\tbh=O7o9URYdWuQuB+HNYJT6a9yPYDRG53BY52xPUmesJIw=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=Nua3s/jb5fejjb51GFf+OrNenifBVhcntnGn/WOI3suLa15DyHW8tOcIUd5GjD3aq\n\tTHfxgNZMLboxLrlvPtbEe41StUp3I+EkEGI6SNII5lZYyJa0PeIQiiOF8SIhAq1QLz\n\ta0L3pXgGyEOAdKDCRKwSFGzigQTGIV0l/ZHz1B0g=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Nua3s/jb\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 4 Oct 2022 00:21:24 +0300", "Message-Id": "<20221003212128.32429-5-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.35.1", "In-Reply-To": "<20221003212128.32429-1-laurent.pinchart@ideasonboard.com>", "References": "<20221003212128.32429-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 4/8] ipa: camera_sensor_helper: Implement\n\tfactories through class templates", "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": "Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The REGISTER_CAMERA_SENSOR_HELPER() macro defines a class type that\ninherits from the CameraSensorHelperFactory class, and implements a\nconstructor and createInstance() function. Replace the code generation\nthrough macro with the C++ equivalent, a class template, as done by the\nAlgorithm factory.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/ipa/ipu3/ipu3.cpp | 2 +-\n src/ipa/libipa/camera_sensor_helper.cpp | 79 +++++++++++++++----------\n src/ipa/libipa/camera_sensor_helper.h | 43 ++++++++------\n src/ipa/rkisp1/rkisp1.cpp | 2 +-\n 4 files changed, 75 insertions(+), 51 deletions(-)", "diff": "diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\nindex b93a09d40c39..852deb710956 100644\n--- a/src/ipa/ipu3/ipu3.cpp\n+++ b/src/ipa/ipu3/ipu3.cpp\n@@ -326,7 +326,7 @@ int IPAIPU3::init(const IPASettings &settings,\n \t\t const ControlInfoMap &sensorControls,\n \t\t ControlInfoMap *ipaControls)\n {\n-\tcamHelper_ = CameraSensorHelperFactory::create(settings.sensorModel);\n+\tcamHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);\n \tif (camHelper_ == nullptr) {\n \t\tLOG(IPAIPU3, Error)\n \t\t\t<< \"Failed to create camera sensor helper for \"\ndiff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\nindex 3a7d701d8616..e655be255f2b 100644\n--- a/src/ipa/libipa/camera_sensor_helper.cpp\n+++ b/src/ipa/libipa/camera_sensor_helper.cpp\n@@ -43,7 +43,8 @@ namespace ipa {\n * \\brief Construct a CameraSensorHelper instance\n *\n * CameraSensorHelper derived class instances shall never be constructed\n- * manually but always through the CameraSensorHelperFactory::create() function.\n+ * manually but always through the CameraSensorHelperFactoryBase::create()\n+ * function.\n */\n \n /**\n@@ -217,27 +218,25 @@ double CameraSensorHelper::gain(uint32_t gainCode) const\n */\n \n /**\n- * \\class CameraSensorHelperFactory\n- * \\brief Registration of CameraSensorHelperFactory classes and creation of instances\n+ * \\class CameraSensorHelperFactoryBase\n+ * \\brief Base class for camera sensor helper factories\n *\n- * To facilitate discovery and instantiation of CameraSensorHelper classes, the\n- * CameraSensorHelperFactory class maintains a registry of camera sensor helper\n- * sub-classes. Each CameraSensorHelper subclass shall register itself using the\n- * REGISTER_CAMERA_SENSOR_HELPER() macro, which will create a corresponding\n- * instance of a CameraSensorHelperFactory subclass and register it with the\n- * static list of factories.\n+ * The CameraSensorHelperFactoryBase class is the base of all specializations of\n+ * the CameraSensorHelperFactory class template. It implements the factory\n+ * registration, maintains a registry of factories, and provides access to the\n+ * registered factories.\n */\n \n /**\n- * \\brief Construct a camera sensor helper factory\n+ * \\brief Construct a camera sensor helper factory base\n * \\param[in] name Name of the camera sensor helper class\n *\n- * Creating an instance of the factory registers it with the global list of\n+ * Creating an instance of the factory base registers it with the global list of\n * factories, accessible through the factories() function.\n *\n * The factory \\a name is used for debug purpose and shall be unique.\n */\n-CameraSensorHelperFactory::CameraSensorHelperFactory(const std::string name)\n+CameraSensorHelperFactoryBase::CameraSensorHelperFactoryBase(const std::string name)\n \t: name_(name)\n {\n \tregisterType(this);\n@@ -252,12 +251,12 @@ CameraSensorHelperFactory::CameraSensorHelperFactory(const std::string name)\n * corresponding to the named factory or a null pointer if no such factory\n * exists\n */\n-std::unique_ptr<CameraSensorHelper> CameraSensorHelperFactory::create(const std::string &name)\n+std::unique_ptr<CameraSensorHelper> CameraSensorHelperFactoryBase::create(const std::string &name)\n {\n-\tconst std::vector<CameraSensorHelperFactory *> &factories =\n-\t\tCameraSensorHelperFactory::factories();\n+\tconst std::vector<CameraSensorHelperFactoryBase *> &factories =\n+\t\tCameraSensorHelperFactoryBase::factories();\n \n-\tfor (const CameraSensorHelperFactory *factory : factories) {\n+\tfor (const CameraSensorHelperFactoryBase *factory : factories) {\n \t\tif (name != factory->name_)\n \t\t\tcontinue;\n \n@@ -274,10 +273,10 @@ std::unique_ptr<CameraSensorHelper> CameraSensorHelperFactory::create(const std:\n * The caller is responsible to guarantee the uniqueness of the camera sensor\n * helper name.\n */\n-void CameraSensorHelperFactory::registerType(CameraSensorHelperFactory *factory)\n+void CameraSensorHelperFactoryBase::registerType(CameraSensorHelperFactoryBase *factory)\n {\n-\tstd::vector<CameraSensorHelperFactory *> &factories =\n-\t\tCameraSensorHelperFactory::factories();\n+\tstd::vector<CameraSensorHelperFactoryBase *> &factories =\n+\t\tCameraSensorHelperFactoryBase::factories();\n \n \tfactories.push_back(factory);\n }\n@@ -286,35 +285,55 @@ void CameraSensorHelperFactory::registerType(CameraSensorHelperFactory *factory)\n * \\brief Retrieve the list of all camera sensor helper factories\n * \\return The list of camera sensor helper factories\n */\n-std::vector<CameraSensorHelperFactory *> &CameraSensorHelperFactory::factories()\n+std::vector<CameraSensorHelperFactoryBase *> &CameraSensorHelperFactoryBase::factories()\n {\n \t/*\n \t * The static factories map is defined inside the function to ensure\n \t * it gets initialized on first use, without any dependency on link\n \t * order.\n \t */\n-\tstatic std::vector<CameraSensorHelperFactory *> factories;\n+\tstatic std::vector<CameraSensorHelperFactoryBase *> factories;\n \treturn factories;\n }\n \n+/**\n+ * \\var CameraSensorHelperFactoryBase::name_\n+ * \\brief The name of the factory\n+ */\n+\n+/**\n+ * \\class CameraSensorHelperFactory\n+ * \\brief Registration of CameraSensorHelperFactory classes and creation of instances\n+ * \\tparam _Helper The camera sensor helepr class type for this factory\n+ *\n+ * To facilitate discovery and instantiation of CameraSensorHelper classes, the\n+ * CameraSensorHelperFactory class implements auto-registration of camera sensor\n+ * helpers. Each CameraSensorHelper subclass shall register itself using the\n+ * REGISTER_CAMERA_SENSOR_HELPER() macro, which will create a corresponding\n+ * instance of a CameraSensorHelperFactory subclass and register it with the\n+ * static list of factories.\n+ */\n+\n+/**\n+ * \\fn CameraSensorHelperFactory::CameraSensorHelperFactory(const char *name)\n+ * \\brief Construct a camera sensor helper factory\n+ * \\param[in] name Name of the camera sensor helper class\n+ *\n+ * Creating an instance of the factory registers it with the global list of\n+ * factories, accessible through the factories() function.\n+ *\n+ * The factory \\a name is used for debug purpose and shall be unique.\n+ */\n+\n /**\n * \\fn CameraSensorHelperFactory::createInstance() const\n * \\brief Create an instance of the CameraSensorHelper corresponding to the\n * factory\n *\n- * This virtual function is implemented by the REGISTER_CAMERA_SENSOR_HELPER()\n- * macro. It creates a camera sensor helper instance associated with the camera\n- * sensor model.\n- *\n * \\return A unique pointer to a newly constructed instance of the\n * CameraSensorHelper subclass corresponding to the factory\n */\n \n-/**\n- * \\var CameraSensorHelperFactory::name_\n- * \\brief The name of the factory\n- */\n-\n /**\n * \\def REGISTER_CAMERA_SENSOR_HELPER\n * \\brief Register a camera sensor helper with the camera sensor helper factory\ndiff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h\nindex 21ee43cc9f9f..3ea1806cb1fd 100644\n--- a/src/ipa/libipa/camera_sensor_helper.h\n+++ b/src/ipa/libipa/camera_sensor_helper.h\n@@ -58,39 +58,44 @@ private:\n \tLIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorHelper)\n };\n \n-class CameraSensorHelperFactory\n+class CameraSensorHelperFactoryBase\n {\n public:\n-\tCameraSensorHelperFactory(const std::string name);\n-\tvirtual ~CameraSensorHelperFactory() = default;\n+\tCameraSensorHelperFactoryBase(const std::string name);\n+\tvirtual ~CameraSensorHelperFactoryBase() = default;\n \n \tstatic std::unique_ptr<CameraSensorHelper> create(const std::string &name);\n \n-\tstatic std::vector<CameraSensorHelperFactory *> &factories();\n+\tstatic std::vector<CameraSensorHelperFactoryBase *> &factories();\n \n private:\n-\tLIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorHelperFactory)\n+\tLIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorHelperFactoryBase)\n \n-\tstatic void registerType(CameraSensorHelperFactory *factory);\n+\tstatic void registerType(CameraSensorHelperFactoryBase *factory);\n \n \tvirtual std::unique_ptr<CameraSensorHelper> createInstance() const = 0;\n \n \tstd::string name_;\n };\n \n-#define REGISTER_CAMERA_SENSOR_HELPER(name, helper)\t\t\\\n-class helper##Factory final : public CameraSensorHelperFactory\t\\\n-{\t\t\t\t\t\t\t\t\\\n-public: \t\t\t\t\t\t\t\\\n-\thelper##Factory() : CameraSensorHelperFactory(name) {}\t\\\n-\t\t\t\t\t\t\t\t\\\n-private:\t\t\t\t\t\t\t\\\n-\tstd::unique_ptr<CameraSensorHelper> createInstance() const \\\n-\t{\t\t\t\t\t\t\t\\\n-\t\treturn std::make_unique<helper>();\t\t\\\n-\t}\t\t\t\t\t\t\t\\\n-};\t\t\t\t\t\t\t\t\\\n-static helper##Factory global_##helper##Factory;\n+template<typename _Helper>\n+class CameraSensorHelperFactory final : public CameraSensorHelperFactoryBase\n+{\n+public:\n+\tCameraSensorHelperFactory(const char *name)\n+\t\t: CameraSensorHelperFactoryBase(name)\n+\t{\n+\t}\n+\n+private:\n+\tstd::unique_ptr<CameraSensorHelper> createInstance() const\n+\t{\n+\t\treturn std::make_unique<_Helper>();\n+\t}\n+};\n+\n+#define REGISTER_CAMERA_SENSOR_HELPER(name, helper) \\\n+static CameraSensorHelperFactory<helper> global_##helper##Factory(name);\n \n } /* namespace ipa */\n \ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 1335e3d14df2..9451cb03a285 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -143,7 +143,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n \t/* Cache the value to set it in configure. */\n \thwRevision_ = static_cast<rkisp1_cif_isp_version>(hwRevision);\n \n-\tcamHelper_ = CameraSensorHelperFactory::create(settings.sensorModel);\n+\tcamHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);\n \tif (!camHelper_) {\n \t\tLOG(IPARkISP1, Error)\n \t\t\t<< \"Failed to create camera sensor helper for \"\n", "prefixes": [ "libcamera-devel", "4/8" ] }