{"id":17630,"url":"https://patchwork.libcamera.org/api/1.1/patches/17630/?format=json","web_url":"https://patchwork.libcamera.org/patch/17630/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20221019090107.19975-6-naush@raspberrypi.com>","date":"2022-10-19T09:01:05","name":"[libcamera-devel,v4,5/7] ipa: raspberrypi: Use an array of RPiController::Metadata objects","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"7a5dd201742c8cf60564c674bb8201285ef1f9d8","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/1.1/people/34/?format=json","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/17630/mbox/","series":[{"id":3566,"url":"https://patchwork.libcamera.org/api/1.1/series/3566/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3566","date":"2022-10-19T09:01:00","name":"Raspberry Pi AGC digital gain fixes","version":4,"mbox":"https://patchwork.libcamera.org/series/3566/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/17630/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/17630/checks/","tags":{},"headers":{"Return-Path":"","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 39AF9C327C\n\tfor ;\n\tWed, 19 Oct 2022 09:01:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6F6462E55;\n\tWed, 19 Oct 2022 11:01:25 +0200 (CEST)","from mail-wr1-x432.google.com (mail-wr1-x432.google.com\n\t[IPv6:2a00:1450:4864:20::432])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A54762E4C\n\tfor ;\n\tWed, 19 Oct 2022 11:01:22 +0200 (CEST)","by mail-wr1-x432.google.com with SMTP id b4so27944138wrs.1\n\tfor ;\n\tWed, 19 Oct 2022 02:01:22 -0700 (PDT)","from naush-laptop.localdomain ([93.93.133.154])\n\tby smtp.gmail.com with ESMTPSA id\n\tr9-20020a05600c458900b003b47b913901sm1017595wmo.1.2022.10.19.02.01.20\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 19 Oct 2022 02:01:20 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666170085;\n\tbh=j5PQ+nf/o1EwX49vwnSLKLQnrh26xJfINWtBYZg6WO4=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=IdprpAqH59Q8mT+q9WBO9AQ6hJXmBQgGz5/m5zm34UoxjwZh/l8z7WWkYwCUFVmE9\n\tkJF7mmjD8mgnY64B2muv5RfvemfjDxIezRAYh+BCf6cYvXiEkHSCOKm5iReMuulf7h\n\tsYlwpPP7oDb2/7C0CUgswUfiSVhDf8tRdzcwYZgf4bDzlYaHFw/J60GbbY2+hNK9wJ\n\t9KwwqnJGV492OTDSY+eLlNYZWJxj/0o0cj501J5+R86zgf2hlYsfGO3wzVBkV+vREu\n\tk4pXRDf5zz7EQtZfJf4LJGGQabjXk9Rit24t90jMazXPn4wOQPIih1rH7hU/cidN+x\n\tDOfqLZVMWlqSw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=C08pD0BBWsfh+mVfb1uG3tXySSr8+tqVZu1o8cdDMyA=;\n\tb=OjFV/vKRlGl+jLlYa2z5S/nRsvjFR0f9Bh3v8SFMeWEe52knR/UMNPg+yms5dfzPlQ\n\txq4N1vTBlKhT6zcmaNUOhqFu9/q8P0VjDQgvASXSAYJWma5REcOExfgv5uAotySLTAkS\n\tIm0Sbzi4cCycRyjVBl/i1yKfzLKK1HYnCOFY/mRTZTmRVvTTkbi9QXNo5cupMT71EQpQ\n\tweayX8M//yVu5CWN189B0/VDbJAjfaREyvsyPu7fv7drh/x+mtKx4VUYGF3U6qfpUT1q\n\tKKgwErmohFmLmL2aYSms1PWecpTj/+nNAqxivi0nDDpX2Ilp0XfXhckISg6qO6rMKqxs\n\tDyZg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"OjFV/vKR\"; 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:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=C08pD0BBWsfh+mVfb1uG3tXySSr8+tqVZu1o8cdDMyA=;\n\tb=P5RxQKWGsgiN05pJ3lLwebJYThviTFWYQxurp2UGl98V+lA8yqnZIPC26NYTp4szwS\n\tFba1RUWls6K0/q3PslXZV/SBT8txGLkWpRMoM39O70MHb+Vuy364RjXBXAD+nUYPW10K\n\tWAKKIr6ybeHlw/Qq4+REjMCtLbCpuODwp6M3uc1jQnxZKsy+KFra1Bw7VwgOvaRZY1gj\n\tjFjtjGEhSco6Wag83K1faRcASa8W6KNGtjrAHHZSZNh+bmZMqXVwhe7a2gY1lzo7rZMB\n\tJq5xPcYs+XPOyT3EclC3zNza7DZaF/yn6oVCIDjaKtB26z/3ql9A6y4MuhxJNHy3b5Dg\n\tu7Iw==","X-Gm-Message-State":"ACrzQf0Q3AGmk+ryTre7Qoj2Um5b5fYqezAOM7rAIg1ozI7KCf0QqmF6\n\tzOeIxtOtXEISSn3UaqBMjZ7SrnySdlxtkg==","X-Google-Smtp-Source":"AMsMyM6iQdwk9Pq5pmOjDs96Qotima4SJeYHbiHY4UaggRsZlxKsglX9wJT4bUEWZJFKNlI06UKEdQ==","X-Received":"by 2002:adf:ce0e:0:b0:22e:525:c090 with SMTP id\n\tp14-20020adfce0e000000b0022e0525c090mr4395820wrn.238.1666170081376; \n\tWed, 19 Oct 2022 02:01:21 -0700 (PDT)","To":"libcamera-devel@lists.libcamera.org","Date":"Wed, 19 Oct 2022 10:01:05 +0100","Message-Id":"<20221019090107.19975-6-naush@raspberrypi.com>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20221019090107.19975-1-naush@raspberrypi.com>","References":"<20221019090107.19975-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v4 5/7] ipa: raspberrypi: Use an array of\n\tRPiController::Metadata objects","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Allow the IPA to cycle through an array of RPiController::Metadata objects, one\nper prepare()/process() invocation, when running the controller algorithms. This\nallows historical metadata objects to be retained, and subsequently passed into\nthe controller algorithms on future frames. At present, only a single index into\nthis array is used.\n\nThis change provides a route to fixing a problem with the AGC algorithm, where\nif a manual shutter/gain is requested, the algorithm does not currently retain\nany context of the total exposure that it has calculated. As a result, the\nwrong digital gain would be applied when the frame with the manual shutter/gain\nis processed by the ISP.\n\nSigned-off-by: Naushir Patuck \nReviewed-by: David Plowman \nTested-by: David Plowman \n---\n src/ipa/raspberrypi/raspberrypi.cpp | 65 ++++++++++++++++-------------\n 1 file changed, 37 insertions(+), 28 deletions(-)","diff":"diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex b74f1ecf738f..9e7792f5dfbe 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -57,6 +57,9 @@ namespace libcamera {\n using namespace std::literals::chrono_literals;\n using utils::Duration;\n \n+/* Number of metadata objects available in the context list. */\n+constexpr unsigned int numMetadataContexts = 16;\n+\n /* Configure the sensor with these values initially. */\n constexpr double defaultAnalogueGain = 1.0;\n constexpr Duration defaultExposureTime = 20.0ms;\n@@ -163,7 +166,8 @@ private:\n \t/* Raspberry Pi controller specific defines. */\n \tstd::unique_ptr helper_;\n \tRPiController::Controller controller_;\n-\tRPiController::Metadata rpiMetadata_;\n+\tstd::array rpiMetadata_;\n+\tunsigned int metadataIdx_;\n \n \t/*\n \t * We count frames to decide if the frame must be hidden (e.g. from\n@@ -320,6 +324,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig)\n \n \tfirstStart_ = false;\n \tlastRunTimestamp_ = 0;\n+\tmetadataIdx_ = 0;\n }\n \n void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo)\n@@ -539,14 +544,15 @@ void IPARPi::signalIspPrepare(const ISPConfig &data)\n \n void IPARPi::reportMetadata()\n {\n-\tstd::unique_lock lock(rpiMetadata_);\n+\tRPiController::Metadata &rpiMetadata = rpiMetadata_[metadataIdx_];\n+\tstd::unique_lock lock(rpiMetadata);\n \n \t/*\n \t * Certain information about the current frame and how it will be\n \t * processed can be extracted and placed into the libcamera metadata\n \t * buffer, where an application could query it.\n \t */\n-\tDeviceStatus *deviceStatus = rpiMetadata_.getLocked(\"device.status\");\n+\tDeviceStatus *deviceStatus = rpiMetadata.getLocked(\"device.status\");\n \tif (deviceStatus) {\n \t\tlibcameraMetadata_.set(controls::ExposureTime,\n \t\t\t\t deviceStatus->shutterSpeed.get());\n@@ -557,24 +563,24 @@ void IPARPi::reportMetadata()\n \t\t\tlibcameraMetadata_.set(controls::SensorTemperature, *deviceStatus->sensorTemperature);\n \t}\n \n-\tAgcStatus *agcStatus = rpiMetadata_.getLocked(\"agc.status\");\n+\tAgcStatus *agcStatus = rpiMetadata.getLocked(\"agc.status\");\n \tif (agcStatus) {\n \t\tlibcameraMetadata_.set(controls::AeLocked, agcStatus->locked);\n \t\tlibcameraMetadata_.set(controls::DigitalGain, agcStatus->digitalGain);\n \t}\n \n-\tLuxStatus *luxStatus = rpiMetadata_.getLocked(\"lux.status\");\n+\tLuxStatus *luxStatus = rpiMetadata.getLocked(\"lux.status\");\n \tif (luxStatus)\n \t\tlibcameraMetadata_.set(controls::Lux, luxStatus->lux);\n \n-\tAwbStatus *awbStatus = rpiMetadata_.getLocked(\"awb.status\");\n+\tAwbStatus *awbStatus = rpiMetadata.getLocked(\"awb.status\");\n \tif (awbStatus) {\n \t\tlibcameraMetadata_.set(controls::ColourGains, { static_cast(awbStatus->gainR),\n \t\t\t\t\t\t\t\tstatic_cast(awbStatus->gainB) });\n \t\tlibcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);\n \t}\n \n-\tBlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked(\"black_level.status\");\n+\tBlackLevelStatus *blackLevelStatus = rpiMetadata.getLocked(\"black_level.status\");\n \tif (blackLevelStatus)\n \t\tlibcameraMetadata_.set(controls::SensorBlackLevels,\n \t\t\t\t { static_cast(blackLevelStatus->blackLevelR),\n@@ -582,7 +588,7 @@ void IPARPi::reportMetadata()\n \t\t\t\t\t static_cast(blackLevelStatus->blackLevelG),\n \t\t\t\t\t static_cast(blackLevelStatus->blackLevelB) });\n \n-\tFocusStatus *focusStatus = rpiMetadata_.getLocked(\"focus.status\");\n+\tFocusStatus *focusStatus = rpiMetadata.getLocked(\"focus.status\");\n \tif (focusStatus && focusStatus->num == 12) {\n \t\t/*\n \t\t * We get a 4x3 grid of regions by default. Calculate the average\n@@ -593,7 +599,7 @@ void IPARPi::reportMetadata()\n \t\tlibcameraMetadata_.set(controls::FocusFoM, focusFoM);\n \t}\n \n-\tCcmStatus *ccmStatus = rpiMetadata_.getLocked(\"ccm.status\");\n+\tCcmStatus *ccmStatus = rpiMetadata.getLocked(\"ccm.status\");\n \tif (ccmStatus) {\n \t\tfloat m[9];\n \t\tfor (unsigned int i = 0; i < 9; i++)\n@@ -1006,9 +1012,10 @@ void IPARPi::prepareISP(const ISPConfig &data)\n {\n \tint64_t frameTimestamp = data.controls.get(controls::SensorTimestamp).value_or(0);\n \tRPiController::Metadata lastMetadata;\n+\tRPiController::Metadata &rpiMetadata = rpiMetadata_[metadataIdx_];\n \tSpan embeddedBuffer;\n \n-\tlastMetadata = std::move(rpiMetadata_);\n+\tlastMetadata = std::move(rpiMetadata);\n \tfillDeviceStatus(data.controls);\n \n \tif (data.embeddedBufferPresent) {\n@@ -1025,7 +1032,7 @@ void IPARPi::prepareISP(const ISPConfig &data)\n \t * This may overwrite the DeviceStatus using values from the sensor\n \t * metadata, and may also do additional custom processing.\n \t */\n-\thelper_->prepare(embeddedBuffer, rpiMetadata_);\n+\thelper_->prepare(embeddedBuffer, rpiMetadata);\n \n \t/* Done with embedded data now, return to pipeline handler asap. */\n \tif (data.embeddedBufferPresent)\n@@ -1041,7 +1048,7 @@ void IPARPi::prepareISP(const ISPConfig &data)\n \t\t * current frame, or any other bits of metadata that were added\n \t\t * in helper_->Prepare().\n \t\t */\n-\t\trpiMetadata_.merge(lastMetadata);\n+\t\trpiMetadata.merge(lastMetadata);\n \t\tprocessPending_ = false;\n \t\treturn;\n \t}\n@@ -1051,48 +1058,48 @@ void IPARPi::prepareISP(const ISPConfig &data)\n \n \tControlList ctrls(ispCtrls_);\n \n-\tcontroller_.prepare(&rpiMetadata_);\n+\tcontroller_.prepare(&rpiMetadata);\n \n \t/* Lock the metadata buffer to avoid constant locks/unlocks. */\n-\tstd::unique_lock lock(rpiMetadata_);\n+\tstd::unique_lock lock(rpiMetadata);\n \n-\tAwbStatus *awbStatus = rpiMetadata_.getLocked(\"awb.status\");\n+\tAwbStatus *awbStatus = rpiMetadata.getLocked(\"awb.status\");\n \tif (awbStatus)\n \t\tapplyAWB(awbStatus, ctrls);\n \n-\tCcmStatus *ccmStatus = rpiMetadata_.getLocked(\"ccm.status\");\n+\tCcmStatus *ccmStatus = rpiMetadata.getLocked(\"ccm.status\");\n \tif (ccmStatus)\n \t\tapplyCCM(ccmStatus, ctrls);\n \n-\tAgcStatus *dgStatus = rpiMetadata_.getLocked(\"agc.status\");\n+\tAgcStatus *dgStatus = rpiMetadata.getLocked(\"agc.status\");\n \tif (dgStatus)\n \t\tapplyDG(dgStatus, ctrls);\n \n-\tAlscStatus *lsStatus = rpiMetadata_.getLocked(\"alsc.status\");\n+\tAlscStatus *lsStatus = rpiMetadata.getLocked(\"alsc.status\");\n \tif (lsStatus)\n \t\tapplyLS(lsStatus, ctrls);\n \n-\tContrastStatus *contrastStatus = rpiMetadata_.getLocked(\"contrast.status\");\n+\tContrastStatus *contrastStatus = rpiMetadata.getLocked(\"contrast.status\");\n \tif (contrastStatus)\n \t\tapplyGamma(contrastStatus, ctrls);\n \n-\tBlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked(\"black_level.status\");\n+\tBlackLevelStatus *blackLevelStatus = rpiMetadata.getLocked(\"black_level.status\");\n \tif (blackLevelStatus)\n \t\tapplyBlackLevel(blackLevelStatus, ctrls);\n \n-\tGeqStatus *geqStatus = rpiMetadata_.getLocked(\"geq.status\");\n+\tGeqStatus *geqStatus = rpiMetadata.getLocked(\"geq.status\");\n \tif (geqStatus)\n \t\tapplyGEQ(geqStatus, ctrls);\n \n-\tDenoiseStatus *denoiseStatus = rpiMetadata_.getLocked(\"denoise.status\");\n+\tDenoiseStatus *denoiseStatus = rpiMetadata.getLocked(\"denoise.status\");\n \tif (denoiseStatus)\n \t\tapplyDenoise(denoiseStatus, ctrls);\n \n-\tSharpenStatus *sharpenStatus = rpiMetadata_.getLocked(\"sharpen.status\");\n+\tSharpenStatus *sharpenStatus = rpiMetadata.getLocked(\"sharpen.status\");\n \tif (sharpenStatus)\n \t\tapplySharpen(sharpenStatus, ctrls);\n \n-\tDpcStatus *dpcStatus = rpiMetadata_.getLocked(\"dpc.status\");\n+\tDpcStatus *dpcStatus = rpiMetadata.getLocked(\"dpc.status\");\n \tif (dpcStatus)\n \t\tapplyDPC(dpcStatus, ctrls);\n \n@@ -1116,11 +1123,13 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n \n \tLOG(IPARPI, Debug) << \"Metadata - \" << deviceStatus;\n \n-\trpiMetadata_.set(\"device.status\", deviceStatus);\n+\trpiMetadata_[metadataIdx_].set(\"device.status\", deviceStatus);\n }\n \n void IPARPi::processStats(unsigned int bufferId)\n {\n+\tRPiController::Metadata &rpiMetadata = rpiMetadata_[metadataIdx_];\n+\n \tauto it = buffers_.find(bufferId);\n \tif (it == buffers_.end()) {\n \t\tLOG(IPARPI, Error) << \"Could not find stats buffer!\";\n@@ -1130,11 +1139,11 @@ void IPARPi::processStats(unsigned int bufferId)\n \tSpan mem = it->second.planes()[0];\n \tbcm2835_isp_stats *stats = reinterpret_cast(mem.data());\n \tRPiController::StatisticsPtr statistics = std::make_shared(*stats);\n-\thelper_->process(statistics, rpiMetadata_);\n-\tcontroller_.process(statistics, &rpiMetadata_);\n+\thelper_->process(statistics, rpiMetadata);\n+\tcontroller_.process(statistics, &rpiMetadata);\n \n \tstruct AgcStatus agcStatus;\n-\tif (rpiMetadata_.get(\"agc.status\", agcStatus) == 0) {\n+\tif (rpiMetadata.get(\"agc.status\", agcStatus) == 0) {\n \t\tControlList ctrls(sensorCtrls_);\n \t\tapplyAGC(&agcStatus, ctrls);\n \n","prefixes":["libcamera-devel","v4","5/7"]}