From patchwork Mon Jul 31 09:46:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 18906 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 35C80C32A9 for ; Mon, 31 Jul 2023 09:46:51 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3ABA3627F9; Mon, 31 Jul 2023 11:46:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1690796810; bh=ZJSi6bOkfN3TjdsBu0GV55WnZa2Ci/74JphlKiKVBE8=; 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=ucl7xIw6WbA4lv+qq70QyRsqaWVX70GqXLckNFIygMTPweT1Brnq2GXMuhsMnuks8 0iF9YAY79tM8PN0Ko9M29ZlabCX6g4xEkAVjDwAZg03U5JqgBsu+VBbPglxVsmKGP4 Y0tySKUXo22RIRhoQfgwYxcpzYsj9nIhfrqu9bGSgEi67InlqILckR9eLhsVIf3UCw QaxELqHcegfkkwBfLBPZXDXgPmATzS0tav3KuYK9Dds9LzOHHMJ+8bjyAyBmHC6QOV /vSCVfEOU4+OvAJP8yGc9hR6v1zSwP0s0jVRF/+VJrAV3f7fVyAKzQGDfhWGWhbCvi d/UbfJ+SSdX9A== Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 27202627EF for ; Mon, 31 Jul 2023 11:46:47 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="IEL0U3VN"; dkim-atps=neutral Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-3fbf1b82d9cso39674235e9.2 for ; Mon, 31 Jul 2023 02:46:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1690796806; x=1691401606; 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=8+ExL/Z4e7Z72TgXvZCoUG9fhEbBhbxBscCQO3R9Tko=; b=IEL0U3VNPzDWQPvIBFOA7kUPF57l6JsX5zIxmVsCFM49SYdgbjoLl9s51JlDbUiQD9 LjXTIVTh49ZmtGdpziumdFYGoMipNUaKosXoyzMSdDRLf4HupzyljAvEyo2uTt7F9TTH mDyru3vXZejNqisXzmsNbbwlG3BKSPveK8ME6z/um/fUuqS4kH2qT3IJfJ8Tfmz2YyqP RLhcFq2vur5VR1n9ryxQC7ugvUUFpl5MuRq+IKRkE2dRSyedT0eL3MjNzEs1J7q2bL00 VSjhmK9y+9h1VPz7QdNbpEuOcmNL0o+K+IYRtZV6M7pxwH7wmcvenIlvBuln+Dbj7/8W mODg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690796806; x=1691401606; 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=8+ExL/Z4e7Z72TgXvZCoUG9fhEbBhbxBscCQO3R9Tko=; b=i6RF+A1MiqxCXjPbPUyIzDgh4Cd+s36ZBj4Vmy5Nwe7DquxT6e+SOg5Vp1DzXIyzs9 bX6xpysATe92swx7fe4JsYEztOIcTjMZ/VTmW0ALDSGFwTGZurA/be/jMm2p5i7jjYfX k24NcN+daDEe9TNcZj0MdVvFqQXGeHP9/R9L4sBxgnsRL7RH9WEqpqKZwAyzrtGQVSwy FwRXWGJ1i1ne+CXslA7tAfv4qT/EBY/4mLXtSctxa5UuOnLyLXGiCjNSPR41KhoagnEm K1Sj0ml/9pKAlOL+8bBITmclaVF1xlmjyw7qDueA5NJTPbwV2Lo1JPEcQr2gA0lp6K+b 6bJg== X-Gm-Message-State: ABy/qLYc+AyPy6S3Yq78Z7YNNmZPg/kUs2CGDkertMJQK9pWa3k9H1G0 +24CB06MySeivtr6nqi/KNGaw/pfqJhNMx7i5K0= X-Google-Smtp-Source: APBJJlH/b5FGCEMCeeeQ/5l2eYWjNDJzO8KjUUwWrKwllqx+lVnxEFaP6bljfKoQk0HCoTst3tr7Ww== X-Received: by 2002:a1c:7917:0:b0:3fe:16f4:d865 with SMTP id l23-20020a1c7917000000b003fe16f4d865mr3915666wme.23.1690796806169; Mon, 31 Jul 2023 02:46:46 -0700 (PDT) Received: from pi4-davidp.pitowers.org ([2a00:1098:3142:14:2bce:64d6:1a5c:49a2]) by smtp.gmail.com with ESMTPSA id 9-20020a05600c240900b003fa98908014sm13612838wmp.8.2023.07.31.02.46.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 31 Jul 2023 02:46:45 -0700 (PDT) To: libcamera-devel@lists.libcamera.org Date: Mon, 31 Jul 2023 10:46:39 +0100 Message-Id: <20230731094641.73646-4-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230731094641.73646-1-david.plowman@raspberrypi.com> References: <20230731094641.73646-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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 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 --- src/ipa/rpi/controller/agc_status.h | 1 + src/ipa/rpi/controller/rpi/agc.cpp | 108 +++++++++++++++++++-- src/ipa/rpi/controller/rpi/agc.h | 1 + src/ipa/rpi/controller/rpi/agc_channel.cpp | 13 +-- src/ipa/rpi/controller/rpi/agc_channel.h | 4 +- 5 files changed, 109 insertions(+), 18 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 c9c9c297..7e627bba 100644 --- a/src/ipa/rpi/controller/rpi/agc.cpp +++ b/src/ipa/rpi/controller/rpi/agc.cpp @@ -22,7 +22,8 @@ LOG_DEFINE_CATEGORY(RPiAgc) Agc::Agc(Controller *controller) : AgcAlgorithm(controller), - activeChannels_({ 0 }) + activeChannels_({ 0 }), + index_(0) { } @@ -203,20 +204,113 @@ 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 getChannelIndex(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 setChannelIndex(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]; + getChannelIndex(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 this channel: */ + unsigned int channelIndex = activeChannels_[index_]; + AgcChannelData &channelData = channelData_[channelIndex]; + /* Stats are from this channel: */ + unsigned int statsIndex = 0; + getChannelIndex(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 this 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); + setChannelIndex(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 a9158910..2eed2bab 100644 --- a/src/ipa/rpi/controller/rpi/agc.h +++ b/src/ipa/rpi/controller/rpi/agc.h @@ -53,6 +53,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 d6e30ef2..ddec611f 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp @@ -447,7 +447,7 @@ void AgcChannel::prepare(Metadata *imageMetadata) } } -void AgcChannel::process(StatisticsPtr &stats, Metadata *imageMetadata) +void AgcChannel::process(StatisticsPtr &stats, const DeviceStatus *deviceStatus, Metadata *imageMetadata) { frameCount_++; /* @@ -458,7 +458,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); @@ -570,18 +570,13 @@ void AgcChannel::housekeepConfig() << meteringModeName_; } -void AgcChannel::fetchCurrentExposure(Metadata *imageMetadata) +void AgcChannel::fetchCurrentExposure(const DeviceStatus *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_.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 dc4356f3..0e3d44b0 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.h +++ b/src/ipa/rpi/controller/rpi/agc_channel.h @@ -83,13 +83,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, const DeviceStatus *deviceStatus, Metadata *imageMetadata); private: bool updateLockStatus(DeviceStatus const &deviceStatus); AgcConfig config_; void housekeepConfig(); - void fetchCurrentExposure(Metadata *imageMetadata); + void fetchCurrentExposure(const DeviceStatus *deviceStatus); void fetchAwbStatus(Metadata *imageMetadata); void computeGain(StatisticsPtr &statistics, Metadata *imageMetadata, double &gain, double &targetY);