[libcamera-devel] [PATCH 1/2] ipa: rpi: Add an HDR algorithm to drive multi-channel AGC
David Plowman
david.plowman at raspberrypi.com
Mon Aug 21 11:09:16 CEST 2023
Hi Naush
Thanks for the review.
On Mon, 21 Aug 2023 at 09:56, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Hi David,
>
> Thank you for this patch.
> Only minor comments below.
>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
>
> On Mon, 31 Jul 2023 at 14:48, David Plowman via libcamera-devel
> <libcamera-devel at lists.libcamera.org> 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>
> > ---
> > 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 | 129 +++++++++++++++++++++++++
> > src/ipa/rpi/controller/rpi/hdr.h | 42 ++++++++
> > 6 files changed, 269 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 f29c32fd..956b1ca0 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"
> > @@ -63,6 +65,7 @@ const ControlInfoMap::Map ipaControls{
> > { &controls::ExposureValue, ControlInfo(-8.0f, 8.0f, 0.0f) },
> > { &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)) },
> > @@ -651,9 +654,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 = {
>
> No need for static here, it's in the anonymous namespace. Having said that,
> perhaps we want to move HdrModeTable down further in the file where the other
> libcamera controls -> RPi strings happen?
Will do!
>
> > + { 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();
> > @@ -1092,6 +1103,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()
> > @@ -1239,6 +1278,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..26b538a0
> > --- /dev/null
> > +++ b/src/ipa/rpi/controller/hdr_status.h
> > @@ -0,0 +1,25 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2022 Raspberry Pi Ltd
>
> s/2022/2023/
Ah yes, of course!
Thanks
David
>
> > + *
> > + * 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;
> > +};
> > 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..2587f622
> > --- /dev/null
> > +++ b/src/ipa/rpi/controller/rpi/hdr.cpp
> > @@ -0,0 +1,129 @@
> > +/* 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)
> > +{
> > + for (const auto &[key, value] : params.asDict()) {
> > + HdrConfig hdrConfig;
> > + hdrConfig.read(value, key);
> > + config_[key] = std::move(hdrConfig);
> > + if (!currentConfig_)
> > + currentConfig_ = &config_[key];
> > + }
> > +
> > + if (!currentConfig_)
> > + LOG(RPiHdr, Fatal) << "No HDR modes read";
> > +
> > + 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;
> > + }
> > +
> > + 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
> > + *
> > + * hdr.h - HDR control algorithm
> > + */
> > +#pragma once
> > +
> > +#include <vector>
> > +
> > +#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