From patchwork Fri Oct 2 09:33:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 9901 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 A3868C3B5B for ; Fri, 2 Oct 2020 09:33:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6929D63B2C; Fri, 2 Oct 2020 11:33:18 +0200 (CEST) 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="OXfanorP"; dkim-atps=neutral Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 86539623B6 for ; Fri, 2 Oct 2020 11:33:16 +0200 (CEST) Received: by mail-wr1-x434.google.com with SMTP id s12so1008127wrw.11 for ; Fri, 02 Oct 2020 02:33:16 -0700 (PDT) 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=F2qOlvN/lG1JG+AotBTh+2Tcwda21PM/APIMEEI8NjI=; b=OXfanorP8vu3vq1C8I7Pk8+154RZw2ipoK4nleGJHVQu3cFmIH/FHbc6S+hza81pyR 0K8SYzOVMy/+1waKuWKBcmodvVxR8VaCdkUCxB6sHuilvnkHsK7nxt+BegJwSOTlr6W9 8oDr5HWAKWw1aG36M7mm/atK8TIxeTGY+lOsF6yOgEv5QM1QWISGL9Tlf2VJR0WQTpxS oXskjaB8yjlFrqtc/xojjDyQhDZW+f4QsonGfBFKyDE7LoXAsBZrsAazFM6Hwp2kqFAw V1119BWvdsSYmu4xUyIbv+mfJFMPIYT0mQgII4vwrQF6HUico7Flw8pyFBPO3iS19zLQ XP8Q== 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=F2qOlvN/lG1JG+AotBTh+2Tcwda21PM/APIMEEI8NjI=; b=kYNlCGFAM+Pqbzyb8fuSTzBO/cjaT1BSmssPpWC+IyYB+rV4kec7naASlJV0eR86eA /a1R6LWZ8x4Q3GOd1msuDD+pdmUC9VGcZAMNbyG24KJUibLJk6e0sfbNXZjnWquD0ur+ OlCi2jPrfQtLrV5jgKa4esO54TpW/xzg1c8dTrnwTmY7gBBsFoIkVIHwg12nbOljCGZF mL3SUTy2F+2nnrPcbafOgpHdLquDt2FSfrJUWmo9EFWfu+IPmc+7Bs5GrKLrEFmQLuCU qC8k3aPN/aw8YZeUqgC/2o74Qlx1lFWWzwuTEJvDb7GtfWt5suYDiu1Vxv1BocK/9vYk NFRw== X-Gm-Message-State: AOAM530OrWmCSAlRsK7uVEjCuKqr/yOQxS3mNC3X7HnjxaiKnrWbKAbA l3/3wKaKofUsqXPFo8jrjEAis4S16sjO+w== X-Google-Smtp-Source: ABdhPJxWoz3qoEr/mPjZmsWsRpPF3H+9NwBH9RjPp3qcSbHyjeACoBPlFy64jzbpiYRxff7xch4c9w== X-Received: by 2002:adf:f7d0:: with SMTP id a16mr1869470wrq.381.1601631195667; Fri, 02 Oct 2020 02:33:15 -0700 (PDT) Received: from naushir-VirtualBox.pitowers.org ([2a00:1098:3142:14:a00:27ff:fe4d:f6a2]) by smtp.gmail.com with ESMTPSA id t203sm1101145wmg.43.2020.10.02.02.33.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 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 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 --- 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 5016ec25..ee7b2e87 100644 --- a/include/libcamera/ipa/ipa_interface.h +++ b/include/libcamera/ipa/ipa_interface.h @@ -153,7 +153,8 @@ public: virtual ~IPAInterface() {} 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 b0c7d1c1..9610229a 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 ef257762..07951c17 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 307ed9a2..2e0b0dcf 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -733,7 +733,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 15af7e5c..872bfc07 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 3b868e4b..8eb1998d 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 1bc93a63..a7bfcce8 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;