From patchwork Mon Nov 21 14:21:05 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nick Hollinghurst X-Patchwork-Id: 17820 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 8595BBD16B for ; Mon, 21 Nov 2022 14:21:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E694B6331A; Mon, 21 Nov 2022 15:21:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1669040477; bh=dSjRfDL/m/ga6ggUf6MhvaECswpbR2mvuSl5IEaxsNM=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=UbRQh2UFvbGYOfK28j+BTTXgr00DNznGSnmthWYR8DMQlaRvr/nUrCe3Oiu5VovBg Da+89erEYaGE2pur1KncRl0Rm5ydd2wMi0c9IUcUzuTsCP/pjqvrjOABI0IGzpv8Jw GIvqPhKBkdsl6syDHb6OBM3e08Mxo9DuQmglf386GHwlhyw/ZSWUetgTK/5cGvj5if zjRy3lciOI4ho1eZYPmLTs3IDRRsQW6cI8H2TKQhUw8MtMXdKw90zlLLOPZumOFAR/ zKdcoC9AVu5M5BL6O8kOdzmWxA2mkOLZBRUZBhAi9ITd0y3itS3U06VtUkz5POHWXu DAQRYhWS2H46Q== Received: from mail-yw1-x1129.google.com (mail-yw1-x1129.google.com [IPv6:2607:f8b0:4864:20::1129]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F3C9163097 for ; Mon, 21 Nov 2022 15:21:16 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="n1X1x+eK"; dkim-atps=neutral Received: by mail-yw1-x1129.google.com with SMTP id 00721157ae682-39451671bdfso89369737b3.10 for ; Mon, 21 Nov 2022 06:21:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=r143vMyPPWjPPnbCkJ/oUaR7R9KaHMHST0LijbepNRY=; b=n1X1x+eKTWI1+m7/bvaQNyXAqrMpQzs6tdvFPMRM0W6euPlejj9YU3FtTa6U4GAWqn cuSJOeGBjg7kjYPmCxBQsgzA9ArruOr6+65rKeUGk8AN5sDzmcd+FYp2ZlVSjP9USz8Q 7meHvngzdeZZ8duI9UpAWqz5f7Pu2iJKAtRmTNnleqEBiLiJh3PJvx/WxTDB9oxAd11Q BKQK68yYmbmk/hSgC5ZstNRBY7DaI5hEYkiplRxo53KF1j5PP9z5YgTgW98cZMV7fpxm Un7Yo7ogsYliOC8YlQYVsDZkPqYkALava7knv1tqY3evR34csHkER1yDkYgFnQHWkIM8 nwCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=r143vMyPPWjPPnbCkJ/oUaR7R9KaHMHST0LijbepNRY=; b=A8szQ7TULUBDVPKv8Y7AX4WJte1KfWVqKA8Swc2S16LRE3kAIu0vK5EiAmSetxSFs4 70/aejPHLiHVH9nn/c0G710q/NHidGs2WdvSkmWs/IUTmHjBTts6GuZ2wRSif888ULZC WNzERZrcxkkU5b0Rhp2Hl4iCFb7aCRPjBOmuFlNEjMzk8/NkVSERWmoUAtPpGDOn6RJV ZMpEffX+rvv619T8YJemD7sjsWdvDG9jssFzjDjAf5Sn3gOoWaDBGjy2rW1WkFUPJh7u JwbcVCf7I4pvRHUvw19i/CBWqKt0ZoO7kuCmNaw2yedFT+j+J/Q4hgbCTSFP+O3aKk09 VXyQ== X-Gm-Message-State: ANoB5plfWvmLE+IHwHLQwRpiI7cOluxMPw0HChh0cGmaKz3tHivmd9vy JNoaHSq2romklbxbTp3YEyfWfC4tCYsxiabnwgMAbTLFLL2xzQ== X-Google-Smtp-Source: AA0mqf7xNZF/1czP+wa4H2SMARPcgHEiuZdYOWg1YmcxXfZ/AZLi7zjKxto44y4ZEgTwG5nhHR1K7BrVHmeW6F26+uA= X-Received: by 2002:a05:690c:c14:b0:345:1d35:8884 with SMTP id cl20-20020a05690c0c1400b003451d358884mr352481ywb.405.1669040475736; Mon, 21 Nov 2022 06:21:15 -0800 (PST) MIME-Version: 1.0 Date: Mon, 21 Nov 2022 14:21:05 +0000 Message-ID: To: libcamera-devel@lists.libcamera.org Subject: [libcamera-devel] [PATCH 1/1] ipa/raspberrypi: Remove generic "pause" mechanism from Algorithm 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: , X-Patchwork-Original-From: Nick Hollinghurst via libcamera-devel From: Nick Hollinghurst Reply-To: Nick Hollinghurst Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" No existing Algorithm used the base pause(), resume() functions or the paused_ flag, nor is there a need or a generic pause API. Remove these. The AGC and AWB algorithms now have methods named disableAuto(), enableAuto() which better describe their functionality. Signed-off-by: Nick Hollinghurst --- src/ipa/raspberrypi/controller/agc_algorithm.h | 2 ++ src/ipa/raspberrypi/controller/algorithm.h | 6 +----- src/ipa/raspberrypi/controller/awb_algorithm.h | 2 ++ src/ipa/raspberrypi/controller/controller.cpp | 6 ++---- src/ipa/raspberrypi/controller/rpi/agc.cpp | 13 ++++--------- src/ipa/raspberrypi/controller/rpi/agc.h | 6 ++---- src/ipa/raspberrypi/controller/rpi/awb.cpp | 11 +++-------- src/ipa/raspberrypi/controller/rpi/awb.h | 6 ++---- src/ipa/raspberrypi/raspberrypi.cpp | 14 ++++++++------ 9 files changed, 26 insertions(+), 40 deletions(-) << "Could not set AE_ENABLE - no AGC algorithm"; @@ -714,9 +715,9 @@ void IPARPi::queueRequest(const ControlList &controls) } if (ctrl.second.get() == false) - agc->pause(); + agc->disableAuto(); else - agc->resume(); + agc->enableAuto(); libcameraMetadata_.set(controls::AeEnable, ctrl.second.get()); break; @@ -835,7 +836,8 @@ void IPARPi::queueRequest(const ControlList &controls) } case controls::AWB_ENABLE: { - RPiController::Algorithm *awb = controller_.getAlgorithm("awb"); + RPiController::AwbAlgorithm *awb = dynamic_cast( + controller_.getAlgorithm("awb")); if (!awb) { LOG(IPARPI, Warning) << "Could not set AWB_ENABLE - no AWB algorithm"; @@ -843,9 +845,9 @@ void IPARPi::queueRequest(const ControlList &controls) } if (ctrl.second.get() == false) - awb->pause(); + awb->disableAuto(); else - awb->resume(); + awb->enableAuto(); libcameraMetadata_.set(controls::AwbEnable, ctrl.second.get()); diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.h b/src/ipa/raspberrypi/controller/agc_algorithm.h index 3a91444c..36e6c110 100644 --- a/src/ipa/raspberrypi/controller/agc_algorithm.h +++ b/src/ipa/raspberrypi/controller/agc_algorithm.h @@ -26,6 +26,8 @@ public: virtual void setMeteringMode(std::string const &meteringModeName) = 0; virtual void setExposureMode(std::string const &exposureModeName) = 0; virtual void setConstraintMode(std::string const &contraintModeName) = 0; + virtual void enableAuto() = 0; + virtual void disableAuto() = 0; }; } /* namespace RPiController */ diff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h index cbbb13ba..4f327598 100644 --- a/src/ipa/raspberrypi/controller/algorithm.h +++ b/src/ipa/raspberrypi/controller/algorithm.h @@ -27,14 +27,11 @@ class Algorithm { public: Algorithm(Controller *controller) - : controller_(controller), paused_(false) + : controller_(controller) { } virtual ~Algorithm() = default; virtual char const *name() const = 0; - virtual bool isPaused() const { return paused_; } - virtual void pause() { paused_ = true; } - virtual void resume() { paused_ = false; } virtual int read(const libcamera::YamlObject ¶ms); virtual void initialise(); virtual void switchMode(CameraMode const &cameraMode, Metadata *metadata); @@ -47,7 +44,6 @@ public: private: Controller *controller_; - bool paused_; }; /* diff --git a/src/ipa/raspberrypi/controller/awb_algorithm.h b/src/ipa/raspberrypi/controller/awb_algorithm.h index 48e08b60..8462c4db 100644 --- a/src/ipa/raspberrypi/controller/awb_algorithm.h +++ b/src/ipa/raspberrypi/controller/awb_algorithm.h @@ -18,6 +18,8 @@ public: virtual unsigned int getConvergenceFrames() const = 0; virtual void setMode(std::string const &modeName) = 0; virtual void setManualGains(double manualR, double manualB) = 0; + virtual void enableAuto() = 0; + virtual void disableAuto() = 0; }; } /* namespace RPiController */ diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp index f4892786..e9156785 100644 --- a/src/ipa/raspberrypi/controller/controller.cpp +++ b/src/ipa/raspberrypi/controller/controller.cpp @@ -108,16 +108,14 @@ void Controller::prepare(Metadata *imageMetadata) { assert(switchModeCalled_); for (auto &algo : algorithms_) - if (!algo->isPaused()) - algo->prepare(imageMetadata); + algo->prepare(imageMetadata); } void Controller::process(StatisticsPtr stats, Metadata *imageMetadata) { assert(switchModeCalled_); for (auto &algo : algorithms_) - if (!algo->isPaused()) - algo->process(stats, imageMetadata); + algo->process(stats, imageMetadata); } Metadata &Controller::getGlobalMetadata() diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp index bd54a639..a30e50c1 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp @@ -270,18 +270,13 @@ int Agc::read(const libcamera::YamlObject ¶ms) return 0; } -bool Agc::isPaused() const -{ - return false; -} - -void Agc::pause() +void Agc::disableAuto() { fixedShutter_ = status_.shutterTime; fixedAnalogueGain_ = status_.analogueGain; } -void Agc::resume() +void Agc::enableAuto() { fixedShutter_ = 0s; fixedAnalogueGain_ = 0; @@ -317,14 +312,14 @@ void Agc::setMaxShutter(Duration maxShutter) void Agc::setFixedShutter(Duration fixedShutter) { fixedShutter_ = fixedShutter; - /* Set this in case someone calls Pause() straight after. */ + /* Set this in case someone calls disableAuto() straight after. */ status_.shutterTime = clipShutter(fixedShutter_); } void Agc::setFixedAnalogueGain(double fixedAnalogueGain) { fixedAnalogueGain_ = fixedAnalogueGain; - /* Set this in case someone calls Pause() straight after. */ + /* Set this in case someone calls disableAuto() straight after. */ status_.analogueGain = fixedAnalogueGain; } diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h b/src/ipa/raspberrypi/controller/rpi/agc.h index 6d6b0e5a..cf04da19 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.h +++ b/src/ipa/raspberrypi/controller/rpi/agc.h @@ -75,10 +75,6 @@ public: Agc(Controller *controller); char const *name() const override; int read(const libcamera::YamlObject ¶ms) override; - /* AGC handles "pausing" for itself. */ - bool isPaused() const override; - void pause() override; - void resume() override; unsigned int getConvergenceFrames() const override; void setEv(double ev) override; void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override; @@ -88,6 +84,8 @@ public: void setMeteringMode(std::string const &meteringModeName) override; void setExposureMode(std::string const &exposureModeName) override; void setConstraintMode(std::string const &contraintModeName) override; + void enableAuto() override; + void disableAuto() override; void switchMode(CameraMode const &cameraMode, Metadata *metadata) override; void prepare(Metadata *imageMetadata) override; void process(StatisticsPtr &stats, Metadata *imageMetadata) override; diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp index 8d8ddf09..4f6af4ba 100644 --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp @@ -222,21 +222,16 @@ void Awb::initialise() asyncResults_ = syncResults_; } -bool Awb::isPaused() const +void Awb::disableAuto() { - return false; -} - -void Awb::pause() -{ - /* "Pause" by fixing everything to the most recent values. */ + /* Freeze the most recent values, and treat them as manual gains */ manualR_ = syncResults_.gainR = prevSyncResults_.gainR; manualB_ = syncResults_.gainB = prevSyncResults_.gainB; syncResults_.gainG = prevSyncResults_.gainG; syncResults_.temperatureK = prevSyncResults_.temperatureK; } -void Awb::resume() +void Awb::enableAuto() { manualR_ = 0.0; manualB_ = 0.0; diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h b/src/ipa/raspberrypi/controller/rpi/awb.h index 30acd89d..2254c3ed 100644 --- a/src/ipa/raspberrypi/controller/rpi/awb.h +++ b/src/ipa/raspberrypi/controller/rpi/awb.h @@ -93,13 +93,11 @@ public: char const *name() const override; void initialise() override; int read(const libcamera::YamlObject ¶ms) override; - /* AWB handles "pausing" for itself. */ - bool isPaused() const override; - void pause() override; - void resume() override; unsigned int getConvergenceFrames() const override; void setMode(std::string const &name) override; void setManualGains(double manualR, double manualB) override; + void enableAuto() override; + void disableAuto() override; void switchMode(CameraMode const &cameraMode, Metadata *metadata) override; void prepare(Metadata *imageMetadata) override; void process(StatisticsPtr &stats, Metadata *imageMetadata) override; diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index b74f1ecf..beb076dc 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -706,7 +706,8 @@ void IPARPi::queueRequest(const ControlList &controls) switch (ctrl.first) { case controls::AE_ENABLE: { - RPiController::Algorithm *agc = controller_.getAlgorithm("agc"); + RPiController::AgcAlgorithm *agc = dynamic_cast( + controller_.getAlgorithm("agc")); if (!agc) { LOG(IPARPI, Warning)