From patchwork Fri Sep 15 12:29:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 19016 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 3148ABE080 for ; Fri, 15 Sep 2023 12:30:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DDDFC62916; Fri, 15 Sep 2023 14:30:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1694781023; bh=91KEkq1VPnk5kUo3EgxZP152fnxIFT0A4RygKV8pwuM=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ZsZe+l+Hrspluye9Sje8Kgi6tmrTCBgpuNyuBMgrMPT1mANPQiGu1IctThde5DySO fnZCIO0LFCh/yHxiWvLbc94VC4GWSQT+3wF6qkaBni9WgZYqj6ifYvJWTBHrAGLFZK 1NYI7wUBH31f+oaEVOcO5B/8pDJSwFiwd1Vs5077qx8IjcARmsojr63NtjY6sKfunE sfNr31uF8NRIwkYOXSQDWoESoBBlWhXEj08TW0AH5Yx7jgyZPbBp/CTCDTkkK0n4JW YlZgvxhieMMC8CuQDNcQlzGDZIzMhLlqxV6K7bIyqwLJjlkJum6Plu3Y10AgA0xcDB LtYWmmqBFy7CA== Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E419862916 for ; Fri, 15 Sep 2023 14:30:21 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="faxOyMcZ"; dkim-atps=neutral Received: by mail-wm1-x333.google.com with SMTP id 5b1f17b1804b1-40475103519so16161655e9.0 for ; Fri, 15 Sep 2023 05:30:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1694781021; x=1695385821; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=DZB5Z+m5mdkinAnRiGZsSDQRNrqXoyqf43kmdwlAz3c=; b=faxOyMcZSOMFXAAnOOwTB93q/QZtqz2TaBZmovp3Uz+Ex8vKBpxNgAvVefbLjcmrJE 2tC0FUgxf2PbqfWdgqZdFZYZ8kSLCUjVDxiBJNXvPHrqMhoncjsJ2jsdqG4rhraQfNoT 9d0s0Ua1wOIerQrgZTmcJMs/qgDiDcTogv/mpDjKzbmKVzr85qvdZ4YYlzwwKtdcGN+Y xB6WZFPYL5/Dun3A9n2guB1YYKE5ZesPJ5+Mhi4KaIEEsb9W8sS/8noZpoGeepor6vEO ZhbNey/TTEi7agQqLSYwuraF4EDfdee90mcnld57NODgFqh1NocVtCGQp4/w59v1n1m5 wJtw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694781021; x=1695385821; 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:message-id:reply-to; bh=DZB5Z+m5mdkinAnRiGZsSDQRNrqXoyqf43kmdwlAz3c=; b=cd5r4GLiKRRC3xpKlmsGMcS9fHOBfSu237FRf1eHPEAjyolP+rrqtrjDBhwFHbxkRw tXxGSqJZDARyQwCzVcZmRoD5dBvjHWKrmbf0o12znIQ3TZuZr3nwaqKtSrdrTM9fraVO /Gy83rVVFy2NzHTbZ/AmZBkH9ELdqfMEYKSew03E17if+cMA6HobvN3618o1RB/BV5rj 8RozXFYyUo2ONgItZ57BY9+c9cZBt8SQU9Yuj2Pa4wTxIFkzn5Riibl51c83ef45xlPK LpMzmkR8sNoJL3H2JTioeuQuE1YGM1LMUITnkrSu6EixJ1GZxAq8nas67UypxFELBoBT eaNQ== X-Gm-Message-State: AOJu0YzJabL7coeofPJdZkxJWX4FqIUxj8iJBbq8WlFKbO6RE7lseBIU 77dpV75affKg6gevZcSfbWKBb41GONG8hHaJBD4= X-Google-Smtp-Source: AGHT+IE/9BtI1x0TE99fOTHg8LIxGfmHVJ/tazw7UrXoqaFxhNGyi5PXvXANi4RcKaa5eaZpDgo2nA== X-Received: by 2002:a05:600c:ad7:b0:402:f555:6523 with SMTP id c23-20020a05600c0ad700b00402f5556523mr1447814wmr.9.1694781020998; Fri, 15 Sep 2023 05:30:20 -0700 (PDT) Received: from localhost.localdomain ([195.180.61.40]) by smtp.gmail.com with ESMTPSA id y14-20020a7bcd8e000000b004030e8ff964sm7435009wmj.34.2023.09.15.05.30.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Sep 2023 05:30:20 -0700 (PDT) To: libcamera-devel@lists.libcamera.org Date: Fri, 15 Sep 2023 13:29:52 +0100 Message-Id: <20230915122954.5231-4-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230915122954.5231-1-david.plowman@raspberrypi.com> References: <20230915122954.5231-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/5] ipa: rpi: agc: Implementation of multi-channel AGC 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: David Plowman via libcamera-devel From: David Plowman Reply-To: David Plowman Cc: Jacopo Mondi Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The switchMode, prepare and process methods are updated to implement multi-channel AGC correctly: * switchMode now invokes switchMode on all the channels (whether active or not). * prepare must find what channel the current frame is, and run on behalf of that channel. * process updates the most recent DeviceStatus and statistics for the channel of the frame that has just arrived, but generates updated values working through the active channels in round-robin fashion. One minor detail in process is that we don't want to change the DeviceStatus metadata of the current frame, so we now pass this to the AgcChannel's process method, rather than letting it find the DeviceStatus in the metadata. Signed-off-by: David Plowman Reviewed-by: Naushir Patuck Reviewed-by: Jacopo Mondi --- src/ipa/rpi/controller/agc_status.h | 1 + src/ipa/rpi/controller/rpi/agc.cpp | 110 +++++++++++++++++++-- src/ipa/rpi/controller/rpi/agc.h | 4 + src/ipa/rpi/controller/rpi/agc_channel.cpp | 19 ++-- src/ipa/rpi/controller/rpi/agc_channel.h | 4 +- 5 files changed, 116 insertions(+), 22 deletions(-) diff --git a/src/ipa/rpi/controller/agc_status.h b/src/ipa/rpi/controller/agc_status.h index 597eddd7..e5c4ee22 100644 --- a/src/ipa/rpi/controller/agc_status.h +++ b/src/ipa/rpi/controller/agc_status.h @@ -36,6 +36,7 @@ struct AgcStatus { int floatingRegionEnable; libcamera::utils::Duration fixedShutter; double fixedAnalogueGain; + unsigned int channel; }; struct AgcPrepareStatus { diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp index 598fc890..8b9d2026 100644 --- a/src/ipa/rpi/controller/rpi/agc.cpp +++ b/src/ipa/rpi/controller/rpi/agc.cpp @@ -22,7 +22,7 @@ LOG_DEFINE_CATEGORY(RPiAgc) Agc::Agc(Controller *controller) : AgcAlgorithm(controller), - activeChannels_({ 0 }) + activeChannels_({ 0 }), index_(0) { } @@ -205,20 +205,116 @@ void Agc::setActiveChannels(const std::vector &activeChannels) void Agc::switchMode(CameraMode const &cameraMode, Metadata *metadata) { - LOG(RPiAgc, Debug) << "switchMode for channel 0"; - channelData_[0].channel.switchMode(cameraMode, metadata); + /* + * We run switchMode on every channel, and then we're going to start over + * with the first active channel again which means that this channel's + * status needs to be the one we leave in the metadata. + */ + AgcStatus status; + + for (unsigned int channelIndex = 0; channelIndex < channelData_.size(); channelIndex++) { + LOG(RPiAgc, Debug) << "switchMode for channel " << channelIndex; + channelData_[channelIndex].channel.switchMode(cameraMode, metadata); + if (channelIndex == activeChannels_[0]) + metadata->get("agc.status", status); + } + + status.channel = activeChannels_[0]; + metadata->set("agc.status", status); + index_ = 0; +} + +static void getDelayedChannelIndex(Metadata *metadata, const char *message, unsigned int &channelIndex) +{ + std::unique_lock lock(*metadata); + AgcStatus *status = metadata->getLocked("agc.delayed_status"); + if (status) + channelIndex = status->channel; + else { + /* This does happen at startup, otherwise it would be a Warning or Error. */ + LOG(RPiAgc, Debug) << message; + } +} + +static void setCurrentChannelIndex(Metadata *metadata, const char *message, unsigned int channelIndex) +{ + std::unique_lock lock(*metadata); + AgcStatus *status = metadata->getLocked("agc.status"); + if (status) + status->channel = channelIndex; + else { + /* This does happen at startup, otherwise it would be a Warning or Error. */ + LOG(RPiAgc, Debug) << message; + } } void Agc::prepare(Metadata *imageMetadata) { - LOG(RPiAgc, Debug) << "prepare for channel 0"; - channelData_[0].channel.prepare(imageMetadata); + /* + * The DeviceStatus in the metadata should be correct for the image we + * are processing. The delayed status should tell us what channel this frame + * was from, so we will use that channel's prepare method. + * + * \todo To be honest, there's not much that's stateful in the prepare methods + * so we should perhaps re-evaluate whether prepare even needs to be done + * "per channel". + */ + unsigned int channelIndex = activeChannels_[0]; + getDelayedChannelIndex(imageMetadata, "prepare: no delayed status", channelIndex); + + LOG(RPiAgc, Debug) << "prepare for channel " << channelIndex; + channelData_[channelIndex].channel.prepare(imageMetadata); } void Agc::process(StatisticsPtr &stats, Metadata *imageMetadata) { - LOG(RPiAgc, Debug) << "process for channel 0"; - channelData_[0].channel.process(stats, imageMetadata); + /* + * We want to generate values for the next channel in round robin fashion + * (i.e. the channel at location index_ in the activeChannel list), even though + * the statistics we have will be for a different channel (which we find + * again from the delayed status). + */ + + /* Generate updated AGC values for channel for new channel that we are requesting. */ + unsigned int channelIndex = activeChannels_[index_]; + AgcChannelData &channelData = channelData_[channelIndex]; + /* The stats that arrived with this image correspond to the following channel. */ + unsigned int statsIndex = 0; + getDelayedChannelIndex(imageMetadata, "process: no delayed status for stats", statsIndex); + LOG(RPiAgc, Debug) << "process for channel " << channelIndex; + + /* + * We keep a cache of the most recent DeviceStatus and stats for each channel, + * so that we can invoke the next channel's process method with the most up to date + * values. + */ + LOG(RPiAgc, Debug) << "Save DeviceStatus and stats for channel " << statsIndex; + DeviceStatus deviceStatus; + if (imageMetadata->get("device.status", deviceStatus) == 0) + channelData_[statsIndex].deviceStatus = deviceStatus; + else + /* Every frame should have a DeviceStatus. */ + LOG(RPiAgc, Error) << "process: no device status found"; + channelData_[statsIndex].statistics = stats; + + /* + * Finally fetch the most recent DeviceStatus and stats for the new channel, if both + * exist, and call process(). We must make the agc.status metadata record correctly + * which channel this is. + */ + if (channelData.statistics && channelData.deviceStatus) { + deviceStatus = *channelData.deviceStatus; + stats = channelData.statistics; + } else { + /* Can also happen when new channels start. */ + LOG(RPiAgc, Debug) << "process: channel " << channelIndex << " not seen yet"; + } + + channelData.channel.process(stats, deviceStatus, imageMetadata); + setCurrentChannelIndex(imageMetadata, "process: no AGC status found", channelIndex); + + /* And onto the next channel for the next call. */ + index_ = (index_ + 1) % activeChannels_.size(); } /* Register algorithm with the system. */ diff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h index a476ea70..ee85c693 100644 --- a/src/ipa/rpi/controller/rpi/agc.h +++ b/src/ipa/rpi/controller/rpi/agc.h @@ -6,6 +6,7 @@ */ #pragma once +#include #include #include @@ -17,6 +18,8 @@ namespace RPiController { struct AgcChannelData { AgcChannel channel; + std::optional deviceStatus; + StatisticsPtr statistics; }; class Agc : public AgcAlgorithm @@ -51,6 +54,7 @@ private: int checkChannel(unsigned int channel) const; std::vector channelData_; std::vector activeChannels_; + unsigned int index_; /* index into the activeChannels_ */ }; } /* namespace RPiController */ diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp index b9b03af5..9c8d1a6f 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp @@ -444,7 +444,7 @@ void AgcChannel::prepare(Metadata *imageMetadata) } } -void AgcChannel::process(StatisticsPtr &stats, Metadata *imageMetadata) +void AgcChannel::process(StatisticsPtr &stats, DeviceStatus const &deviceStatus, Metadata *imageMetadata) { frameCount_++; /* @@ -455,7 +455,7 @@ void AgcChannel::process(StatisticsPtr &stats, Metadata *imageMetadata) /* Fetch the AWB status immediately, so that we can assume it's there. */ fetchAwbStatus(imageMetadata); /* Get the current exposure values for the frame that's just arrived. */ - fetchCurrentExposure(imageMetadata); + fetchCurrentExposure(deviceStatus); /* Compute the total gain we require relative to the current exposure. */ double gain, targetY; computeGain(stats, imageMetadata, gain, targetY); @@ -567,18 +567,11 @@ void AgcChannel::housekeepConfig() << meteringModeName_; } -void AgcChannel::fetchCurrentExposure(Metadata *imageMetadata) +void AgcChannel::fetchCurrentExposure(DeviceStatus const &deviceStatus) { - std::unique_lock lock(*imageMetadata); - DeviceStatus *deviceStatus = - imageMetadata->getLocked("device.status"); - if (!deviceStatus) - LOG(RPiAgc, Fatal) << "No device metadata"; - current_.shutter = deviceStatus->shutterSpeed; - current_.analogueGain = deviceStatus->analogueGain; - AgcStatus *agcStatus = - imageMetadata->getLocked("agc.status"); - current_.totalExposure = agcStatus ? agcStatus->totalExposureValue : 0s; + current_.shutter = deviceStatus.shutterSpeed; + current_.analogueGain = deviceStatus.analogueGain; + current_.totalExposure = 0s; /* this value is unused */ current_.totalExposureNoDG = current_.shutter * current_.analogueGain; } diff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h index aca79ef2..b2f554db 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.h +++ b/src/ipa/rpi/controller/rpi/agc_channel.h @@ -85,13 +85,13 @@ public: void disableAuto(); void switchMode(CameraMode const &cameraMode, Metadata *metadata); void prepare(Metadata *imageMetadata); - void process(StatisticsPtr &stats, Metadata *imageMetadata); + void process(StatisticsPtr &stats, DeviceStatus const &deviceStatus, Metadata *imageMetadata); private: bool updateLockStatus(DeviceStatus const &deviceStatus); AgcConfig config_; void housekeepConfig(); - void fetchCurrentExposure(Metadata *imageMetadata); + void fetchCurrentExposure(DeviceStatus const &deviceStatus); void fetchAwbStatus(Metadata *imageMetadata); void computeGain(StatisticsPtr &statistics, Metadata *imageMetadata, double &gain, double &targetY);