From patchwork Mon Sep 26 09:57:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 17402 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 F2535BD16B for ; Mon, 26 Sep 2022 09:58:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A781A6225E; Mon, 26 Sep 2022 11:58:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1664186292; bh=FdO+0iW6DgkgyYw5e18qWIoV4tuve6nHlsQNyOBKS0o=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=0Y8vCAlol3tzdGYxqiPDVCdyX2tr5VZjJ2pKk5lQ8DfCcMrTAy+5+DeK5RJrkI8SS Bkad2JkaY7OXn6IQeISHY2vsdxw14EUIcbARJ6b2lmpx6MlOf8ZYsdXBQcF8L5yIHQ NRt8hkwwpOmhrTos1Qe+ilFVnKZWeoS9Ck+GLg08zyOpX2PpHRJdOyuGUbklFqunZi dP3UGA1a/VYjQNEt6Ww6zWiYfL3Z6kAx0pHWu/9DWAEH4QxH9Fn/gmXOCGopn8Y+RP /To2/pYtMBMjCm/x4OqrsrKZ1niKTyd3di+XPE5SCRheTkgGyj5tne3WYB27OkUvNO rfXn2nAoTUJiQ== Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 887476224C for ; Mon, 26 Sep 2022 11:58:11 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="MIDXOyEm"; dkim-atps=neutral Received: by mail-wm1-x334.google.com with SMTP id n35-20020a05600c502300b003b4924c6868so7778695wmr.1 for ; Mon, 26 Sep 2022 02:58:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date; bh=J0rP7W+kbeDPc1Be4ITqwvmzGhcrPvfbVLStz8XRHTA=; b=MIDXOyEmRzvpQBI7ffCYsGeTnwA7mf5DhfEttHLPQA6Wr4jXYw+TGACcROptQMcuo/ 0+O6/ThG5BZWn9bd3UvDaAVTE8e14G2PW2zdT4poYTeRGAMBFKtdFf7opv1r7Xpdzrvc hDSaWgUhPGs/Hn3aAuj1Wt5VkkRUTt8CNwf46KWjjR0J4AjFrAMnFp2sF6HtOwZkEoev 3v8cffMoRyEWbC/0AUDFQo9dS3Tl6tSNctI/xtExaRK2eCpMHexb1yXGoGeNSg5GQuHv BDJxS+NUonVf7O8jPxrnirqY7XAUuXlWUqpAOtxJgy3BxvwhdjrLapAtjfrevaiIekSH 6Mag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date; bh=J0rP7W+kbeDPc1Be4ITqwvmzGhcrPvfbVLStz8XRHTA=; b=cW+dhUrAj2tN7NTrSvJ1u3kYRRVTrMQGJRemXZm4fxfr8Aeer0B8c6XOAJS0gxV3iX AITQddZ2xC/tmcD3Sp2NmoacwI5SRW7L0qtXZ4Vb3RZD3Ih/3PIHNZ16VdWI6ZeEEH1N 81QOMWppnRDQulAUE+e+jIslrXipbCqzRd65xWlOWexrirBNrW6EbVeRV04M3K1zQWF7 WRiLlZ5DSbcm4DcLOnkEjRy20Pw8UWDMt+Sv49nXq2LKfRf+RyLXSR/yQojbW2Zmqk7S wYIEtA2Ee1WLffj7+51Mu176lSOEdAu4G0lwP5xuIlBD7HgqE1bgEi9gNcQ3bGwDzK9L 5AHw== X-Gm-Message-State: ACrzQf3RhBLWflH6+mnpqIwPM/0SgNxz4iTWMxYgCayTxfOfYmQjsTaB 2YdcGxPkGnr97Bwbb4W0fhVE2gF2jrgHy6fk X-Google-Smtp-Source: AMsMyM5awHHv0roMXulIIth7KaspwmyoCNVPwGwkCAjnNS7dMAQLzCJIh57R6LlfNeRx4cZyKSVOmA== X-Received: by 2002:a05:600c:1906:b0:3b4:c979:e686 with SMTP id j6-20020a05600c190600b003b4c979e686mr14742722wmq.107.1664186290836; Mon, 26 Sep 2022 02:58:10 -0700 (PDT) Received: from naush-laptop.localdomain ([93.93.133.154]) by smtp.gmail.com with ESMTPSA id p22-20020a05600c419600b003a4efb794d7sm10160671wmh.36.2022.09.26.02.58.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Sep 2022 02:58:09 -0700 (PDT) To: libcamera-devel@lists.libcamera.org Date: Mon, 26 Sep 2022 10:57:35 +0100 Message-Id: <20220926095737.30506-6-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220926095737.30506-1-naush@raspberrypi.com> References: <20220926095737.30506-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 5/7] ipa: raspberrypi: Use an array of RPiController::Metadata objects 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: Naushir Patuck via libcamera-devel From: Naushir Patuck Reply-To: Naushir Patuck Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Cycle through an array of RPiController::Metadata objects, one per prepare()/process() invocation, when running the controller algorithms. This allows historical metadata objects to be retained, and subsequently passed into the controller algorithms on future frames. This change provides a route to fixing a problem with the AGC algorithm, where if a manual shutter/gain is requested, the algorithm does not currently retain any context of the total exposure that it has calculated. As a result, the wrong digital gain would be applied when the frame with the manual shutter/gain is processed by the ISP. Signed-off-by: Naushir Patuck Reviewed-by: David Plowman Tested-by: David Plowman --- src/ipa/raspberrypi/raspberrypi.cpp | 69 +++++++++++++++++------------ 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 8d731435764e..63cccda901c1 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -57,6 +57,9 @@ namespace libcamera { using namespace std::literals::chrono_literals; using utils::Duration; +/* Number of metadata objects available in the context list. */ +constexpr unsigned int numMetadataContexts = 16; + /* Configure the sensor with these values initially. */ constexpr double defaultAnalogueGain = 1.0; constexpr Duration defaultExposureTime = 20.0ms; @@ -163,7 +166,8 @@ private: /* Raspberry Pi controller specific defines. */ std::unique_ptr helper_; RPiController::Controller controller_; - RPiController::Metadata rpiMetadata_; + std::array rpiMetadata_; + unsigned int metadataIdx_; /* * We count frames to decide if the frame must be hidden (e.g. from @@ -319,6 +323,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig) firstStart_ = false; lastRunTimestamp_ = 0; + metadataIdx_ = 0; } void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo) @@ -513,6 +518,7 @@ void IPARPi::signalStatReady(uint32_t bufferId) reportMetadata(); statsMetadataComplete.emit(bufferId & MaskID, libcameraMetadata_); + metadataIdx_ = (metadataIdx_ + 1) % rpiMetadata_.size(); } void IPARPi::signalQueueRequest(const ControlList &controls) @@ -536,14 +542,15 @@ void IPARPi::signalIspPrepare(const ISPConfig &data) void IPARPi::reportMetadata() { - std::unique_lock lock(rpiMetadata_); + RPiController::Metadata &rpiMetadata = rpiMetadata_[metadataIdx_]; + std::unique_lock lock(rpiMetadata); /* * Certain information about the current frame and how it will be * processed can be extracted and placed into the libcamera metadata * buffer, where an application could query it. */ - DeviceStatus *deviceStatus = rpiMetadata_.getLocked("device.status"); + DeviceStatus *deviceStatus = rpiMetadata.getLocked("device.status"); if (deviceStatus) { libcameraMetadata_.set(controls::ExposureTime, deviceStatus->shutterSpeed.get()); @@ -554,17 +561,17 @@ void IPARPi::reportMetadata() libcameraMetadata_.set(controls::SensorTemperature, *deviceStatus->sensorTemperature); } - AgcStatus *agcStatus = rpiMetadata_.getLocked("agc.status"); + AgcStatus *agcStatus = rpiMetadata.getLocked("agc.status"); if (agcStatus) { libcameraMetadata_.set(controls::AeLocked, agcStatus->locked); libcameraMetadata_.set(controls::DigitalGain, agcStatus->digitalGain); } - LuxStatus *luxStatus = rpiMetadata_.getLocked("lux.status"); + LuxStatus *luxStatus = rpiMetadata.getLocked("lux.status"); if (luxStatus) libcameraMetadata_.set(controls::Lux, luxStatus->lux); - AwbStatus *awbStatus = rpiMetadata_.getLocked("awb.status"); + AwbStatus *awbStatus = rpiMetadata.getLocked("awb.status"); if (awbStatus) { libcameraMetadata_.set(controls::ColourGains, Span({ static_cast(awbStatus->gainR), @@ -572,7 +579,7 @@ void IPARPi::reportMetadata() libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK); } - BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked("black_level.status"); + BlackLevelStatus *blackLevelStatus = rpiMetadata.getLocked("black_level.status"); if (blackLevelStatus) libcameraMetadata_.set(controls::SensorBlackLevels, Span({ static_cast(blackLevelStatus->blackLevelR), @@ -580,7 +587,7 @@ void IPARPi::reportMetadata() static_cast(blackLevelStatus->blackLevelG), static_cast(blackLevelStatus->blackLevelB) })); - FocusStatus *focusStatus = rpiMetadata_.getLocked("focus.status"); + FocusStatus *focusStatus = rpiMetadata.getLocked("focus.status"); if (focusStatus && focusStatus->num == 12) { /* * We get a 4x3 grid of regions by default. Calculate the average @@ -591,7 +598,7 @@ void IPARPi::reportMetadata() libcameraMetadata_.set(controls::FocusFoM, focusFoM); } - CcmStatus *ccmStatus = rpiMetadata_.getLocked("ccm.status"); + CcmStatus *ccmStatus = rpiMetadata.getLocked("ccm.status"); if (ccmStatus) { float m[9]; for (unsigned int i = 0; i < 9; i++) @@ -1002,10 +1009,10 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId) void IPARPi::prepareISP(const ISPConfig &data) { int64_t frameTimestamp = data.controls.get(controls::SensorTimestamp).value_or(0); - RPiController::Metadata lastMetadata; + RPiController::Metadata &rpiMetadata = rpiMetadata_[metadataIdx_]; Span embeddedBuffer; - lastMetadata = std::move(rpiMetadata_); + rpiMetadata.clear(); fillDeviceStatus(data.controls); if (data.embeddedBufferPresent) { @@ -1022,7 +1029,7 @@ void IPARPi::prepareISP(const ISPConfig &data) * This may overwrite the DeviceStatus using values from the sensor * metadata, and may also do additional custom processing. */ - helper_->prepare(embeddedBuffer, rpiMetadata_); + helper_->prepare(embeddedBuffer, rpiMetadata); /* Done with embedded data now, return to pipeline handler asap. */ if (data.embeddedBufferPresent) @@ -1038,7 +1045,9 @@ void IPARPi::prepareISP(const ISPConfig &data) * current frame, or any other bits of metadata that were added * in helper_->Prepare(). */ - rpiMetadata_.merge(lastMetadata); + RPiController::Metadata &lastMetadata = + rpiMetadata_[metadataIdx_ == 0 ? rpiMetadata_.size() - 1 : metadataIdx_ - 1]; + rpiMetadata.mergeCopy(lastMetadata); processPending_ = false; return; } @@ -1048,48 +1057,48 @@ void IPARPi::prepareISP(const ISPConfig &data) ControlList ctrls(ispCtrls_); - controller_.prepare(&rpiMetadata_); + controller_.prepare(&rpiMetadata); /* Lock the metadata buffer to avoid constant locks/unlocks. */ - std::unique_lock lock(rpiMetadata_); + std::unique_lock lock(rpiMetadata); - AwbStatus *awbStatus = rpiMetadata_.getLocked("awb.status"); + AwbStatus *awbStatus = rpiMetadata.getLocked("awb.status"); if (awbStatus) applyAWB(awbStatus, ctrls); - CcmStatus *ccmStatus = rpiMetadata_.getLocked("ccm.status"); + CcmStatus *ccmStatus = rpiMetadata.getLocked("ccm.status"); if (ccmStatus) applyCCM(ccmStatus, ctrls); - AgcStatus *dgStatus = rpiMetadata_.getLocked("agc.status"); + AgcStatus *dgStatus = rpiMetadata.getLocked("agc.status"); if (dgStatus) applyDG(dgStatus, ctrls); - AlscStatus *lsStatus = rpiMetadata_.getLocked("alsc.status"); + AlscStatus *lsStatus = rpiMetadata.getLocked("alsc.status"); if (lsStatus) applyLS(lsStatus, ctrls); - ContrastStatus *contrastStatus = rpiMetadata_.getLocked("contrast.status"); + ContrastStatus *contrastStatus = rpiMetadata.getLocked("contrast.status"); if (contrastStatus) applyGamma(contrastStatus, ctrls); - BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked("black_level.status"); + BlackLevelStatus *blackLevelStatus = rpiMetadata.getLocked("black_level.status"); if (blackLevelStatus) applyBlackLevel(blackLevelStatus, ctrls); - GeqStatus *geqStatus = rpiMetadata_.getLocked("geq.status"); + GeqStatus *geqStatus = rpiMetadata.getLocked("geq.status"); if (geqStatus) applyGEQ(geqStatus, ctrls); - DenoiseStatus *denoiseStatus = rpiMetadata_.getLocked("denoise.status"); + DenoiseStatus *denoiseStatus = rpiMetadata.getLocked("denoise.status"); if (denoiseStatus) applyDenoise(denoiseStatus, ctrls); - SharpenStatus *sharpenStatus = rpiMetadata_.getLocked("sharpen.status"); + SharpenStatus *sharpenStatus = rpiMetadata.getLocked("sharpen.status"); if (sharpenStatus) applySharpen(sharpenStatus, ctrls); - DpcStatus *dpcStatus = rpiMetadata_.getLocked("dpc.status"); + DpcStatus *dpcStatus = rpiMetadata.getLocked("dpc.status"); if (dpcStatus) applyDPC(dpcStatus, ctrls); @@ -1111,11 +1120,13 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls) LOG(IPARPI, Debug) << "Metadata - " << deviceStatus; - rpiMetadata_.set("device.status", deviceStatus); + rpiMetadata_[metadataIdx_].set("device.status", deviceStatus); } void IPARPi::processStats(unsigned int bufferId) { + RPiController::Metadata &rpiMetadata = rpiMetadata_[metadataIdx_]; + auto it = buffers_.find(bufferId); if (it == buffers_.end()) { LOG(IPARPI, Error) << "Could not find stats buffer!"; @@ -1125,11 +1136,11 @@ void IPARPi::processStats(unsigned int bufferId) Span mem = it->second.planes()[0]; bcm2835_isp_stats *stats = reinterpret_cast(mem.data()); RPiController::StatisticsPtr statistics = std::make_shared(*stats); - helper_->process(statistics, rpiMetadata_); - controller_.process(statistics, &rpiMetadata_); + helper_->process(statistics, rpiMetadata); + controller_.process(statistics, &rpiMetadata); struct AgcStatus agcStatus; - if (rpiMetadata_.get("agc.status", agcStatus) == 0) { + if (rpiMetadata.get("agc.status", agcStatus) == 0) { ControlList ctrls(sensorCtrls_); applyAGC(&agcStatus, ctrls);