{"id":17514,"url":"https://patchwork.libcamera.org/api/1.1/patches/17514/?format=json","web_url":"https://patchwork.libcamera.org/patch/17514/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","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=json","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=json","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"]}