{"id":9901,"url":"https://patchwork.libcamera.org/api/patches/9901/?format=json","web_url":"https://patchwork.libcamera.org/patch/9901/","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":"<20201002093309.109180-4-naush@raspberrypi.com>","date":"2020-10-02T09:33:07","name":"[libcamera-devel,3/5] libcamera: ipa: Pass a set of controls and return results from ipa::start()","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"a09d1a90211e94178c08f30293390b943899b387","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/9901/mbox/","series":[{"id":1345,"url":"https://patchwork.libcamera.org/api/series/1345/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1345","date":"2020-10-02T09:33:04","name":"RFQ: Pass controls on camera:start()","version":1,"mbox":"https://patchwork.libcamera.org/series/1345/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/9901/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/9901/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 A3868C3B5B\n\tfor ;\n\tFri, 2 Oct 2020 09:33:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6929D63B2C;\n\tFri, 2 Oct 2020 11:33:18 +0200 (CEST)","from mail-wr1-x434.google.com (mail-wr1-x434.google.com\n\t[IPv6:2a00:1450:4864:20::434])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 86539623B6\n\tfor ;\n\tFri, 2 Oct 2020 11:33:16 +0200 (CEST)","by mail-wr1-x434.google.com with SMTP id s12so1008127wrw.11\n\tfor ;\n\tFri, 02 Oct 2020 02:33:16 -0700 (PDT)","from naushir-VirtualBox.pitowers.org\n\t([2a00:1098:3142:14:a00:27ff:fe4d:f6a2])\n\tby smtp.gmail.com with ESMTPSA id\n\tt203sm1101145wmg.43.2020.10.02.02.33.14\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 02 Oct 2020 02:33:15 -0700 (PDT)"],"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=\"OXfanorP\"; 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=F2qOlvN/lG1JG+AotBTh+2Tcwda21PM/APIMEEI8NjI=;\n\tb=OXfanorP8vu3vq1C8I7Pk8+154RZw2ipoK4nleGJHVQu3cFmIH/FHbc6S+hza81pyR\n\t0K8SYzOVMy/+1waKuWKBcmodvVxR8VaCdkUCxB6sHuilvnkHsK7nxt+BegJwSOTlr6W9\n\t8oDr5HWAKWw1aG36M7mm/atK8TIxeTGY+lOsF6yOgEv5QM1QWISGL9Tlf2VJR0WQTpxS\n\toXskjaB8yjlFrqtc/xojjDyQhDZW+f4QsonGfBFKyDE7LoXAsBZrsAazFM6Hwp2kqFAw\n\tV1119BWvdsSYmu4xUyIbv+mfJFMPIYT0mQgII4vwrQF6HUico7Flw8pyFBPO3iS19zLQ\n\tXP8Q==","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=F2qOlvN/lG1JG+AotBTh+2Tcwda21PM/APIMEEI8NjI=;\n\tb=kYNlCGFAM+Pqbzyb8fuSTzBO/cjaT1BSmssPpWC+IyYB+rV4kec7naASlJV0eR86eA\n\t/a1R6LWZ8x4Q3GOd1msuDD+pdmUC9VGcZAMNbyG24KJUibLJk6e0sfbNXZjnWquD0ur+\n\tOlCi2jPrfQtLrV5jgKa4esO54TpW/xzg1c8dTrnwTmY7gBBsFoIkVIHwg12nbOljCGZF\n\tmL3SUTy2F+2nnrPcbafOgpHdLquDt2FSfrJUWmo9EFWfu+IPmc+7Bs5GrKLrEFmQLuCU\n\tqC8k3aPN/aw8YZeUqgC/2o74Qlx1lFWWzwuTEJvDb7GtfWt5suYDiu1Vxv1BocK/9vYk\n\tNFRw==","X-Gm-Message-State":"AOAM530OrWmCSAlRsK7uVEjCuKqr/yOQxS3mNC3X7HnjxaiKnrWbKAbA\n\tl3/3wKaKofUsqXPFo8jrjEAis4S16sjO+w==","X-Google-Smtp-Source":"ABdhPJxWoz3qoEr/mPjZmsWsRpPF3H+9NwBH9RjPp3qcSbHyjeACoBPlFy64jzbpiYRxff7xch4c9w==","X-Received":"by 2002:adf:f7d0:: with SMTP id\n\ta16mr1869470wrq.381.1601631195667; \n\tFri, 02 Oct 2020 02:33:15 -0700 (PDT)","From":"Naushir Patuck ","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 2 Oct 2020 10:33:07 +0100","Message-Id":"<20201002093309.109180-4-naush@raspberrypi.com>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20201002093309.109180-1-naush@raspberrypi.com>","References":"<20201002093309.109180-1-naush@raspberrypi.com>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH 3/5] 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 5016ec25..ee7b2e87 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() {}\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 b0c7d1c1..9610229a 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 ef257762..07951c17 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 307ed9a2..2e0b0dcf 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -733,7 +733,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 15af7e5c..872bfc07 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 3b868e4b..8eb1998d 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 1bc93a63..a7bfcce8 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","3/5"]}