From patchwork Fri Dec 4 15:31:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 10562 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 137E2BE178 for ; Fri, 4 Dec 2020 15:31:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5F48B635FA; Fri, 4 Dec 2020 16:31:30 +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="PhrssKzj"; dkim-atps=neutral Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 457CC635D6 for ; Fri, 4 Dec 2020 16:31:29 +0100 (CET) Received: by mail-wr1-x42f.google.com with SMTP id g14so5690012wrm.13 for ; Fri, 04 Dec 2020 07:31:29 -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=JNvPjsKqf/Ia/ymHimIOfo8y4FhZtcQ109ptuF8ZPzE=; b=PhrssKzjAScSl1m7RmmQbst8C74ANFrlNJIrt5VlsSegMcENPc4Y8ZLZKSGNwAcqqQ 5sIhLDL2h72NerlJeyLcd7rXpw7vwOUFie2cVFX4VPOCT3+AyHSsnqje4KAhd/1JeJy3 rDtXzvnmJMcQ54e1xht49EVfw4u969XJR4AduKOJaRR1dTxMiOPgS3Hf7FTMYwfTodb4 HSIul2qtsex8hGiZtezWL+JosGuvW0idMbkjX1Y6xP+nX9oahmORJRRwXRuk+J7VSA6j gSOPru0GSApPYBuxkbblmRbgbtGAwsqR5UCpWKZFBuZm/duk4DeMTwzPPuiBsyjVgSIR rSxg== 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=JNvPjsKqf/Ia/ymHimIOfo8y4FhZtcQ109ptuF8ZPzE=; b=kpkhzxIyCQaWM08Lzl7uDuqBtfjYGeIc4nduJg6KoGxhHXsrzsJlSXmVzZMuln54g1 fdE3+UEt7ItmTt6bVzwBaovxVomYLohR/csM063B3UVcUbQAS1G87IMgg0CW9dhydoIr xa1mi7t2vj6E7nu1d51/1MwgDXc0XMBS3kSxVnwD3MKpb+/ih4dajDUE5vDtbgMaMVY+ FB/bUJjsCCXQZL3rDxLdFURg11z7s0x3TefbmR4j21JS2FP8Ot40j0Q6F6Car+JoGIYT 7atzEFucsWXtgwVVoZcpIzAv6URRx7rZDE+IdtReIw1huuKkfw4QtSTz5+db6vrM4HAc n2+A== X-Gm-Message-State: AOAM5308Q7ukoilyVtnRpRxlYS98cqktB6Zoxf1EUzv0PX7OorXImmVa 6+R853D30E1UySGIkngR9R2YjlRE5kT7fQ== X-Google-Smtp-Source: ABdhPJxSTpZm06d7NW8RrkLwj1zNROjNI56vySwQeHIqZC2TUUOwBXSSpDGSYwbpqNGMHsz/PpULOg== X-Received: by 2002:adf:f9c5:: with SMTP id w5mr5452508wrr.69.1607095887171; Fri, 04 Dec 2020 07:31:27 -0800 (PST) Received: from naushir-VirtualBox.pitowers.org ([2a00:1098:3142:14:a00:27ff:fe4d:f6a2]) by smtp.gmail.com with ESMTPSA id x66sm3465426wmg.26.2020.12.04.07.31.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Dec 2020 07:31:26 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 4 Dec 2020 15:31:20 +0000 Message-Id: <20201204153121.66136-3-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201204153121.66136-1-naush@raspberrypi.com> References: <20201204153121.66136-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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 Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- 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 | 5 +++-- src/ipa/rkisp1/rkisp1.cpp | 3 ++- src/ipa/vimc/vimc.cpp | 6 ++++-- src/libcamera/ipa_context_wrapper.cpp | 5 +++-- src/libcamera/ipa_interface.cpp | 7 +++++++ src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 +++++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- 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..ec27e48e 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 &data, + 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..df12f9ce 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 &data, + 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..40628489 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 data and result. */ + IPAOperationData data = {}; + return ctx->ipa_->start(data, nullptr); } void IPAInterfaceWrapper::stop(struct ipa_context *_ctx) diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 29d48b1b..b8c0f955 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -78,13 +78,14 @@ public: } int init(const IPASettings &settings) override; - int start() override { return 0; } + int start([[maybe_unused]] const IPAOperationData &data, + [[maybe_unused]] IPAOperationData *result) override { return 0; } void stop() override {} void configure(const CameraSensorInfo &sensorInfo, const std::map &streamConfig, const std::map &entityControls, - const IPAOperationData &data, + const IPAOperationData &ipaConfig, IPAOperationData *response) override; void mapBuffers(const std::vector &buffers) override; void unmapBuffers(const std::vector &ids) override; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 07d7f1b2..39783abd 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 &data, + [[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..074902ee 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 &data, + 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 &data, + [[maybe_unused]] IPAOperationData *result) { trace(IPAOperationStart); diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp index 231300ce..19c44ad8 100644 --- a/src/libcamera/ipa_context_wrapper.cpp +++ b/src/libcamera/ipa_context_wrapper.cpp @@ -86,10 +86,11 @@ int IPAContextWrapper::init(const IPASettings &settings) return 0; } -int IPAContextWrapper::start() +int IPAContextWrapper::start(const IPAOperationData &data, + IPAOperationData *result) { if (intf_) - return intf_->start(); + return intf_->start(data, result); if (!ctx_) return 0; diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index 23fc56d7..5be6f787 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] data Protocol-specific data for the start operation + * \param[out] result Result of the start operation * * 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 data 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 start() 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 9937db73..bafd0b2d 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -752,7 +752,10 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont } /* Start the IPA. */ - ret = data->ipa_->start(); + IPAOperationData ipaData = {}, result = {}; + if (controls) + ipaData.controls.emplace_back(*controls); + ret = data->ipa_->start(ipaData, &result); if (ret) { LOG(RPI, Error) << "Failed to start IPA for " << camera->id(); @@ -1189,7 +1192,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 4e959fde..eaa10f9f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -842,7 +842,8 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c if (ret) return ret; - ret = data->ipa_->start(); + IPAOperationData ipaData = {}; + ret = data->ipa_->start(ipaData, nullptr); if (ret) { freeBuffers(camera); LOG(RkISP1, Error) diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index d81b8598..2a5054a8 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 ipaData = {}; + ret = data->ipa_->start(ipaData, 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..ea6f3e5e 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 &data, + [[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..a5fda2c8 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 &data, + IPAOperationData *result) override; void stop() override; void configure(const CameraSensorInfo &sensorInfo, @@ -50,9 +51,9 @@ private: ipa_ = ipa; } - int start() + int start(const IPAOperationData &data, IPAOperationData *result) { - return ipa_->start(); + return ipa_->start(data, result); } void stop() @@ -111,12 +112,14 @@ int IPAProxyThread::init(const IPASettings &settings) return 0; } -int IPAProxyThread::start() +int IPAProxyThread::start(const IPAOperationData &data, + IPAOperationData *result) { running_ = true; thread_.start(); - return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking); + return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking, + data, result); } void IPAProxyThread::stop() diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 9f575f93..dbb6727f 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 data = {}; + ipa_->start(data, 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..47533d10 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 &data, + [[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;