From patchwork Thu Nov 12 08:59:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 10413 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 71300BE082 for ; Thu, 12 Nov 2020 08:59:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6DCFD63143; Thu, 12 Nov 2020 09:59:23 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="Jk6lyOlF"; dkim-atps=neutral Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E2BB6033E for ; Thu, 12 Nov 2020 09:59:22 +0100 (CET) Received: by mail-wr1-x431.google.com with SMTP id b8so5155119wrn.0 for ; Thu, 12 Nov 2020 00:59:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=S7k3cc1Sjv3QzhqAmVDYPdHSfgz5rIANOHcnJ3q9DCE=; b=Jk6lyOlFoC2ODLyE1a+Kp5k1I6YYf4pPMT7BrE2+HZFBZJkKtpnZG4AVPkfly28X+i lrMsRUdup+G/ufb7U1P6ykpjpWhL34xtmXVJ6oo9ZWxggP0ecpSrm3cBzH2rr0TQSeS3 FYEw5cUVrM5y5q3qqqvBuQ9zLSDTvHV3Eq50alvZek1muI6UidgPw9I2WGDPmM7ZYSdc Lp+9XfOrjXaiCBRh8OsMVzw7+b+yYFf5q2AXM3ypFNI9Iqk9Bufa/95xJ9Zwg+1u0n2Q ZlozxQFsI6NwcaVwfcJnuUKY+jZYyDNJEqY4azFi3gN2lTH81cwe1NGyMsSLrQy0FZkT tWYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=S7k3cc1Sjv3QzhqAmVDYPdHSfgz5rIANOHcnJ3q9DCE=; b=bLrlXDbNEJs2aF/uJV1GqpTaqguYFXvBvcfupGmiJ83joOfXPvqqr6MqWJkR6FCrr5 aNlaj/KhW+K8vn/Y8uT1IOnBHHS1ZOo7u9idbky2+Big2SbTeDBzMgId+TE5EozTW0Q1 0kAn+/iAGa9LmpVwgDfb201eqJj/gZNIhlz7UAAkeNEHEAb10hqQRHNPN7v0caa5FWWm B4ECIdX1IgwrBKPMILfvR7ZhX7l23YpHd2iFPN4BaoFm9SVhSqbnKZDzRDLHJRi1CaxK VjkJOYROwYJtM7cu364B5DNgCn8YnPOsEZjm4u9jd080cL8O6xu2p0o06479/p5cNZq0 ByfA== X-Gm-Message-State: AOAM5332sgA66gUjIo3Ugi4uuutaq13uPN47Ia11w+aJ7kJuKMb6rLXu zVmTTI5LG8QPS3YFdQraA1YeK77VhGM7Jw== X-Google-Smtp-Source: ABdhPJzdK6fNhaxEw2k/8/GpNXBicfYJ9PJ76wlp1oYQBbEbC8BafgezBvSK0s1PU7dZgfJ6e4bWvA== X-Received: by 2002:a5d:6751:: with SMTP id l17mr20326779wrw.109.1605171561311; Thu, 12 Nov 2020 00:59:21 -0800 (PST) Received: from naushir-VirtualBox.pitowers.org ([2a00:1098:3142:14:a00:27ff:fe4d:f6a2]) by smtp.gmail.com with ESMTPSA id f17sm5641599wmf.41.2020.11.12.00.59.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Nov 2020 00:59:20 -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 controls 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: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" This change allows controls passed into pipeline_hanlder::start to be forwarded onto ipa::start(). We also add a return channel if the pipeline handle must action any of these controls, e.g. setting the analogue gain or shutter speed in the sensor device. todo: The IPA interface wrapper needs a translation for passing IPAOperationData into start() and configure() Signed-off-by: Naushir Patuck Reviewed-by: David Plowman Tested-by: David Plowman --- include/libcamera/internal/ipa_context_wrapper.h | 3 ++- include/libcamera/ipa/ipa_interface.h | 3 ++- src/ipa/libipa/ipa_interface_wrapper.cpp | 4 +++- src/ipa/raspberrypi/raspberrypi.cpp | 3 ++- src/ipa/rkisp1/rkisp1.cpp | 3 ++- src/ipa/vimc/vimc.cpp | 6 ++++-- src/libcamera/ipa_context_wrapper.cpp | 6 ++++-- src/libcamera/ipa_interface.cpp | 7 +++++++ src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++-- src/libcamera/pipeline/vimc/vimc.cpp | 3 ++- src/libcamera/proxy/ipa_proxy_linux.cpp | 3 ++- src/libcamera/proxy/ipa_proxy_thread.cpp | 13 ++++++++----- test/ipa/ipa_interface_test.cpp | 3 ++- test/ipa/ipa_wrappers_test.cpp | 5 +++-- 15 files changed, 49 insertions(+), 22 deletions(-) diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h index 8f767e84..1878a615 100644 --- a/include/libcamera/internal/ipa_context_wrapper.h +++ b/include/libcamera/internal/ipa_context_wrapper.h @@ -20,7 +20,8 @@ public: ~IPAContextWrapper(); int init(const IPASettings &settings) override; - int start() override; + int start(const IPAOperationData &ipaConfig, + IPAOperationData *result) override; void stop() override; void configure(const CameraSensorInfo &sensorInfo, const std::map &streamConfig, diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h index 322b7079..b44e2538 100644 --- a/include/libcamera/ipa/ipa_interface.h +++ b/include/libcamera/ipa/ipa_interface.h @@ -153,7 +153,8 @@ public: virtual ~IPAInterface() = default; virtual int init(const IPASettings &settings) = 0; - virtual int start() = 0; + virtual int start(const IPAOperationData &ipaConfig, + IPAOperationData *result) = 0; virtual void stop() = 0; virtual void configure(const CameraSensorInfo &sensorInfo, diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp index cee532e3..fb2da9d3 100644 --- a/src/ipa/libipa/ipa_interface_wrapper.cpp +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp @@ -95,7 +95,9 @@ int IPAInterfaceWrapper::start(struct ipa_context *_ctx) { IPAInterfaceWrapper *ctx = static_cast(_ctx); - return ctx->ipa_->start(); + /* \todo Translate the ipaConfig and result. */ + IPAOperationData ipaConfig; + return ctx->ipa_->start(ipaConfig, nullptr); } void IPAInterfaceWrapper::stop(struct ipa_context *_ctx) diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index f338ff8b..7a07b477 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -77,7 +77,8 @@ public: } int init(const IPASettings &settings) override; - int start() override { return 0; } + int start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) override { return 0; } void stop() override {} void configure(const CameraSensorInfo &sensorInfo, diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 07d7f1b2..0b8a9096 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -37,7 +37,8 @@ public: { return 0; } - int start() override { return 0; } + int start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) override { return 0; } void stop() override {} void configure(const CameraSensorInfo &info, diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp index cf841135..ed26331d 100644 --- a/src/ipa/vimc/vimc.cpp +++ b/src/ipa/vimc/vimc.cpp @@ -34,7 +34,8 @@ public: int init(const IPASettings &settings) override; - int start() override; + int start(const IPAOperationData &ipaConfig, + IPAOperationData *result) override; void stop() override; void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo, @@ -82,7 +83,8 @@ int IPAVimc::init(const IPASettings &settings) return 0; } -int IPAVimc::start() +int IPAVimc::start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) { trace(IPAOperationStart); diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp index 231300ce..de96a606 100644 --- a/src/libcamera/ipa_context_wrapper.cpp +++ b/src/libcamera/ipa_context_wrapper.cpp @@ -86,14 +86,16 @@ int IPAContextWrapper::init(const IPASettings &settings) return 0; } -int IPAContextWrapper::start() +int IPAContextWrapper::start(const IPAOperationData &ipaConfig, + IPAOperationData *result) { if (intf_) - return intf_->start(); + return intf_->start(ipaConfig, result); if (!ctx_) return 0; + /* \todo Translate the ipaConfig and reponse */ return ctx_->ops->start(ctx_); } diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index 23fc56d7..282c3c0f 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -536,10 +536,17 @@ namespace libcamera { /** * \fn IPAInterface::start() * \brief Start the IPA + * \param[in] ipaConfig Pipeline-handler-specific configuration data + * \param[out] result Pipeline-handler-specific configuration result * * This method informs the IPA module that the camera is about to be started. * The IPA module shall prepare any resources it needs to operate. * + * The \a ipaConfig and \a result parameters carry custom data passed by the + * pipeline handler to the IPA and back. The pipeline handler may set the \a + * result parameter to null if the IPA protocol doesn't need to pass a result + * back through the configure() function. + * * \return 0 on success or a negative error code otherwise */ diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index ddb30e49..a8e4997a 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -747,7 +747,9 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont } /* Start the IPA. */ - ret = data->ipa_->start(); + IPAOperationData ipaConfig, result; + ipaConfig.controls.emplace_back(*controls); + ret = data->ipa_->start(ipaConfig, &result); if (ret) { LOG(RPI, Error) << "Failed to start IPA for " << camera->id(); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 2e8d2930..a6c82a48 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -832,7 +832,8 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c if (ret) return ret; - ret = data->ipa_->start(); + IPAOperationData ipaConfig = {}; + ret = data->ipa_->start(ipaConfig, nullptr); if (ret) { freeBuffers(camera); LOG(RkISP1, Error) @@ -911,7 +912,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c std::map entityControls; entityControls.emplace(0, data->sensor_->controls()); - IPAOperationData ipaConfig; + ipaConfig = {}; data->ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, nullptr); diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index d81b8598..b4773cf5 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -322,7 +322,8 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con if (ret < 0) return ret; - ret = data->ipa_->start(); + IPAOperationData ipaConfig = {}; + ret = data->ipa_->start(ipaConfig, nullptr); if (ret) { data->video_->releaseBuffers(); return ret; diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp index b78a0e45..619976e5 100644 --- a/src/libcamera/proxy/ipa_proxy_linux.cpp +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp @@ -30,7 +30,8 @@ public: { return 0; } - int start() override { return 0; } + int start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) override { return 0; } void stop() override {} void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo, [[maybe_unused]] const std::map &streamConfig, diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp index eead2883..9a81b6e7 100644 --- a/src/libcamera/proxy/ipa_proxy_thread.cpp +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp @@ -26,7 +26,8 @@ public: IPAProxyThread(IPAModule *ipam); int init(const IPASettings &settings) override; - int start() override; + int start(const IPAOperationData &ipaConfig, + IPAOperationData *result) override; void stop() override; void configure(const CameraSensorInfo &sensorInfo, @@ -50,9 +51,9 @@ private: ipa_ = ipa; } - int start() + int start(const IPAOperationData &ipaConfig, IPAOperationData *result) { - return ipa_->start(); + return ipa_->start(ipaConfig, result); } void stop() @@ -111,12 +112,14 @@ int IPAProxyThread::init(const IPASettings &settings) return 0; } -int IPAProxyThread::start() +int IPAProxyThread::start(const IPAOperationData &ipaConfig, + IPAOperationData *result) { running_ = true; thread_.start(); - return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking); + return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking, + ipaConfig, result); } void IPAProxyThread::stop() diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 67488409..6222938f 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -120,7 +120,8 @@ protected: } /* Test start of IPA module. */ - ipa_->start(); + IPAOperationData ipaConfig; + ipa_->start(ipaConfig, nullptr); timer.start(1000); while (timer.isRunning() && trace_ != IPAOperationStart) dispatcher->processEvents(); diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp index 59d991cb..7b8ae77b 100644 --- a/test/ipa/ipa_wrappers_test.cpp +++ b/test/ipa/ipa_wrappers_test.cpp @@ -56,7 +56,8 @@ public: return 0; } - int start() override + int start([[maybe_unused]] const IPAOperationData &ipaConfig, + [[maybe_unused]] IPAOperationData *result) override { report(Op_start, TestPass); return 0; @@ -376,7 +377,7 @@ protected: return TestFail; } - ret = INVOKE(start); + ret = INVOKE(start, ipaConfig, nullptr); if (ret == TestFail) { cerr << "Failed to run start()"; return TestFail;