{"id":17798,"url":"https://patchwork.libcamera.org/api/1.1/patches/17798/?format=json","web_url":"https://patchwork.libcamera.org/patch/17798/","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":"<20221115090755.2921-7-naush@raspberrypi.com>","date":"2022-11-15T09:07:53","name":"[libcamera-devel,v6,6/8] ipa: raspberrypi: Use an array of RPiController::Metadata objects","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"80c832c18a216048e6f7a7cb525055a80b6e14ee","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/17798/mbox/","series":[{"id":3609,"url":"https://patchwork.libcamera.org/api/1.1/series/3609/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3609","date":"2022-11-15T09:07:47","name":"Raspberry Pi AGC digital gain fixes","version":6,"mbox":"https://patchwork.libcamera.org/series/3609/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/17798/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/17798/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 36034BE08B\n\tfor ;\n\tTue, 15 Nov 2022 09:08:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E316563089;\n\tTue, 15 Nov 2022 10:08:10 +0100 (CET)","from mail-wr1-x431.google.com (mail-wr1-x431.google.com\n\t[IPv6:2a00:1450:4864:20::431])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0A8863097\n\tfor ;\n\tTue, 15 Nov 2022 10:08:05 +0100 (CET)","by mail-wr1-x431.google.com with SMTP id z14so22945398wrn.7\n\tfor ;\n\tTue, 15 Nov 2022 01:08:05 -0800 (PST)","from naush-laptop.localdomain ([93.93.133.154])\n\tby smtp.gmail.com with ESMTPSA id\n\tp25-20020a05600c1d9900b003c64c186206sm15143696wms.16.2022.11.15.01.08.04\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 15 Nov 2022 01:08:04 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668503290;\n\tbh=jlRfxeMNJmHpGoiVrybdPgGhig7tY5AWa9GoNAXL7UI=;\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=UlrBDyOiYjup7vPPYfs24omnJcAffF/47OyjHlpAK2U8M0isVP8iMTBmN4CfPwRJC\n\tE8eUYAMlhyB0+VOPPhhuKqmsYej7ZQtNQcawsUBVxJJZGMYMIdYxZYOUUB/geySz4o\n\tzkxppRI8+tz5U/2hxWX0aK+ZveRjc4znXhtJngajHbcH6NX2VJOgEtEYWzsNi1XEIm\n\tIJWNqwt9CQbrJPsASkyLNqverGwfCX0vzVxsnLgO1B1ZNdLyPGh0mbk4u1bdvkXI9r\n\tqeCi6ylcyL4a97WzYUYA/vmZVLn875nIdvLQpZ7t/Pw2fEqxqFceOTzuOHPTtEkxeR\n\tMNchDahARPsGg==","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=7pCZVHEGkH4rkfudxa5QoNd2EeOZUCc279G5kmfq7Bk=;\n\tb=XwLnmbttdCZAw1Ab1iumLNvE3cbdXp3aQmRmOUuIYf9GA2bHwf2lacfFB37jvZ+Gu4\n\tQp1I8ePXu7c+D/UEQX7b8hUOwYuqg4INQlyAu7fvEr4G63R/SBaEidQ+hsb0wpQN84vL\n\tioBk6yrC4NZ310lztXCh77auiLKs4xsQHXh6ixb5BdTos7zwgds0kHmIZ/NKKh+xAKMb\n\tFzlyNtKd8lpgHIpggDJoUNueCrmt4PUJYgvYBF+SgYuZbqOnr/fznzwFOzdT8HvpNNB/\n\tWCin3uA/EybsHARZo47j6h4GpX6hlaa/lh+48QCCf5kWkKJmPfR/sBHydDvWqiIfwWmH\n\t4z0g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"XwLnmbtt\"; 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=7pCZVHEGkH4rkfudxa5QoNd2EeOZUCc279G5kmfq7Bk=;\n\tb=awtT142bEtYoM87abqosl2Hz8tQ6CvUFEGqxIu6RKDjlfu+/h+jgKnfuvKlyNgiJHG\n\tijSpKqc3zMQedomtPix+q4WzWMxabsi68EjeJkvwUHoHTW+7qu4cfJyWNhimH9Y8BKJ0\n\txW6cwSNV8HCIyiWho7kVHxUFjnszLW2pkBK1UayXPMWzf15MxYDYxNV6cSS0Xc0PF9g+\n\t+S7StY3CbsqspRqSB0MmdGn6f5/q/tqHKkWQBwvBiC3c0hs6xLqEL1XkJQaM4Q0kC/b/\n\twJri578Rw5QxPozcCTr+6kjqUZidBuU5Gb29aGwFtRRxwVOZUWVyjXzNQSJfxsU89daG\n\t1Cfg==","X-Gm-Message-State":"ANoB5pkmtX1udu8fCPN3K4aYal6sIFrXnXfg2bqMYr2ES3hFiaLKulE4\n\tim7kdm0PnSQumdWs3JCDdA9xmhOhDo/uCQ==","X-Google-Smtp-Source":"AA0mqf49O+mFJiDRfrcBwN7wQIVAicNQ2nFaqctkuluGLqnHyt2V+oWGWSiOl9v5xWdVGDtto3KPzA==","X-Received":"by 2002:a5d:46c6:0:b0:236:8710:41f2 with SMTP id\n\tg6-20020a5d46c6000000b00236871041f2mr9968225wrs.239.1668503285110; \n\tTue, 15 Nov 2022 01:08:05 -0800 (PST)","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 15 Nov 2022 09:07:53 +0000","Message-Id":"<20221115090755.2921-7-naush@raspberrypi.com>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20221115090755.2921-1-naush@raspberrypi.com>","References":"<20221115090755.2921-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v6 6/8] 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 \nReviewed-by: Kieran Bingham \n---\n src/ipa/raspberrypi/raspberrypi.cpp | 63 ++++++++++++++++-------------\n 1 file changed, 35 insertions(+), 28 deletions(-)","diff":"diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex b74f1ecf738f..799a4fe70000 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,7 @@ 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 \n \t/*\n \t * We count frames to decide if the frame must be hidden (e.g. from\n@@ -539,14 +542,15 @@ void IPARPi::signalIspPrepare(const ISPConfig &data)\n \n void IPARPi::reportMetadata()\n {\n-\tstd::unique_lock lock(rpiMetadata_);\n+\tRPiController::Metadata &rpiMetadata = rpiMetadata_[0];\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 +561,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 +586,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 +597,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 +1010,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_[0];\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 +1030,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 +1046,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 +1056,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 +1121,13 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n \n \tLOG(IPARPI, Debug) << \"Metadata - \" << deviceStatus;\n \n-\trpiMetadata_.set(\"device.status\", deviceStatus);\n+\trpiMetadata_[0].set(\"device.status\", deviceStatus);\n }\n \n void IPARPi::processStats(unsigned int bufferId)\n {\n+\tRPiController::Metadata &rpiMetadata = rpiMetadata_[0];\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 +1137,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","v6","6/8"]}