Patch Detail
Show a patch.
GET /api/1.1/patches/3559/?format=api
{ "id": 3559, "url": "https://patchwork.libcamera.org/api/1.1/patches/3559/?format=api", "web_url": "https://patchwork.libcamera.org/patch/3559/", "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": "<20200427031713.14013-8-laurent.pinchart@ideasonboard.com>", "date": "2020-04-27T03:17:09", "name": "[libcamera-devel,07/11] ipa: Pass IPA initialization settings to IPAInterface::init()", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "afce2004cba66ad7babc871c24012dd1dc3d3c59", "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/3559/mbox/", "series": [ { "id": 831, "url": "https://patchwork.libcamera.org/api/1.1/series/831/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=831", "date": "2020-04-27T03:17:02", "name": "libcamera: Add support for IPA configuration", "version": 1, "mbox": "https://patchwork.libcamera.org/series/831/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/3559/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/3559/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7958560B02\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Apr 2020 05:17:35 +0200 (CEST)", "from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1253572C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Apr 2020 05:17:35 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rjBTTwQW\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587957455;\n\tbh=yTQIcQx7iiYii1WpyFu45u9ADXFUWy/YU6VYuN22hCc=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=rjBTTwQWLg1wZ0nmNqdbXBVxFGwdXOIINXC6hGn05HYkC7B3tupRVnzHvs8cMvYGa\n\tdEkGO1u02X7hOQGf1oUGXt3/INGGSY2VvwjN+MQya2BgUXbNModvnf8P4iekXeyRs5\n\tlCasCtcrD+MUbOYh08kyQoLvzFRZ91C9iRe0pbso=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 27 Apr 2020 06:17:09 +0300", "Message-Id": "<20200427031713.14013-8-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.25.3", "In-Reply-To": "<20200427031713.14013-1-laurent.pinchart@ideasonboard.com>", "References": "<20200427031713.14013-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 07/11] ipa: Pass IPA initialization\n\tsettings to IPAInterface::init()", "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>", "X-List-Received-Date": "Mon, 27 Apr 2020 03:17:37 -0000" }, "content": "Add a new IPASettings class to pass IPA initialization settings through\nthe IPAInterface::init() method. The settings currently only contain the\nname of a configuration file, and are expected to be extended later.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/ipa/ipa_interface.h | 13 +++++++--\n src/ipa/libipa/ipa_interface_wrapper.cpp | 8 ++++--\n src/ipa/libipa/ipa_interface_wrapper.h | 3 +-\n src/ipa/rkisp1/rkisp1.cpp | 2 +-\n src/ipa/vimc/vimc.cpp | 4 +--\n src/libcamera/include/ipa_context_wrapper.h | 2 +-\n src/libcamera/ipa_context_wrapper.cpp | 9 ++++--\n src/libcamera/ipa_interface.cpp | 32 +++++++++++++++++++++\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-\n src/libcamera/pipeline/vimc/vimc.cpp | 2 +-\n src/libcamera/proxy/ipa_proxy_linux.cpp | 2 +-\n src/libcamera/proxy/ipa_proxy_thread.cpp | 6 ++--\n test/ipa/ipa_interface_test.cpp | 3 +-\n test/ipa/ipa_wrappers_test.cpp | 13 +++++++--\n 14 files changed, 80 insertions(+), 21 deletions(-)", "diff": "diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h\nindex e65844bc7b34..ef3d6507b692 100644\n--- a/include/ipa/ipa_interface.h\n+++ b/include/ipa/ipa_interface.h\n@@ -18,6 +18,10 @@ struct ipa_context {\n \tconst struct ipa_context_ops *ops;\n };\n \n+struct ipa_settings {\n+\tconst char *configuration_file;\n+};\n+\n struct ipa_stream {\n \tunsigned int id;\n \tunsigned int pixel_format;\n@@ -63,7 +67,8 @@ struct ipa_callback_ops {\n struct ipa_context_ops {\n \tvoid (*destroy)(struct ipa_context *ctx);\n \tvoid *(*get_interface)(struct ipa_context *ctx);\n-\tvoid (*init)(struct ipa_context *ctx);\n+\tvoid (*init)(struct ipa_context *ctx,\n+\t\t const struct ipa_settings *settings);\n \tint (*start)(struct ipa_context *ctx);\n \tvoid (*stop)(struct ipa_context *ctx);\n \tvoid (*register_callbacks)(struct ipa_context *ctx,\n@@ -100,6 +105,10 @@ struct ipa_context *ipaCreate();\n \n namespace libcamera {\n \n+struct IPASettings {\n+\tstd::string configurationFile;\n+};\n+\n struct IPAStream {\n \tunsigned int pixelFormat;\n \tSize size;\n@@ -121,7 +130,7 @@ class IPAInterface\n public:\n \tvirtual ~IPAInterface() {}\n \n-\tvirtual int init() = 0;\n+\tvirtual int init(const IPASettings &settings) = 0;\n \tvirtual int start() = 0;\n \tvirtual void stop() = 0;\n \ndiff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\nindex f50f93a0185a..7776d9881575 100644\n--- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n@@ -79,11 +79,15 @@ void *IPAInterfaceWrapper::get_interface(struct ipa_context *_ctx)\n \treturn ctx->ipa_.get();\n }\n \n-void IPAInterfaceWrapper::init(struct ipa_context *_ctx)\n+void IPAInterfaceWrapper::init(struct ipa_context *_ctx,\n+\t\t\t const struct ipa_settings *settings)\n {\n \tIPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);\n \n-\tctx->ipa_->init();\n+\tIPASettings ipaSettings{\n+\t\t.configurationFile = std::string{ settings->configuration_file }\n+\t};\n+\tctx->ipa_->init(ipaSettings);\n }\n \n int IPAInterfaceWrapper::start(struct ipa_context *_ctx)\ndiff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h\nindex e4bc6dd4535d..78ccf0f59d42 100644\n--- a/src/ipa/libipa/ipa_interface_wrapper.h\n+++ b/src/ipa/libipa/ipa_interface_wrapper.h\n@@ -23,7 +23,8 @@ public:\n private:\n \tstatic void destroy(struct ipa_context *ctx);\n \tstatic void *get_interface(struct ipa_context *ctx);\n-\tstatic void init(struct ipa_context *ctx);\n+\tstatic void init(struct ipa_context *ctx,\n+\t\t\t const struct ipa_settings *settings);\n \tstatic int start(struct ipa_context *ctx);\n \tstatic void stop(struct ipa_context *ctx);\n \tstatic void register_callbacks(struct ipa_context *ctx,\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 7f0ffb0a791f..9a347e527cd2 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -32,7 +32,7 @@ LOG_DEFINE_CATEGORY(IPARkISP1)\n class IPARkISP1 : public IPAInterface\n {\n public:\n-\tint init() override { return 0; }\n+\tint init(const IPASettings &settings) override { return 0; }\n \tint start() override { return 0; }\n \tvoid stop() override {}\n \ndiff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\nindex cfdbd6f99155..e6bda8ec58b0 100644\n--- a/src/ipa/vimc/vimc.cpp\n+++ b/src/ipa/vimc/vimc.cpp\n@@ -31,7 +31,7 @@ public:\n \tIPAVimc();\n \t~IPAVimc();\n \n-\tint init() override;\n+\tint init(const IPASettings &settings) override;\n \n \tint start() override;\n \tvoid stop() override;\n@@ -61,7 +61,7 @@ IPAVimc::~IPAVimc()\n \t\t::close(fd_);\n }\n \n-int IPAVimc::init()\n+int IPAVimc::init(const IPASettings &settings)\n {\n \ttrace(IPAOperationInit);\n \ndiff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h\nindex 0a48bfe732c8..64395b4a450b 100644\n--- a/src/libcamera/include/ipa_context_wrapper.h\n+++ b/src/libcamera/include/ipa_context_wrapper.h\n@@ -19,7 +19,7 @@ public:\n \tIPAContextWrapper(struct ipa_context *context);\n \t~IPAContextWrapper();\n \n-\tint init() override;\n+\tint init(const IPASettings &settings) override;\n \tint start() override;\n \tvoid stop() override;\n \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\ndiff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\nindex ab6ce396b88a..5bd5d916ccc0 100644\n--- a/src/libcamera/ipa_context_wrapper.cpp\n+++ b/src/libcamera/ipa_context_wrapper.cpp\n@@ -69,15 +69,18 @@ IPAContextWrapper::~IPAContextWrapper()\n \tctx_->ops->destroy(ctx_);\n }\n \n-int IPAContextWrapper::init()\n+int IPAContextWrapper::init(const IPASettings &settings)\n {\n \tif (intf_)\n-\t\treturn intf_->init();\n+\t\treturn intf_->init(settings);\n \n \tif (!ctx_)\n \t\treturn 0;\n \n-\tctx_->ops->init(ctx_);\n+\tstruct ipa_settings c_settings;\n+\tc_settings.configuration_file = settings.configurationFile.c_str();\n+\n+\tctx_->ops->init(ctx_, &c_settings);\n \n \treturn 0;\n }\ndiff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\nindex 890d4340e4f3..3949b7d6ea5d 100644\n--- a/src/libcamera/ipa_interface.cpp\n+++ b/src/libcamera/ipa_interface.cpp\n@@ -92,6 +92,16 @@\n * \\brief The IPA context operations\n */\n \n+/**\n+ * \\struct ipa_settings\n+ * \\brief IPA initialization settings for the IPA context operations\n+ * \\sa IPASettings\n+ *\n+ * \\var ipa_settings::configuration_file\n+ * \\brief The name of the IPA configuration file (may be null or point to an\n+ * empty string)\n+ */\n+\n /**\n * \\struct ipa_stream\n * \\brief Stream information for the IPA context operations\n@@ -231,6 +241,7 @@\n * \\var ipa_context_ops::init\n * \\brief Initialise the IPA context\n * \\param[in] ctx The IPA context\n+ * \\param[in] settings The IPA initialization settings\n *\n * \\sa libcamera::IPAInterface::init()\n */\n@@ -310,6 +321,22 @@\n \n namespace libcamera {\n \n+/**\n+ * \\struct IPASettings\n+ * \\brief IPA interface initialization settings\n+ *\n+ * The IPASettings structure stores data passed to the IPAInterface::init()\n+ * function.\n+ */\n+\n+/**\n+ * \\var IPASettings::configurationFile\n+ * \\brief The name of the IPA configuration file\n+ *\n+ * This field may be an empty string if the IPA doesn't require a configuration\n+ * file.\n+ */\n+\n /**\n * \\struct IPAStream\n * \\brief Stream configuration for the IPA interface\n@@ -424,6 +451,11 @@ namespace libcamera {\n /**\n * \\fn IPAInterface::init()\n * \\brief Initialise the IPAInterface\n+ * \\param[in] settings The IPA initialization settings\n+ *\n+ * This function initializes the IPA interface. It shall be called before any\n+ * other function of the IPAInterface. The \\a settings carry initialization\n+ * parameters that will remain valid for the whole life time of the interface.\n */\n \n /**\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex f42264361785..fde445b99a46 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -401,7 +401,7 @@ int RkISP1CameraData::loadIPA()\n \tipa_->queueFrameAction.connect(this,\n \t\t\t\t &RkISP1CameraData::queueFrameAction);\n \n-\tipa_->init();\n+\tipa_->init(IPASettings{});\n \n \treturn 0;\n }\ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex c5eea3a01b0e..699f788aa1b8 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -374,7 +374,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n \tif (data->ipa_ == nullptr)\n \t\tLOG(VIMC, Warning) << \"no matching IPA found\";\n \telse\n-\t\tdata->ipa_->init();\n+\t\tdata->ipa_->init(IPASettings{});\n \n \t/* Locate and open the capture video node. */\n \tif (data->init(media))\ndiff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\nindex 89f5cb54687b..cd8ac824679d 100644\n--- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n@@ -26,7 +26,7 @@ public:\n \tIPAProxyLinux(IPAModule *ipam);\n \t~IPAProxyLinux();\n \n-\tint init() override { return 0; }\n+\tint init(const IPASettings &settings) override { return 0; }\n \tint start() override { return 0; }\n \tvoid stop() override {}\n \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\ndiff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\nindex 1ebb9b6a6c9d..be82fde34bd9 100644\n--- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n@@ -25,7 +25,7 @@ class IPAProxyThread : public IPAProxy, public Object\n public:\n \tIPAProxyThread(IPAModule *ipam);\n \n-\tint init() override;\n+\tint init(const IPASettings &settings) override;\n \tint start() override;\n \tvoid stop() override;\n \n@@ -97,9 +97,9 @@ IPAProxyThread::IPAProxyThread(IPAModule *ipam)\n \tvalid_ = true;\n }\n \n-int IPAProxyThread::init()\n+int IPAProxyThread::init(const IPASettings &settings)\n {\n-\tint ret = ipa_->init();\n+\tint ret = ipa_->init(settings);\n \tif (ret)\n \t\treturn ret;\n \ndiff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\nindex 22f9ca41ef37..2e2dfb8d1ebd 100644\n--- a/test/ipa/ipa_interface_test.cpp\n+++ b/test/ipa/ipa_interface_test.cpp\n@@ -98,7 +98,8 @@ protected:\n \t\t}\n \n \t\t/* Test initialization of IPA module. */\n-\t\tipa_->init();\n+\t\tIPASettings settings;\n+\t\tipa_->init(settings);\n \t\ttimer.start(1000);\n \t\twhile (timer.isRunning() && trace_ != IPAOperationInit)\n \t\t\tdispatcher->processEvents();\ndiff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\nindex fae1d56b67c7..21bf51a2e046 100644\n--- a/test/ipa/ipa_wrappers_test.cpp\n+++ b/test/ipa/ipa_wrappers_test.cpp\n@@ -43,8 +43,14 @@ public:\n \t{\n \t}\n \n-\tint init() override\n+\tint init(const IPASettings &settings) override\n \t{\n+\t\tif (settings.configurationFile != \"/ipa/configuration/file\") {\n+\t\t\tcerr << \"init(): Invalid configuration file\" << endl;\n+\t\t\treport(Op_init, TestFail);\n+\t\t\treturn 0;\n+\t\t}\n+\n \t\treport(Op_init, TestPass);\n \t\treturn 0;\n \t}\n@@ -339,7 +345,10 @@ protected:\n \t\t * Test init(), start() and stop() last to ensure nothing in the\n \t\t * wrappers or serializer depends on them being called first.\n \t\t */\n-\t\tret = INVOKE(init);\n+\t\tIPASettings settings{\n+\t\t\t.configurationFile = \"/ipa/configuration/file\"\n+\t\t};\n+\t\tret = INVOKE(init, settings);\n \t\tif (ret == TestFail) {\n \t\t\tcerr << \"Failed to run init()\";\n \t\t\treturn TestFail;\n", "prefixes": [ "libcamera-devel", "07/11" ] }