From patchwork Mon Nov 21 14:47:29 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nick Hollinghurst X-Patchwork-Id: 17821 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 B8764BE08B for ; Mon, 21 Nov 2022 14:47:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 175826331A; Mon, 21 Nov 2022 15:47:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1669042054; bh=kz7K0etMNJdLcO8Ttxgg7fYDXcD8nP//b09GboQpcC4=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=vlNTdVLa205B9UYpPegKIQGQUnB6Gf2xcoKL2ERlnbeggJa2efilAQM6+GITvO8AI QjznEwuezROQqKlc5NmrrDteHBoVIV8vvpTFqKGF/W/GA4VYtydZd2pZLvCDp8cvfu aWaZKsO4zovT2RHSPQFRsGMXKBQ3TtS5DzxXuMQ6yY11gmyVkdes9gyIYETud8wu5P 3Cc9+GlebG9oOftUuSRzKG4CbhjjVckrRK37XaRUjvQ80z+8YPI50IH5TJqgwwZYaJ X7c78Us/k04GtN73jO7LD28ShR5AsbDGc0SnmWdHGdHlxhrOHiByXUx0w/v8UF7XVG UeHVR+6OyuqnA== Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 23AE663097 for ; Mon, 21 Nov 2022 15:47:33 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="Sl7nTsTD"; dkim-atps=neutral Received: by mail-wr1-x42d.google.com with SMTP id e11so7279542wru.8 for ; Mon, 21 Nov 2022 06:47:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=jnsyIcfP6wyfB/9Dwk9raDbjy9maA5LEDTGXsVcs+I0=; b=Sl7nTsTDwYov7gc4/9RdTYwKs9TjnXSLWVszqEhUWKsk7PS+UBD1evRIech5up74Rn 1J0qhWfs0dG4BAIlfQkRFL2fqPhSdMVAaBpJA65v0Ez67ah+NUGUsWX29PmSWnWXpDbK ZcN+kVpqEJnOYLG1R2r1aBGX9ee8zTy93xrPe7tWWXsvIv8sP2lY+Q/3WwJY3CWQbNhu XNWPoUGx4gp4iUBRUfRapeY7i7JV4NxGAtuPqQD1oCfB6uyJ2Ks/hLBBbfMeGsTzPknC TsM5Mt0Qnxjaef95yjwC8+ORF8jzpa3s5dAxCeZxD2PBsFiGgPwTsDdh0WGYM3X0ZVuN /eEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=jnsyIcfP6wyfB/9Dwk9raDbjy9maA5LEDTGXsVcs+I0=; b=r8BPc/NFpM7WBu9cakeWWdmUoQOr9Kj/ZKaBN7QEMXJvA4MmA9rddV9RBHcqe1YVt2 dacyrdBpgpWHoY+V4NoUR1hFyIZTAaknef6hQ4nC9YzfK78iBrPCJ+z0STHe8AT/gEOi gxB1+L/rELEVlGF4cF4C4oh9IudrSlelfK7noIRQvjNZypLwedGcPGz3TcQ3eCAqrUSG Qv6PmZufd4NF7JnZ/xNqZRr2iVxD0EeAjoUI3wtblCw+FgSGSRILuej/X8W3LO/yYchr iQtbCbWPS6lz9UmhdgQTno7jmM8m5niqRkz/v9+OM93v1aY3cZXaH5pQKQ8FD/nrnHPv wzCw== X-Gm-Message-State: ANoB5ploDhz0560t8P/SdSUmFcHHTUyzVOxy8GuVWvT7Qct8GP4DYs4T g54Q+Anr48sRpBLSMPwNN2AsCaT1ITuvzQ== X-Google-Smtp-Source: AA0mqf5CqaGBQRvMnU6B3tZsKvSxzPFJxeKoXJhAz0n3yjpmZJ0CpuRLjjxlp71ll2Y2srM15eGjSw== X-Received: by 2002:adf:d844:0:b0:241:be45:54eb with SMTP id k4-20020adfd844000000b00241be4554ebmr9109348wrl.49.1669042052680; Mon, 21 Nov 2022 06:47:32 -0800 (PST) Received: from raspberrypi.pitowers.org ([2a00:1098:3142:14:8629:311b:808f:cb27]) by smtp.gmail.com with ESMTPSA id q3-20020a05600000c300b00241c8794d38sm7486667wrx.105.2022.11.21.06.47.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Nov 2022 06:47:32 -0800 (PST) To: libcamera-devel@lists.libcamera.org Date: Mon, 21 Nov 2022 14:47:29 +0000 Message-Id: <20221121144729.3264-1-nick.hollinghurst@raspberrypi.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH] 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 Cc: 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 Reviewed-by: David Plowman Reviewed-by: Naushir Patuck --- 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(-) 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) << "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());