Patch Detail
Show a patch.
GET /api/patches/17821/?format=api
{ "id": 17821, "url": "https://patchwork.libcamera.org/api/patches/17821/?format=api", "web_url": "https://patchwork.libcamera.org/patch/17821/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20221121144729.3264-1-nick.hollinghurst@raspberrypi.com>", "date": "2022-11-21T14:47:29", "name": "[libcamera-devel] ipa: raspberrypi: Remove generic \"pause\" mechanism from Algorithm", "commit_ref": "1bcb7539dfcc2dde9745a9637c0a4132de34a9d4", "pull_url": null, "state": "accepted", "archived": false, "hash": "85b07340d348d4abdb9088265c05c358c6640aa2", "submitter": { "id": 130, "url": "https://patchwork.libcamera.org/api/people/130/?format=api", "name": "Nick Hollinghurst", "email": "nick.hollinghurst@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/17821/mbox/", "series": [ { "id": 3624, "url": "https://patchwork.libcamera.org/api/series/3624/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3624", "date": "2022-11-21T14:47:29", "name": "[libcamera-devel] ipa: raspberrypi: Remove generic \"pause\" mechanism from Algorithm", "version": 1, "mbox": "https://patchwork.libcamera.org/series/3624/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/17821/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/17821/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B8764BE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Nov 2022 14:47:34 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 175826331A;\n\tMon, 21 Nov 2022 15:47:34 +0100 (CET)", "from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com\n\t[IPv6:2a00:1450:4864:20::42d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 23AE663097\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Nov 2022 15:47:33 +0100 (CET)", "by mail-wr1-x42d.google.com with SMTP id e11so7279542wru.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Nov 2022 06:47:33 -0800 (PST)", "from raspberrypi.pitowers.org\n\t([2a00:1098:3142:14:8629:311b:808f:cb27])\n\tby smtp.gmail.com with ESMTPSA id\n\tq3-20020a05600000c300b00241c8794d38sm7486667wrx.105.2022.11.21.06.47.32\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 21 Nov 2022 06:47:32 -0800 (PST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669042054;\n\tbh=kz7K0etMNJdLcO8Ttxgg7fYDXcD8nP//b09GboQpcC4=;\n\th=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post:\n\tList-Help:List-Subscribe:From:Reply-To:Cc:From;\n\tb=vlNTdVLa205B9UYpPegKIQGQUnB6Gf2xcoKL2ERlnbeggJa2efilAQM6+GITvO8AI\n\tQjznEwuezROQqKlc5NmrrDteHBoVIV8vvpTFqKGF/W/GA4VYtydZd2pZLvCDp8cvfu\n\taWaZKsO4zovT2RHSPQFRsGMXKBQ3TtS5DzxXuMQ6yY11gmyVkdes9gyIYETud8wu5P\n\t3Cc9+GlebG9oOftUuSRzKG4CbhjjVckrRK37XaRUjvQ80z+8YPI50IH5TJqgwwZYaJ\n\tX7c78Us/k04GtN73jO7LD28ShR5AsbDGc0SnmWdHGdHlxhrOHiByXUx0w/v8UF7XVG\n\tUeHVR+6OyuqnA==", "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=content-transfer-encoding:mime-version:message-id:date:subject:cc\n\t:to:from:from:to:cc:subject:date:message-id:reply-to;\n\tbh=jnsyIcfP6wyfB/9Dwk9raDbjy9maA5LEDTGXsVcs+I0=;\n\tb=Sl7nTsTDwYov7gc4/9RdTYwKs9TjnXSLWVszqEhUWKsk7PS+UBD1evRIech5up74Rn\n\t1J0qhWfs0dG4BAIlfQkRFL2fqPhSdMVAaBpJA65v0Ez67ah+NUGUsWX29PmSWnWXpDbK\n\tZcN+kVpqEJnOYLG1R2r1aBGX9ee8zTy93xrPe7tWWXsvIv8sP2lY+Q/3WwJY3CWQbNhu\n\tXNWPoUGx4gp4iUBRUfRapeY7i7JV4NxGAtuPqQD1oCfB6uyJ2Ks/hLBBbfMeGsTzPknC\n\tTsM5Mt0Qnxjaef95yjwC8+ORF8jzpa3s5dAxCeZxD2PBsFiGgPwTsDdh0WGYM3X0ZVuN\n\t/eEw==" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"Sl7nTsTD\"; dkim-atps=neutral", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=content-transfer-encoding:mime-version:message-id:date:subject:cc\n\t:to:from:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=jnsyIcfP6wyfB/9Dwk9raDbjy9maA5LEDTGXsVcs+I0=;\n\tb=r8BPc/NFpM7WBu9cakeWWdmUoQOr9Kj/ZKaBN7QEMXJvA4MmA9rddV9RBHcqe1YVt2\n\tdacyrdBpgpWHoY+V4NoUR1hFyIZTAaknef6hQ4nC9YzfK78iBrPCJ+z0STHe8AT/gEOi\n\tgxB1+L/rELEVlGF4cF4C4oh9IudrSlelfK7noIRQvjNZypLwedGcPGz3TcQ3eCAqrUSG\n\tQv6PmZufd4NF7JnZ/xNqZRr2iVxD0EeAjoUI3wtblCw+FgSGSRILuej/X8W3LO/yYchr\n\tiQtbCbWPS6lz9UmhdgQTno7jmM8m5niqRkz/v9+OM93v1aY3cZXaH5pQKQ8FD/nrnHPv\n\twzCw==", "X-Gm-Message-State": "ANoB5ploDhz0560t8P/SdSUmFcHHTUyzVOxy8GuVWvT7Qct8GP4DYs4T\n\tg54Q+Anr48sRpBLSMPwNN2AsCaT1ITuvzQ==", "X-Google-Smtp-Source": "AA0mqf5CqaGBQRvMnU6B3tZsKvSxzPFJxeKoXJhAz0n3yjpmZJ0CpuRLjjxlp71ll2Y2srM15eGjSw==", "X-Received": "by 2002:adf:d844:0:b0:241:be45:54eb with SMTP id\n\tk4-20020adfd844000000b00241be4554ebmr9109348wrl.49.1669042052680; \n\tMon, 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", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH] ipa: raspberrypi: Remove generic \"pause\"\n\tmechanism from Algorithm", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "From": "Nick Hollinghurst via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>", "Cc": "Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "No existing Algorithm used the base pause(), resume() functions\nor the paused_ flag, nor is there a need or a generic pause API.\nRemove these. The AGC and AWB algorithms now have methods named\ndisableAuto(), enableAuto() which better describe their functionality.\n\nSigned-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>\n---\n src/ipa/raspberrypi/controller/agc_algorithm.h | 2 ++\n src/ipa/raspberrypi/controller/algorithm.h | 6 +-----\n src/ipa/raspberrypi/controller/awb_algorithm.h | 2 ++\n src/ipa/raspberrypi/controller/controller.cpp | 6 ++----\n src/ipa/raspberrypi/controller/rpi/agc.cpp | 13 ++++---------\n src/ipa/raspberrypi/controller/rpi/agc.h | 6 ++----\n src/ipa/raspberrypi/controller/rpi/awb.cpp | 11 +++--------\n src/ipa/raspberrypi/controller/rpi/awb.h | 6 ++----\n src/ipa/raspberrypi/raspberrypi.cpp | 14 ++++++++------\n 9 files changed, 26 insertions(+), 40 deletions(-)", "diff": "diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.h b/src/ipa/raspberrypi/controller/agc_algorithm.h\nindex 3a91444c..36e6c110 100644\n--- a/src/ipa/raspberrypi/controller/agc_algorithm.h\n+++ b/src/ipa/raspberrypi/controller/agc_algorithm.h\n@@ -26,6 +26,8 @@ public:\n \tvirtual void setMeteringMode(std::string const &meteringModeName) = 0;\n \tvirtual void setExposureMode(std::string const &exposureModeName) = 0;\n \tvirtual void setConstraintMode(std::string const &contraintModeName) = 0;\n+\tvirtual void enableAuto() = 0;\n+\tvirtual void disableAuto() = 0;\n };\n \n } /* namespace RPiController */\ndiff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h\nindex cbbb13ba..4f327598 100644\n--- a/src/ipa/raspberrypi/controller/algorithm.h\n+++ b/src/ipa/raspberrypi/controller/algorithm.h\n@@ -27,14 +27,11 @@ class Algorithm\n {\n public:\n \tAlgorithm(Controller *controller)\n-\t\t: controller_(controller), paused_(false)\n+\t\t: controller_(controller)\n \t{\n \t}\n \tvirtual ~Algorithm() = default;\n \tvirtual char const *name() const = 0;\n-\tvirtual bool isPaused() const { return paused_; }\n-\tvirtual void pause() { paused_ = true; }\n-\tvirtual void resume() { paused_ = false; }\n \tvirtual int read(const libcamera::YamlObject ¶ms);\n \tvirtual void initialise();\n \tvirtual void switchMode(CameraMode const &cameraMode, Metadata *metadata);\n@@ -47,7 +44,6 @@ public:\n \n private:\n \tController *controller_;\n-\tbool paused_;\n };\n \n /*\ndiff --git a/src/ipa/raspberrypi/controller/awb_algorithm.h b/src/ipa/raspberrypi/controller/awb_algorithm.h\nindex 48e08b60..8462c4db 100644\n--- a/src/ipa/raspberrypi/controller/awb_algorithm.h\n+++ b/src/ipa/raspberrypi/controller/awb_algorithm.h\n@@ -18,6 +18,8 @@ public:\n \tvirtual unsigned int getConvergenceFrames() const = 0;\n \tvirtual void setMode(std::string const &modeName) = 0;\n \tvirtual void setManualGains(double manualR, double manualB) = 0;\n+\tvirtual void enableAuto() = 0;\n+\tvirtual void disableAuto() = 0;\n };\n \n } /* namespace RPiController */\ndiff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp\nindex f4892786..e9156785 100644\n--- a/src/ipa/raspberrypi/controller/controller.cpp\n+++ b/src/ipa/raspberrypi/controller/controller.cpp\n@@ -108,16 +108,14 @@ void Controller::prepare(Metadata *imageMetadata)\n {\n \tassert(switchModeCalled_);\n \tfor (auto &algo : algorithms_)\n-\t\tif (!algo->isPaused())\n-\t\t\talgo->prepare(imageMetadata);\n+\t\talgo->prepare(imageMetadata);\n }\n \n void Controller::process(StatisticsPtr stats, Metadata *imageMetadata)\n {\n \tassert(switchModeCalled_);\n \tfor (auto &algo : algorithms_)\n-\t\tif (!algo->isPaused())\n-\t\t\talgo->process(stats, imageMetadata);\n+\t\talgo->process(stats, imageMetadata);\n }\n \n Metadata &Controller::getGlobalMetadata()\ndiff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\nindex bd54a639..a30e50c1 100644\n--- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n+++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n@@ -270,18 +270,13 @@ int Agc::read(const libcamera::YamlObject ¶ms)\n \treturn 0;\n }\n \n-bool Agc::isPaused() const\n-{\n-\treturn false;\n-}\n-\n-void Agc::pause()\n+void Agc::disableAuto()\n {\n \tfixedShutter_ = status_.shutterTime;\n \tfixedAnalogueGain_ = status_.analogueGain;\n }\n \n-void Agc::resume()\n+void Agc::enableAuto()\n {\n \tfixedShutter_ = 0s;\n \tfixedAnalogueGain_ = 0;\n@@ -317,14 +312,14 @@ void Agc::setMaxShutter(Duration maxShutter)\n void Agc::setFixedShutter(Duration fixedShutter)\n {\n \tfixedShutter_ = fixedShutter;\n-\t/* Set this in case someone calls Pause() straight after. */\n+\t/* Set this in case someone calls disableAuto() straight after. */\n \tstatus_.shutterTime = clipShutter(fixedShutter_);\n }\n \n void Agc::setFixedAnalogueGain(double fixedAnalogueGain)\n {\n \tfixedAnalogueGain_ = fixedAnalogueGain;\n-\t/* Set this in case someone calls Pause() straight after. */\n+\t/* Set this in case someone calls disableAuto() straight after. */\n \tstatus_.analogueGain = fixedAnalogueGain;\n }\n \ndiff --git a/src/ipa/raspberrypi/controller/rpi/agc.h b/src/ipa/raspberrypi/controller/rpi/agc.h\nindex 6d6b0e5a..cf04da19 100644\n--- a/src/ipa/raspberrypi/controller/rpi/agc.h\n+++ b/src/ipa/raspberrypi/controller/rpi/agc.h\n@@ -75,10 +75,6 @@ public:\n \tAgc(Controller *controller);\n \tchar const *name() const override;\n \tint read(const libcamera::YamlObject ¶ms) override;\n-\t/* AGC handles \"pausing\" for itself. */\n-\tbool isPaused() const override;\n-\tvoid pause() override;\n-\tvoid resume() override;\n \tunsigned int getConvergenceFrames() const override;\n \tvoid setEv(double ev) override;\n \tvoid setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override;\n@@ -88,6 +84,8 @@ public:\n \tvoid setMeteringMode(std::string const &meteringModeName) override;\n \tvoid setExposureMode(std::string const &exposureModeName) override;\n \tvoid setConstraintMode(std::string const &contraintModeName) override;\n+\tvoid enableAuto() override;\n+\tvoid disableAuto() override;\n \tvoid switchMode(CameraMode const &cameraMode, Metadata *metadata) override;\n \tvoid prepare(Metadata *imageMetadata) override;\n \tvoid process(StatisticsPtr &stats, Metadata *imageMetadata) override;\ndiff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp\nindex 8d8ddf09..4f6af4ba 100644\n--- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n+++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n@@ -222,21 +222,16 @@ void Awb::initialise()\n \tasyncResults_ = syncResults_;\n }\n \n-bool Awb::isPaused() const\n+void Awb::disableAuto()\n {\n-\treturn false;\n-}\n-\n-void Awb::pause()\n-{\n-\t/* \"Pause\" by fixing everything to the most recent values. */\n+\t/* Freeze the most recent values, and treat them as manual gains */\n \tmanualR_ = syncResults_.gainR = prevSyncResults_.gainR;\n \tmanualB_ = syncResults_.gainB = prevSyncResults_.gainB;\n \tsyncResults_.gainG = prevSyncResults_.gainG;\n \tsyncResults_.temperatureK = prevSyncResults_.temperatureK;\n }\n \n-void Awb::resume()\n+void Awb::enableAuto()\n {\n \tmanualR_ = 0.0;\n \tmanualB_ = 0.0;\ndiff --git a/src/ipa/raspberrypi/controller/rpi/awb.h b/src/ipa/raspberrypi/controller/rpi/awb.h\nindex 30acd89d..2254c3ed 100644\n--- a/src/ipa/raspberrypi/controller/rpi/awb.h\n+++ b/src/ipa/raspberrypi/controller/rpi/awb.h\n@@ -93,13 +93,11 @@ public:\n \tchar const *name() const override;\n \tvoid initialise() override;\n \tint read(const libcamera::YamlObject ¶ms) override;\n-\t/* AWB handles \"pausing\" for itself. */\n-\tbool isPaused() const override;\n-\tvoid pause() override;\n-\tvoid resume() override;\n \tunsigned int getConvergenceFrames() const override;\n \tvoid setMode(std::string const &name) override;\n \tvoid setManualGains(double manualR, double manualB) override;\n+\tvoid enableAuto() override;\n+\tvoid disableAuto() override;\n \tvoid switchMode(CameraMode const &cameraMode, Metadata *metadata) override;\n \tvoid prepare(Metadata *imageMetadata) override;\n \tvoid process(StatisticsPtr &stats, Metadata *imageMetadata) override;\ndiff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex b74f1ecf..beb076dc 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -706,7 +706,8 @@ void IPARPi::queueRequest(const ControlList &controls)\n \n \t\tswitch (ctrl.first) {\n \t\tcase controls::AE_ENABLE: {\n-\t\t\tRPiController::Algorithm *agc = controller_.getAlgorithm(\"agc\");\n+\t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n+\t\t\t\tcontroller_.getAlgorithm(\"agc\"));\n \t\t\tif (!agc) {\n \t\t\t\tLOG(IPARPI, Warning)\n \t\t\t\t\t<< \"Could not set AE_ENABLE - no AGC algorithm\";\n@@ -714,9 +715,9 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\t\t}\n \n \t\t\tif (ctrl.second.get<bool>() == false)\n-\t\t\t\tagc->pause();\n+\t\t\t\tagc->disableAuto();\n \t\t\telse\n-\t\t\t\tagc->resume();\n+\t\t\t\tagc->enableAuto();\n \n \t\t\tlibcameraMetadata_.set(controls::AeEnable, ctrl.second.get<bool>());\n \t\t\tbreak;\n@@ -835,7 +836,8 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\t}\n \n \t\tcase controls::AWB_ENABLE: {\n-\t\t\tRPiController::Algorithm *awb = controller_.getAlgorithm(\"awb\");\n+\t\t\tRPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(\n+\t\t\t\tcontroller_.getAlgorithm(\"awb\"));\n \t\t\tif (!awb) {\n \t\t\t\tLOG(IPARPI, Warning)\n \t\t\t\t\t<< \"Could not set AWB_ENABLE - no AWB algorithm\";\n@@ -843,9 +845,9 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\t\t}\n \n \t\t\tif (ctrl.second.get<bool>() == false)\n-\t\t\t\tawb->pause();\n+\t\t\t\tawb->disableAuto();\n \t\t\telse\n-\t\t\t\tawb->resume();\n+\t\t\t\tawb->enableAuto();\n \n \t\t\tlibcameraMetadata_.set(controls::AwbEnable,\n \t\t\t\t\t ctrl.second.get<bool>());\n", "prefixes": [ "libcamera-devel" ] }