Patch Detail
Show a patch.
GET /api/1.1/patches/10413/?format=api
{ "id": 10413, "url": "https://patchwork.libcamera.org/api/1.1/patches/10413/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10413/", "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": "<20201112085915.3053-3-naush@raspberrypi.com>", "date": "2020-11-12T08:59:14", "name": "[libcamera-devel,2/3] libcamera: ipa: Pass a set of controls and return results from ipa::start()", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "3d639396202ca8393efe775bab07b3bf778a9e6e", "submitter": { "id": 34, "url": "https://patchwork.libcamera.org/api/1.1/people/34/?format=api", "name": "Naushir Patuck", "email": "naush@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/10413/mbox/", "series": [ { "id": 1457, "url": "https://patchwork.libcamera.org/api/1.1/series/1457/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1457", "date": "2020-11-12T08:59:12", "name": "Pass controls on camera:start()", "version": 1, "mbox": "https://patchwork.libcamera.org/series/1457/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10413/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10413/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 71300BE082\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Nov 2020 08:59:24 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6DCFD63143;\n\tThu, 12 Nov 2020 09:59:23 +0100 (CET)", "from mail-wr1-x431.google.com (mail-wr1-x431.google.com\n\t[IPv6:2a00:1450:4864:20::431])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E2BB6033E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Nov 2020 09:59:22 +0100 (CET)", "by mail-wr1-x431.google.com with SMTP id b8so5155119wrn.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Nov 2020 00:59:22 -0800 (PST)", "from naushir-VirtualBox.pitowers.org\n\t([2a00:1098:3142:14:a00:27ff:fe4d:f6a2])\n\tby smtp.gmail.com with ESMTPSA id\n\tf17sm5641599wmf.41.2020.11.12.00.59.20\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 12 Nov 2020 00:59:20 -0800 (PST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Jk6lyOlF\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=S7k3cc1Sjv3QzhqAmVDYPdHSfgz5rIANOHcnJ3q9DCE=;\n\tb=Jk6lyOlFoC2ODLyE1a+Kp5k1I6YYf4pPMT7BrE2+HZFBZJkKtpnZG4AVPkfly28X+i\n\tlrMsRUdup+G/ufb7U1P6ykpjpWhL34xtmXVJ6oo9ZWxggP0ecpSrm3cBzH2rr0TQSeS3\n\tFYEw5cUVrM5y5q3qqqvBuQ9zLSDTvHV3Eq50alvZek1muI6UidgPw9I2WGDPmM7ZYSdc\n\tLp+9XfOrjXaiCBRh8OsMVzw7+b+yYFf5q2AXM3ypFNI9Iqk9Bufa/95xJ9Zwg+1u0n2Q\n\tZlozxQFsI6NwcaVwfcJnuUKY+jZYyDNJEqY4azFi3gN2lTH81cwe1NGyMsSLrQy0FZkT\n\ttWYA==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=S7k3cc1Sjv3QzhqAmVDYPdHSfgz5rIANOHcnJ3q9DCE=;\n\tb=bLrlXDbNEJs2aF/uJV1GqpTaqguYFXvBvcfupGmiJ83joOfXPvqqr6MqWJkR6FCrr5\n\taNlaj/KhW+K8vn/Y8uT1IOnBHHS1ZOo7u9idbky2+Big2SbTeDBzMgId+TE5EozTW0Q1\n\t0kAn+/iAGa9LmpVwgDfb201eqJj/gZNIhlz7UAAkeNEHEAb10hqQRHNPN7v0caa5FWWm\n\tB4ECIdX1IgwrBKPMILfvR7ZhX7l23YpHd2iFPN4BaoFm9SVhSqbnKZDzRDLHJRi1CaxK\n\tVjkJOYROwYJtM7cu364B5DNgCn8YnPOsEZjm4u9jd080cL8O6xu2p0o06479/p5cNZq0\n\tByfA==", "X-Gm-Message-State": "AOAM5332sgA66gUjIo3Ugi4uuutaq13uPN47Ia11w+aJ7kJuKMb6rLXu\n\tzVmTTI5LG8QPS3YFdQraA1YeK77VhGM7Jw==", "X-Google-Smtp-Source": "ABdhPJzdK6fNhaxEw2k/8/GpNXBicfYJ9PJ76wlp1oYQBbEbC8BafgezBvSK0s1PU7dZgfJ6e4bWvA==", "X-Received": "by 2002:a5d:6751:: with SMTP id\n\tl17mr20326779wrw.109.1605171561311; \n\tThu, 12 Nov 2020 00:59:21 -0800 (PST)", "From": "Naushir Patuck <naush@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 12 Nov 2020 08:59:14 +0000", "Message-Id": "<20201112085915.3053-3-naush@raspberrypi.com>", "X-Mailer": "git-send-email 2.25.1", "In-Reply-To": "<20201112085915.3053-1-naush@raspberrypi.com>", "References": "<20201112085915.3053-1-naush@raspberrypi.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH 2/3] libcamera: ipa: Pass a set of\n\tcontrols and return results from ipa::start()", "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>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "This change allows controls passed into pipeline_hanlder::start to be\nforwarded onto ipa::start(). We also add a return channel if the\npipeline handle must action any of these controls, e.g. setting the\nanalogue gain or shutter speed in the sensor device.\n\ntodo: The IPA interface wrapper needs a translation for passing\nIPAOperationData into start() and configure()\n\nSigned-off-by: Naushir Patuck <naush@raspberrypi.com>\n---\n include/libcamera/internal/ipa_context_wrapper.h | 3 ++-\n include/libcamera/ipa/ipa_interface.h | 3 ++-\n src/ipa/libipa/ipa_interface_wrapper.cpp | 4 +++-\n src/ipa/raspberrypi/raspberrypi.cpp | 3 ++-\n src/ipa/rkisp1/rkisp1.cpp | 3 ++-\n src/ipa/vimc/vimc.cpp | 6 ++++--\n src/libcamera/ipa_context_wrapper.cpp | 6 ++++--\n src/libcamera/ipa_interface.cpp | 7 +++++++\n src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++-\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++--\n src/libcamera/pipeline/vimc/vimc.cpp | 3 ++-\n src/libcamera/proxy/ipa_proxy_linux.cpp | 3 ++-\n src/libcamera/proxy/ipa_proxy_thread.cpp | 13 ++++++++-----\n test/ipa/ipa_interface_test.cpp | 3 ++-\n test/ipa/ipa_wrappers_test.cpp | 5 +++--\n 15 files changed, 49 insertions(+), 22 deletions(-)", "diff": "diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h\nindex 8f767e84..1878a615 100644\n--- a/include/libcamera/internal/ipa_context_wrapper.h\n+++ b/include/libcamera/internal/ipa_context_wrapper.h\n@@ -20,7 +20,8 @@ public:\n \t~IPAContextWrapper();\n \n \tint init(const IPASettings &settings) override;\n-\tint start() override;\n+\tint start(const IPAOperationData &ipaConfig,\n+\t\t IPAOperationData *result) override;\n \tvoid stop() override;\n \tvoid configure(const CameraSensorInfo &sensorInfo,\n \t\t const std::map<unsigned int, IPAStream> &streamConfig,\ndiff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\nindex 322b7079..b44e2538 100644\n--- a/include/libcamera/ipa/ipa_interface.h\n+++ b/include/libcamera/ipa/ipa_interface.h\n@@ -153,7 +153,8 @@ public:\n \tvirtual ~IPAInterface() = default;\n \n \tvirtual int init(const IPASettings &settings) = 0;\n-\tvirtual int start() = 0;\n+\tvirtual int start(const IPAOperationData &ipaConfig,\n+\t\t\t IPAOperationData *result) = 0;\n \tvirtual void stop() = 0;\n \n \tvirtual void configure(const CameraSensorInfo &sensorInfo,\ndiff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\nindex cee532e3..fb2da9d3 100644\n--- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n@@ -95,7 +95,9 @@ int IPAInterfaceWrapper::start(struct ipa_context *_ctx)\n {\n \tIPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);\n \n-\treturn ctx->ipa_->start();\n+\t/* \\todo Translate the ipaConfig and result. */\n+\tIPAOperationData ipaConfig;\n+\treturn ctx->ipa_->start(ipaConfig, nullptr);\n }\n \n void IPAInterfaceWrapper::stop(struct ipa_context *_ctx)\ndiff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex f338ff8b..7a07b477 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -77,7 +77,8 @@ public:\n \t}\n \n \tint init(const IPASettings &settings) override;\n-\tint start() override { return 0; }\n+\tint start([[maybe_unused]] const IPAOperationData &ipaConfig,\n+\t\t [[maybe_unused]] IPAOperationData *result) override { return 0; }\n \tvoid stop() override {}\n \n \tvoid configure(const CameraSensorInfo &sensorInfo,\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 07d7f1b2..0b8a9096 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -37,7 +37,8 @@ public:\n \t{\n \t\treturn 0;\n \t}\n-\tint start() override { return 0; }\n+\tint start([[maybe_unused]] const IPAOperationData &ipaConfig,\n+\t\t [[maybe_unused]] IPAOperationData *result) override { return 0; }\n \tvoid stop() override {}\n \n \tvoid configure(const CameraSensorInfo &info,\ndiff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\nindex cf841135..ed26331d 100644\n--- a/src/ipa/vimc/vimc.cpp\n+++ b/src/ipa/vimc/vimc.cpp\n@@ -34,7 +34,8 @@ public:\n \n \tint init(const IPASettings &settings) override;\n \n-\tint start() override;\n+\tint start(const IPAOperationData &ipaConfig,\n+\t\t IPAOperationData *result) override;\n \tvoid stop() override;\n \n \tvoid configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,\n@@ -82,7 +83,8 @@ int IPAVimc::init(const IPASettings &settings)\n \treturn 0;\n }\n \n-int IPAVimc::start()\n+int IPAVimc::start([[maybe_unused]] const IPAOperationData &ipaConfig,\n+\t\t [[maybe_unused]] IPAOperationData *result)\n {\n \ttrace(IPAOperationStart);\n \ndiff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\nindex 231300ce..de96a606 100644\n--- a/src/libcamera/ipa_context_wrapper.cpp\n+++ b/src/libcamera/ipa_context_wrapper.cpp\n@@ -86,14 +86,16 @@ int IPAContextWrapper::init(const IPASettings &settings)\n \treturn 0;\n }\n \n-int IPAContextWrapper::start()\n+int IPAContextWrapper::start(const IPAOperationData &ipaConfig,\n+\t\t\t IPAOperationData *result)\n {\n \tif (intf_)\n-\t\treturn intf_->start();\n+\t\treturn intf_->start(ipaConfig, result);\n \n \tif (!ctx_)\n \t\treturn 0;\n \n+\t/* \\todo Translate the ipaConfig and reponse */\n \treturn ctx_->ops->start(ctx_);\n }\n \ndiff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\nindex 23fc56d7..282c3c0f 100644\n--- a/src/libcamera/ipa_interface.cpp\n+++ b/src/libcamera/ipa_interface.cpp\n@@ -536,10 +536,17 @@ namespace libcamera {\n /**\n * \\fn IPAInterface::start()\n * \\brief Start the IPA\n+ * \\param[in] ipaConfig Pipeline-handler-specific configuration data\n+ * \\param[out] result Pipeline-handler-specific configuration result\n *\n * This method informs the IPA module that the camera is about to be started.\n * The IPA module shall prepare any resources it needs to operate.\n *\n+ * The \\a ipaConfig and \\a result parameters carry custom data passed by the\n+ * pipeline handler to the IPA and back. The pipeline handler may set the \\a\n+ * result parameter to null if the IPA protocol doesn't need to pass a result\n+ * back through the configure() function.\n+ *\n * \\return 0 on success or a negative error code otherwise\n */\n \ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex ddb30e49..a8e4997a 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -747,7 +747,9 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont\n \t}\n \n \t/* Start the IPA. */\n-\tret = data->ipa_->start();\n+\tIPAOperationData ipaConfig, result;\n+\tipaConfig.controls.emplace_back(*controls);\n+\tret = data->ipa_->start(ipaConfig, &result);\n \tif (ret) {\n \t\tLOG(RPI, Error)\n \t\t\t<< \"Failed to start IPA for \" << camera->id();\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 2e8d2930..a6c82a48 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -832,7 +832,8 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n \tif (ret)\n \t\treturn ret;\n \n-\tret = data->ipa_->start();\n+\tIPAOperationData ipaConfig = {};\n+\tret = data->ipa_->start(ipaConfig, nullptr);\n \tif (ret) {\n \t\tfreeBuffers(camera);\n \t\tLOG(RkISP1, Error)\n@@ -911,7 +912,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n \tstd::map<unsigned int, const ControlInfoMap &> entityControls;\n \tentityControls.emplace(0, data->sensor_->controls());\n \n-\tIPAOperationData ipaConfig;\n+\tipaConfig = {};\n \tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n \t\t\t ipaConfig, nullptr);\n \ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex d81b8598..b4773cf5 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -322,7 +322,8 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con\n \tif (ret < 0)\n \t\treturn ret;\n \n-\tret = data->ipa_->start();\n+\tIPAOperationData ipaConfig = {};\n+\tret = data->ipa_->start(ipaConfig, nullptr);\n \tif (ret) {\n \t\tdata->video_->releaseBuffers();\n \t\treturn ret;\ndiff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\nindex b78a0e45..619976e5 100644\n--- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n@@ -30,7 +30,8 @@ public:\n \t{\n \t\treturn 0;\n \t}\n-\tint start() override { return 0; }\n+\tint start([[maybe_unused]] const IPAOperationData &ipaConfig,\n+\t\t [[maybe_unused]] IPAOperationData *result) override { return 0; }\n \tvoid stop() override {}\n \tvoid configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,\n \t\t [[maybe_unused]] 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 eead2883..9a81b6e7 100644\n--- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n@@ -26,7 +26,8 @@ public:\n \tIPAProxyThread(IPAModule *ipam);\n \n \tint init(const IPASettings &settings) override;\n-\tint start() override;\n+\tint start(const IPAOperationData &ipaConfig,\n+\t\t IPAOperationData *result) override;\n \tvoid stop() override;\n \n \tvoid configure(const CameraSensorInfo &sensorInfo,\n@@ -50,9 +51,9 @@ private:\n \t\t\tipa_ = ipa;\n \t\t}\n \n-\t\tint start()\n+\t\tint start(const IPAOperationData &ipaConfig, IPAOperationData *result)\n \t\t{\n-\t\t\treturn ipa_->start();\n+\t\t\treturn ipa_->start(ipaConfig, result);\n \t\t}\n \n \t\tvoid stop()\n@@ -111,12 +112,14 @@ int IPAProxyThread::init(const IPASettings &settings)\n \treturn 0;\n }\n \n-int IPAProxyThread::start()\n+int IPAProxyThread::start(const IPAOperationData &ipaConfig,\n+\t\t\t IPAOperationData *result)\n {\n \trunning_ = true;\n \tthread_.start();\n \n-\treturn proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking);\n+\treturn proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking,\n+\t\t\t\t ipaConfig, result);\n }\n \n void IPAProxyThread::stop()\ndiff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\nindex 67488409..6222938f 100644\n--- a/test/ipa/ipa_interface_test.cpp\n+++ b/test/ipa/ipa_interface_test.cpp\n@@ -120,7 +120,8 @@ protected:\n \t\t}\n \n \t\t/* Test start of IPA module. */\n-\t\tipa_->start();\n+\t\tIPAOperationData ipaConfig;\n+\t\tipa_->start(ipaConfig, nullptr);\n \t\ttimer.start(1000);\n \t\twhile (timer.isRunning() && trace_ != IPAOperationStart)\n \t\t\tdispatcher->processEvents();\ndiff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\nindex 59d991cb..7b8ae77b 100644\n--- a/test/ipa/ipa_wrappers_test.cpp\n+++ b/test/ipa/ipa_wrappers_test.cpp\n@@ -56,7 +56,8 @@ public:\n \t\treturn 0;\n \t}\n \n-\tint start() override\n+\tint start([[maybe_unused]] const IPAOperationData &ipaConfig,\n+\t\t [[maybe_unused]] IPAOperationData *result) override\n \t{\n \t\treport(Op_start, TestPass);\n \t\treturn 0;\n@@ -376,7 +377,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tret = INVOKE(start);\n+\t\tret = INVOKE(start, ipaConfig, nullptr);\n \t\tif (ret == TestFail) {\n \t\t\tcerr << \"Failed to run start()\";\n \t\t\treturn TestFail;\n", "prefixes": [ "libcamera-devel", "2/3" ] }