[libcamera-devel] [PATCH 3/5] ipa: rpi: agc: Implementation of multi-channel AGC
Naushir Patuck
naush at raspberrypi.com
Tue Aug 22 15:15:48 CEST 2023
Hi David,
Thank you for this patch.
On Mon, 31 Jul 2023 at 10:46, David Plowman via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> 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 <david.plowman at raspberrypi.com>
Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> 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<unsigned int> &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<RPiController::Metadata> lock(*metadata);
> + AgcStatus *status = metadata->getLocked<AgcStatus>("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<RPiController::Metadata> lock(*metadata);
> + AgcStatus *status = metadata->getLocked<AgcStatus>("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<DeviceStatus>("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<AgcChannelData> channelData_;
> std::vector<unsigned int> 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<Metadata> lock(*imageMetadata);
> - DeviceStatus *deviceStatus =
> - imageMetadata->getLocked<DeviceStatus>("device.status");
> if (!deviceStatus)
> LOG(RPiAgc, Fatal) << "No device metadata";
> current_.shutter = deviceStatus->shutterSpeed;
> current_.analogueGain = deviceStatus->analogueGain;
> - AgcStatus *agcStatus =
> - imageMetadata->getLocked<AgcStatus>("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);
> --
> 2.30.2
>
More information about the libcamera-devel
mailing list