[{"id":12588,"web_url":"https://patchwork.libcamera.org/comment/12588/","msgid":"<20200919124322.GU1850958@oden.dyn.berto.se>","date":"2020-09-19T12:43:22","subject":"Re: [libcamera-devel] [PATCH 19/23] libcamera: IPAProxy: Remove\n\tregistration mechanism","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your work.\n\nOn 2020-09-15 23:20:34 +0900, Paul Elder wrote:\n> Implementations of IPA proxies use a registration mechanism to register\n> themselves with the main IPA proxy factory. This registration declares\n> static objects, causing a risk of things being constructed before the\n> proper libcamera facilities are ready. Since each pipeline handler has\n> its own IPA proxy and knows the type, it isn't necessary to have a proxy\n> factory. Remove it to alleviate the risk of early construction.\n> \n> Signed-off-by: Paul Elder \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> include/libcamera/internal/ipa_proxy.h | 29 ---------\n> src/libcamera/ipa_proxy.cpp | 85 --------------------------\n> 2 files changed, 114 deletions(-)\n> \n> diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h\n> index 1903150e..f651a3ae 100644\n> --- a/include/libcamera/internal/ipa_proxy.h\n> +++ b/include/libcamera/internal/ipa_proxy.h\n> @@ -36,35 +36,6 @@ private:\n> \tIPAModule *ipam_;\n> };\n> \n> -class IPAProxyFactory\n> -{\n> -public:\n> -\tIPAProxyFactory(const char *name);\n> -\tvirtual ~IPAProxyFactory() {}\n> -\n> -\tvirtual std::unique_ptr create(IPAModule *ipam, bool isolate) = 0;\n> -\n> -\tconst std::string &name() const { return name_; }\n> -\n> -\tstatic void registerType(IPAProxyFactory *factory);\n> -\tstatic std::vector &factories();\n> -\n> -private:\n> -\tstd::string name_;\n> -};\n> -\n> -#define REGISTER_IPA_PROXY(proxy)\t\t\t\t\t\\\n> -class proxy##Factory final : public IPAProxyFactory\t\t\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -public:\t\t\t\t\t\t\t\t\t\\\n> -\tproxy##Factory() : IPAProxyFactory(#proxy) {}\t\t\t\\\n> -\tstd::unique_ptr create(IPAModule *ipam, bool isolate)\t\\\n> -\t{\t\t\t\t\t\t\t\t\\\n> -\t\treturn std::make_unique(ipam, isolate);\t\t\\\n> -\t}\t\t\t\t\t\t\t\t\\\n> -};\t\t\t\t\t\t\t\t\t\\\n> -static proxy##Factory global_##proxy##Factory;\n> -\n> } /* namespace libcamera */\n> \n> #endif /* __LIBCAMERA_INTERNAL_IPA_PROXY_H__ */\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index ff4d7fd1..e7d4db06 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -229,89 +229,4 @@ std::string IPAProxy::resolvePath(const std::string &file) const\n> * construction.\n> */\n> \n> -/**\n> - * \\class IPAProxyFactory\n> - * \\brief Registration of IPAProxy classes and creation of instances\n> - *\n> - * To facilitate discovery and instantiation of IPAProxy classes, the\n> - * IPAProxyFactory class maintains a registry of IPAProxy classes. Each\n> - * IPAProxy subclass shall register itself using the REGISTER_IPA_PROXY()\n> - * macro, which will create a corresponding instance of a IPAProxyFactory\n> - * subclass and register it with the static list of factories.\n> - */\n> -\n> -/**\n> - * \\brief Construct a IPAProxy factory\n> - * \\param[in] name Name of the IPAProxy class\n> - *\n> - * Creating an instance of the factory registers is with the global list of\n> - * factories, accessible through the factories() function.\n> - *\n> - * The factory \\a name is used for debugging and IPAProxy matching purposes\n> - * and shall be unique.\n> - */\n> -IPAProxyFactory::IPAProxyFactory(const char *name)\n> -\t: name_(name)\n> -{\n> -\tregisterType(this);\n> -}\n> -\n> -/**\n> - * \\fn IPAProxyFactory::create()\n> - * \\brief Create an instance of the IPAProxy corresponding to the factory\n> - * \\param[in] ipam The IPA module\n> - *\n> - * This virtual function is implemented by the REGISTER_IPA_PROXY() macro.\n> - * It creates a IPAProxy instance that isolates an IPA interface designated\n> - * by the IPA module \\a ipam.\n> - *\n> - * \\return A pointer to a newly constructed instance of the IPAProxy subclass\n> - * corresponding to the factory\n> - */\n> -\n> -/**\n> - * \\fn IPAProxyFactory::name()\n> - * \\brief Retrieve the factory name\n> - * \\return The factory name\n> - */\n> -\n> -/**\n> - * \\brief Add a IPAProxy class to the registry\n> - * \\param[in] factory Factory to use to construct the IPAProxy\n> - *\n> - * The caller is responsible to guarantee the uniqueness of the IPAProxy name.\n> - */\n> -void IPAProxyFactory::registerType(IPAProxyFactory *factory)\n> -{\n> -\tstd::vector &factories = IPAProxyFactory::factories();\n> -\n> -\tfactories.push_back(factory);\n> -\n> -\tLOG(IPAProxy, Debug)\n> -\t\t<< \"Registered proxy \\\"\" << factory->name() << \"\\\"\";\n> -}\n> -\n> -/**\n> - * \\brief Retrieve the list of all IPAProxy factories\n> - *\n> - * The static factories map is defined inside the function to ensure it gets\n> - * initialized on first use, without any dependency on link order.\n> - *\n> - * \\return The list of pipeline handler factories\n> - */\n> -std::vector &IPAProxyFactory::factories()\n> -{\n> -\tstatic std::vector factories;\n> -\treturn factories;\n> -}\n> -\n> -/**\n> - * \\def REGISTER_IPA_PROXY\n> - * \\brief Register a IPAProxy with the IPAProxy factory\n> - * \\param[in] proxy Class name of IPAProxy derived class to register\n> - *\n> - * Register a proxy subclass with the factory and make it available to\n> - * isolate IPA modules.\n> - */\n> -\n> } /* namespace libcamera */\n> -- \n> 2.27.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","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 AE07EBF01C\n\tfor ;\n\tSat, 19 Sep 2020 12:43:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B5DC62FBC;\n\tSat, 19 Sep 2020 14:43:24 +0200 (CEST)","from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9F70560367\n\tfor ;\n\tSat, 19 Sep 2020 14:43:23 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id u8so9068415lff.1\n\tfor ;\n\tSat, 19 Sep 2020 05:43:23 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tq7sm1267399lfr.16.2020.09.19.05.43.22\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 19 Sep 2020 05:43:22 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"VeKWCn1N\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=dhQcIoMr4SnCjCsA4IuUJv+6L0PC6RPr9awV1iUJD2Y=;\n\tb=VeKWCn1N3G746wd+BX+09yi390bd4nZ6IvMiwKo8ckCeS+7l1sEBIZxB4n/eZslFlS\n\t/YIpx9FGu+pJEXmH7ZATaJbWY829/Vn8gXaFGeHKWHpLrZ3MBVb50KhLr/OYRYfxuxUj\n\tSFNbLPEYsqzVDqZZS0NFsGJGIRCl1k06s9+vVNQz2LA1LA0OGdkpgGFPDxzx7H0ezn3k\n\tn8kRuNFF9XPcTRY66sw8CDQ53vwWuiUPrfgQT8xNys9G0lDFvI7fFgaTOrwj82X8VrkA\n\tMw6MqQMDOVT6L3CIEcpML2olSDJbsdTONyPQkBeQTLSQiaD6EE6mPQq7YJtmSC998jRz\n\tJhxA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=dhQcIoMr4SnCjCsA4IuUJv+6L0PC6RPr9awV1iUJD2Y=;\n\tb=DePxH/HuKiud7lq54eaz9luAepwH61XEb4mOnNLFBwt8ynIa+8YZNRTD+7vdk2gkg1\n\tFB7XelZmgdlCfx+qpOtOxsBYmvKgKjrnUPWIgsDA0EYx7h75WlsoVzm6dN6mL3vlDjHh\n\t3wj9y54MwMDQnPnH00+M0mk16/oBHDFR+RH7wYvSFgXYl5puzM/XgNc8KHbx2FoV5KjH\n\tpJ+b5NGxDNEL+SeqED60RQdawxG5L2XSA4/X4bwotv2nZdNiSTZdg0VunJYooauipqQn\n\t1q2J3RLZddD1R11JdFxDDAkmETuyb9yR2H0sTohAuZDnH/CywWuJcbDUfUhMy8X5SVap\n\tLrkQ==","X-Gm-Message-State":"AOAM533qIpaMSdRlustIPTGz6SldRkN76qgXyTl9KQ3EbP72xLUA6ZH2\n\t+H4iUUTvm9zPc8k4ZBXe8/ok8g==","X-Google-Smtp-Source":"ABdhPJxyMgblm+dzK2tdSsH5y3CyUxlBLqVswQFW9k7rEYp6lMk9UOl5BZ8muaMa1ECAe0H7QNwVfg==","X-Received":"by 2002:a19:ca48:: with SMTP id\n\th8mr12356899lfj.173.1600519403042; \n\tSat, 19 Sep 2020 05:43:23 -0700 (PDT)","Date":"Sat, 19 Sep 2020 14:43:22 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Message-ID":"<20200919124322.GU1850958@oden.dyn.berto.se>","References":"<20200915142038.28757-1-paul.elder@ideasonboard.com>\n\t<20200915142038.28757-20-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200915142038.28757-20-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 19/23] libcamera: IPAProxy: Remove\n\tregistration mechanism","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]