[libcamera-devel] [PATCH v2 1/2] ipa: rpi: Add an HDR algorithm to drive multi-channel AGC
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Sat Sep 16 15:25:15 CEST 2023
Hi David
On Wed, Aug 23, 2023 at 03:49:24PM +0100, David Plowman via libcamera-devel wrote:
> This HDR algorithm doesn't combine images in any way, but simply
> allows an application to drive the AGC in a multi-channel HDR type of
> mode, such as to produce short and long exposure images.
>
> Sufficient plumbing is added to enable the libcamera HDR controls and
> metadata to work.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> src/ipa/rpi/common/ipa_base.cpp | 49 ++++++++++
> src/ipa/rpi/controller/hdr_algorithm.h | 23 +++++
> src/ipa/rpi/controller/hdr_status.h | 25 +++++
> src/ipa/rpi/controller/meson.build | 1 +
> src/ipa/rpi/controller/rpi/hdr.cpp | 127 +++++++++++++++++++++++++
> src/ipa/rpi/controller/rpi/hdr.h | 42 ++++++++
> 6 files changed, 267 insertions(+)
> create mode 100644 src/ipa/rpi/controller/hdr_algorithm.h
> create mode 100644 src/ipa/rpi/controller/hdr_status.h
> create mode 100644 src/ipa/rpi/controller/rpi/hdr.cpp
> create mode 100644 src/ipa/rpi/controller/rpi/hdr.h
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index f7e7ad5e..231e8f96 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -24,6 +24,8 @@
> #include "controller/ccm_status.h"
> #include "controller/contrast_algorithm.h"
> #include "controller/denoise_algorithm.h"
> +#include "controller/hdr_algorithm.h"
> +#include "controller/hdr_status.h"
> #include "controller/lux_status.h"
> #include "controller/sharpen_algorithm.h"
> #include "controller/statistics.h"
> @@ -67,6 +69,7 @@ const ControlInfoMap::Map ipaControls{
> { &controls::AeFlickerPeriod, ControlInfo(100, 1000000) },
> { &controls::Brightness, ControlInfo(-1.0f, 1.0f, 0.0f) },
> { &controls::Contrast, ControlInfo(0.0f, 32.0f, 1.0f) },
> + { &controls::HdrMode, ControlInfo(controls::HdrModeValues) },
> { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
> { &controls::FrameDurationLimits, ControlInfo(INT64_C(33333), INT64_C(120000)) },
> @@ -658,9 +661,17 @@ static const std::map<int32_t, RPiController::AfAlgorithm::AfPause> AfPauseTable
> { controls::AfPauseResume, RPiController::AfAlgorithm::AfPauseResume },
> };
>
> +static const std::map<int32_t, std::string> HdrModeTable = {
> + { controls::HdrModeOff, "Off" },
> + { controls::HdrModeMultiExposure, "MultiExposure" },
> + { controls::HdrModeSingleExposure, "SingleExposure" },
> +};
> +
> void IpaBase::applyControls(const ControlList &controls)
> {
> + using RPiController::AgcAlgorithm;
> using RPiController::AfAlgorithm;
> + using RPiController::HdrAlgorithm;
>
> /* Clear the return metadata buffer. */
> libcameraMetadata_.clear();
> @@ -1157,6 +1168,34 @@ void IpaBase::applyControls(const ControlList &controls)
> break;
> }
>
> + case controls::HDR_MODE: {
> + HdrAlgorithm *hdr = dynamic_cast<HdrAlgorithm *>(controller_.getAlgorithm("hdr"));
> + if (!hdr) {
> + LOG(IPARPI, Warning) << "No HDR algorithm available";
> + break;
> + }
> +
> + auto mode = HdrModeTable.find(ctrl.second.get<int32_t>());
> + if (mode == HdrModeTable.end()) {
> + LOG(IPARPI, Warning) << "Unrecognised HDR mode";
> + break;
> + }
> +
> + AgcAlgorithm *agc = dynamic_cast<AgcAlgorithm *>(controller_.getAlgorithm("agc"));
> + if (!agc) {
> + LOG(IPARPI, Warning) << "HDR requires an AGC algorithm";
> + break;
> + }
> +
> + if (hdr->setMode(mode->second) == 0)
> + agc->setActiveChannels(hdr->getChannels());
> + else
> + LOG(IPARPI, Warning)
> + << "HDR mode " << mode->second << " not supported";
> +
> + break;
> + }
> +
> default:
> LOG(IPARPI, Warning)
> << "Ctrl " << controls::controls.at(ctrl.first)->name()
> @@ -1304,6 +1343,16 @@ void IpaBase::reportMetadata(unsigned int ipaContext)
> libcameraMetadata_.set(controls::AfPauseState, p);
> }
>
> + const HdrStatus *hdrStatus = rpiMetadata.getLocked<HdrStatus>("hdr.status");
> + if (hdrStatus) {
> + if (hdrStatus->channel == "short")
> + libcameraMetadata_.set(controls::HdrChannel, controls::HdrChannelShort);
> + else if (hdrStatus->channel == "long")
> + libcameraMetadata_.set(controls::HdrChannel, controls::HdrChannelLong);
> + else
> + libcameraMetadata_.set(controls::HdrChannel, controls::HdrChannelNone);
> + }
> +
> metadataReady.emit(libcameraMetadata_);
> }
>
> diff --git a/src/ipa/rpi/controller/hdr_algorithm.h b/src/ipa/rpi/controller/hdr_algorithm.h
> new file mode 100644
> index 00000000..5164d50e
> --- /dev/null
> +++ b/src/ipa/rpi/controller/hdr_algorithm.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2023, Raspberry Pi Ltd
> + *
> + * hdr_algorithm.h - HDR control algorithm interface
> + */
> +#pragma once
> +
> +#include "algorithm.h"
> +
> +namespace RPiController {
> +
> +class HdrAlgorithm : public Algorithm
> +{
> +public:
> + HdrAlgorithm(Controller *controller)
> + : Algorithm(controller) {}
> + /* An HDR algorithm must provide the following: */
> + virtual int setMode(std::string const &modeName) = 0;
> + virtual std::vector<unsigned int> getChannels() const = 0;
> +};
> +
> +} /* namespace RPiController */
> diff --git a/src/ipa/rpi/controller/hdr_status.h b/src/ipa/rpi/controller/hdr_status.h
> new file mode 100644
> index 00000000..120dcb1a
> --- /dev/null
> +++ b/src/ipa/rpi/controller/hdr_status.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2023 Raspberry Pi Ltd
> + *
> + * hdr_status.h - HDR control algorithm status
> + */
> +#pragma once
> +
> +#include <string>
> +
> +/*
> + * The HDR algorithm process method should post an HdrStatus into the image
> + * metadata under the tag "hdr.status".
> + */
> +
> +struct HdrStatus {
> + std::string mode;
> + std::string channel;
> +
> + /* Parameters for programming the stitch block (where available). */
> + bool stitch_enable;
> + uint16_t thresholdLo;
> + uint8_t diffPower;
> + double motionThreshold;
I'll let you consider if it's worth adding them now or later.
> +};
> diff --git a/src/ipa/rpi/controller/meson.build b/src/ipa/rpi/controller/meson.build
> index 20b9cda9..c9a3e850 100644
> --- a/src/ipa/rpi/controller/meson.build
> +++ b/src/ipa/rpi/controller/meson.build
> @@ -16,6 +16,7 @@ rpi_ipa_controller_sources = files([
> 'rpi/contrast.cpp',
> 'rpi/dpc.cpp',
> 'rpi/geq.cpp',
> + 'rpi/hdr.cpp',
> 'rpi/lux.cpp',
> 'rpi/noise.cpp',
> 'rpi/sdn.cpp',
> diff --git a/src/ipa/rpi/controller/rpi/hdr.cpp b/src/ipa/rpi/controller/rpi/hdr.cpp
> new file mode 100644
> index 00000000..2700d35e
> --- /dev/null
> +++ b/src/ipa/rpi/controller/rpi/hdr.cpp
> @@ -0,0 +1,127 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2022 Raspberry Pi Ltd
> + *
> + * hdr.cpp - HDR control algorithm
> + */
> +
> +#include "hdr.h"
> +
> +#include <libcamera/base/log.h>
> +
> +#include "../agc_status.h"
> +
> +using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiHdr)
> +
> +#define NAME "rpi.hdr"
> +
> +void HdrConfig::read(const libcamera::YamlObject ¶ms, const std::string &modeName)
> +{
> + name = modeName;
> +
> + if (!params.contains("cadence"))
> + LOG(RPiHdr, Fatal) << "No cadence for HDR mode " << name;
> + cadence = params["cadence"].getList<unsigned int>().value();
> + if (cadence.empty())
> + LOG(RPiHdr, Fatal) << "Empty cadence in HDR mode " << name;
> +
> + /*
> + * In the JSON file it's easier to use the channel name as the key, but
> + * for us it's convenient to swap them over.
> + */
> + for (const auto &[k, v] : params["channel_map"].asDict())
> + channelMap[v.get<unsigned int>().value()] = k;
> +}
> +
> +Hdr::Hdr(Controller *controller)
> + : HdrAlgorithm(controller),
> + currentConfig_(nullptr)
> +{
> +}
> +
> +char const *Hdr::name() const
> +{
> + return NAME;
> +}
> +
> +int Hdr::read(const libcamera::YamlObject ¶ms)
> +{
> + /* Make an "HDR off" mode by default so that tuning files don't have to. */
> + HdrConfig &offMode = config_["Off"];
> + offMode.name = "Off";
> + offMode.cadence = { 0 };
Isn't "Off" part of the config file ?
"Off":
{
"cadence": [ 0 ]
},
> + currentConfig_ = &offMode;
I understand this should be the default
> +
> + for (const auto &[key, value] : params.asDict())
> + config_[key].read(value, key);
But won't "Off" be parsed here too ?
> +
> + return 0;
> +}
> +
> +int Hdr::setMode(std::string const &mode)
> +{
> + auto it = config_.find(mode);
> + if (it == config_.end()) {
> + LOG(RPiHdr, Warning) << "No such HDR mode " << mode;
> + return -1;
> + }
> +
> + currentConfig_ = &it->second;
> +
> + return 0;
> +}
> +
> +std::vector<unsigned int> Hdr::getChannels() const
> +{
> + return currentConfig_->cadence;
> +}
> +
> +void Hdr::switchMode([[maybe_unused]] CameraMode const &cameraMode,
> + [[maybe_unused]] Metadata *metadata)
> +{
> +}
> +
> +void Hdr::process([[maybe_unused]] StatisticsPtr &stats, Metadata *imageMetadata)
> +{
> + if (currentConfig_->name == "Off")
> + return;
> +
> + /* First find out what AGC channel this is, which comes from the delayed_status. */
> + bool channelKnown = false;
> + unsigned int agcChannel = 0;
> + {
> + std::scoped_lock<RPiController::Metadata> lock(*imageMetadata);
> + AgcStatus *agcStatus = imageMetadata->getLocked<AgcStatus>("agc.delayed_status");
> + if (agcStatus) {
> + channelKnown = true;
> + agcChannel = agcStatus->channel;
> + }
> + }
> +
> + /* Fill out the HdrStatus information. */
> + HdrStatus hdrStatus;
> + hdrStatus.mode = currentConfig_->name;
> + if (channelKnown) {
> + /*
> + * Channels that aren't required for HDR processing might come through for
> + * various reasons, such as when HDR starts up, or even because the cadence
> + * demands some frames that you need for other purposes (e.g. for preview).
> + * We'll leave the channel name empty in these cases.
> + */
> + auto it = currentConfig_->channelMap.find(agcChannel);
> + if (it != currentConfig_->channelMap.end())
> + hdrStatus.channel = it->second;
Related to the discussion about the "HdrChannel" metadata, it would be
trivial returning the channel index here if I got this right
> + }
> +
> + imageMetadata->set("hdr.status", hdrStatus);
> +}
> +
> +/* Register algorithm with the system. */
> +static Algorithm *create(Controller *controller)
> +{
> + return (Algorithm *)new Hdr(controller);
> +}
> +static RegisterAlgorithm reg(NAME, &create);
> diff --git a/src/ipa/rpi/controller/rpi/hdr.h b/src/ipa/rpi/controller/rpi/hdr.h
> new file mode 100644
> index 00000000..8f6457f2
> --- /dev/null
> +++ b/src/ipa/rpi/controller/rpi/hdr.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2022, Raspberry Pi Ltd
All of these are 2023 now
> + *
> + * hdr.h - HDR control algorithm
> + */
> +#pragma once
> +
> +#include <vector>
and string and map ?
> +
> +#include "../hdr_algorithm.h"
> +#include "../hdr_status.h"
> +
> +/* This is our implementation of an HDR algorithm. */
> +
> +namespace RPiController {
> +
> +struct HdrConfig {
> + std::string name;
> + std::vector<unsigned int> cadence;
> + std::map<unsigned int, std::string> channelMap;
> +
> + void read(const libcamera::YamlObject ¶ms, const std::string &name);
> +};
> +
> +class Hdr : public HdrAlgorithm
> +{
> +public:
> + Hdr(Controller *controller);
> + char const *name() const override;
> + void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
> + int read(const libcamera::YamlObject ¶ms) override;
> + void process(StatisticsPtr &stats, Metadata *imageMetadata) override;
> + int setMode(std::string const &mode) override;
> + std::vector<unsigned int> getChannels() const override;
> +
> +private:
> + std::map<std::string, HdrConfig> config_;
> + const HdrConfig *currentConfig_;
> +};
> +
> +} /* namespace RPiController */
> --
> 2.30.2
>
More information about the libcamera-devel
mailing list