From patchwork Thu Nov 26 09:51:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 10499 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 9BF61BE08A for ; Thu, 26 Nov 2020 09:51:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4E45A63446; Thu, 26 Nov 2020 10:51:33 +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="Z6u+p9+I"; dkim-atps=neutral Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B0EA560331 for ; Thu, 26 Nov 2020 10:51:31 +0100 (CET) Received: by mail-wr1-x435.google.com with SMTP id p8so1434196wrx.5 for ; Thu, 26 Nov 2020 01:51:31 -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=dbthfx5bvmZxckK65DBFn+CwnKpKLWJDelW51rLTDIE=; b=Z6u+p9+I+nSVXdb3u6SfD7Of0xpDRVbV56891zBbb0N5BBa2G7CedbsEiEgN/REzOY apX4pMmvE6XxRw/l9gHCuNxhpMpffKxgYsXWVWwUq6oUJKqxSCr0Qk9yiCb9J6cD5/WW RzrO1jCDsgIzHyAuAOBZ41RIq1qpPkVYuudqpjbLeX+77M2KMbkwIhg0Ejz82oTdc43u 2mKzZZSAnyOwJ0QdKZdedI2qhZzV5oagmUs260UwnRFAZCV7N1g0bvHKE+PYgwRwoP5H tkuYNEbsyslD+8mAylmq/2fNE4W8frwHEDvfAe3J/Prf5zhXeGw0NcFsph2xQiKrxhSa JszQ== 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=dbthfx5bvmZxckK65DBFn+CwnKpKLWJDelW51rLTDIE=; b=ka9JpblJSM1juJoxH/d8RbrKr4srALqtCGHQrNxllueW8asqhi2s/jZU/6u16cg4bQ WQoS0qSDqk9qdLd/p398/Lth/KJkw8tnEnvBaGkKn/3YYlpRYRHReDkA8mEbcMRX5Srn po2n8Rw4FWMyWvO7TD8aS4qm71tQbvhAOaklAfFDnVJHND7QGOIz4td8mFNWqn8CD0g4 wHRdEoxcJ5TBKTlkgZBJ9TjMVZXNAiBDGQ9o8o1fvsxhFVdO0/16HxQ8arHTnmTo3R8J GdEDQt+nm+ByYRQ307vbMkW9a+Ryxg47m42qKmm38SOceIM38yt0VMqO/cfPyPL/lfpf iYiQ== X-Gm-Message-State: AOAM53055HuZDIZTFHcFqMeC3OGnadEMNOyXCjlYirH5P6Zbeepndki9 FdGDSyjb9C0IxwA3dRo61eu/LErGgu2brg== X-Google-Smtp-Source: ABdhPJy4CAZ6GvbFVcTqGpOBH5zGI7X42O+Dbo3cEIka/CXE7H5RYkIiufVkM+B1EegKYakMoVTM5A== X-Received: by 2002:adf:e502:: with SMTP id j2mr2765398wrm.73.1606384291012; Thu, 26 Nov 2020 01:51:31 -0800 (PST) Received: from naushir-VirtualBox.pitowers.org ([2a00:1098:3142:14:a00:27ff:fe4d:f6a2]) by smtp.gmail.com with ESMTPSA id i11sm8176827wrm.1.2020.11.26.01.51.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Nov 2020 01:51:30 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Thu, 26 Nov 2020 09:51:25 +0000 Message-Id: <20201126095126.997055-2-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201126095126.997055-1-naush@raspberrypi.com> References: <20201126095126.997055-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 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 PipelineHandler::start to be forwarded onto IPAInterface::start(). We also add a return channel if the pipeline handler 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 | 6 ++++-- 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, 50 insertions(+), 23 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..2b305b56 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..8c4ec40a 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 response */ 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..29bcef07 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(); @@ -1176,7 +1178,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) } /* Ready the IPA - it must know about the sensor resolution. */ - IPAOperationData result; + IPAOperationData result = {}; ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig, &result); 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..03b7f0ad 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;