{"id":10413,"url":"https://patchwork.libcamera.org/api/patches/10413/?format=json","web_url":"https://patchwork.libcamera.org/patch/10413/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/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":"<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/people/34/?format=json","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/series/1457/?format=json","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":"","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 ;\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 ;\n\tThu, 12 Nov 2020 09:59:22 +0100 (CET)","by mail-wr1-x431.google.com with SMTP id b8so5155119wrn.0\n\tfor ;\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 ","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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"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 \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 &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(_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 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 &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"]}