[libcamera-devel] [PATCH v3 04/13] ipa: rkisp1: Add support for manual gain and exposure
Kieran Bingham
kieran.bingham at ideasonboard.com
Sat Oct 29 00:02:58 CEST 2022
Quoting Laurent Pinchart via libcamera-devel (2022-10-24 01:03:47)
> From: Paul Elder <paul.elder at ideasonboard.com>
>
> Add support for manual gain and exposure in the rkisp1 IPA.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> include/libcamera/ipa/rkisp1.mojom | 2 +-
> src/ipa/rkisp1/algorithms/agc.cpp | 62 +++++++++++++++++++++---
> src/ipa/rkisp1/algorithms/agc.h | 4 ++
> src/ipa/rkisp1/ipa_context.h | 13 ++++-
> src/ipa/rkisp1/rkisp1.cpp | 17 +++++++
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> 6 files changed, 89 insertions(+), 11 deletions(-)
>
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index eaf3955e4096..b526450d9949 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -10,7 +10,7 @@ import "include/libcamera/ipa/core.mojom";
>
> interface IPARkISP1Interface {
> init(libcamera.IPASettings settings,
> - uint32 hwRevision)
> + uint32 hwRevision, libcamera.ControlInfoMap sensorControls)
> => (int32 ret, libcamera.ControlInfoMap ipaControls);
> start() => (int32 ret);
> stop();
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 169afe372803..377a3e8a0efd 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -74,9 +74,14 @@ Agc::Agc()
> int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
> {
> /* Configure the default exposure and gain. */
> - context.activeState.agc.gain = std::max(context.configuration.sensor.minAnalogueGain,
> - kMinAnalogueGain);
> - context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration;
> + context.activeState.agc.automatic.gain =
> + std::max(context.configuration.sensor.minAnalogueGain,
> + kMinAnalogueGain);
> + context.activeState.agc.automatic.exposure =
> + 10ms / context.configuration.sensor.lineDuration;
> + context.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;
> + context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;
> + context.activeState.agc.autoEnabled = true;
>
> /*
> * According to the RkISP1 documentation:
> @@ -108,14 +113,57 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
> return 0;
> }
>
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Agc::queueRequest(IPAContext &context,
> + [[maybe_unused]] const uint32_t frame,
> + IPAFrameContext &frameContext,
> + const ControlList &controls)
> +{
> + auto &agc = context.activeState.agc;
> +
> + const auto &agcEnable = controls.get(controls::AeEnable);
> + if (agcEnable && *agcEnable != agc.autoEnabled) {
> + agc.autoEnabled = *agcEnable;
> +
> + LOG(RkISP1Agc, Debug)
> + << (*agcEnable ? "Enabling" : "Disabling") << " AGC";
> + }
> +
> + const auto &exposure = controls.get(controls::ExposureTime);
> + if (exposure && !agc.autoEnabled) {
> + agc.manual.exposure = *exposure;
> +
> + LOG(RkISP1Agc, Debug)
> + << "Set exposure to: " << agc.manual.exposure;
> + }
> +
> + const auto &gain = controls.get(controls::AnalogueGain);
> + if (gain && !agc.autoEnabled) {
> + agc.manual.gain = *gain;
> +
> + LOG(RkISP1Agc, Debug) << "Set gain to: " << agc.manual.gain;
> + }
> +
> + frameContext.agc.autoEnabled = agc.autoEnabled;
> +
> + if (!frameContext.agc.autoEnabled) {
> + frameContext.agc.exposure = agc.manual.exposure;
> + frameContext.agc.gain = agc.manual.gain;
> + }
> +}
> +
> /**
> * \copydoc libcamera::ipa::Algorithm::prepare
> */
> void Agc::prepare(IPAContext &context, const uint32_t frame,
> IPAFrameContext &frameContext, rkisp1_params_cfg *params)
> {
> - frameContext.agc.exposure = context.activeState.agc.exposure;
> - frameContext.agc.gain = context.activeState.agc.gain;
> + if (frameContext.agc.autoEnabled) {
> + frameContext.agc.exposure = context.activeState.agc.automatic.exposure;
> + frameContext.agc.gain = context.activeState.agc.automatic.gain;
> + }
>
> if (frame > 0)
> return;
> @@ -263,8 +311,8 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
> << stepGain;
>
> /* Update the estimated exposure and gain. */
> - activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
> - activeState.agc.gain = stepGain;
> + activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
> + activeState.agc.automatic.gain = stepGain;
> }
>
> /**
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index da4d2d4e8359..a228d0c37768 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -26,6 +26,10 @@ public:
> ~Agc() = default;
>
> int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
> + void queueRequest(IPAContext &context,
> + const uint32_t frame,
> + IPAFrameContext &frameContext,
> + const ControlList &controls) override;
> void prepare(IPAContext &context, const uint32_t frame,
> IPAFrameContext &frameContext,
> rkisp1_params_cfg *params) override;
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 3e47ac663c58..b9b2065328d6 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -54,8 +54,16 @@ struct IPASessionConfiguration {
>
> struct IPAActiveState {
> struct {
> - uint32_t exposure;
> - double gain;
> + struct {
> + uint32_t exposure;
> + double gain;
> + } manual;
> + struct {
> + uint32_t exposure;
> + double gain;
> + } automatic;
> +
> + bool autoEnabled;
> } agc;
>
> struct {
> @@ -96,6 +104,7 @@ struct IPAFrameContext : public FrameContext {
> struct {
> uint32_t exposure;
> double gain;
> + bool autoEnabled;
> } agc;
>
> struct {
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 538e42cb6f7f..5c041ded0db4 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -50,6 +50,7 @@ public:
> IPARkISP1();
>
> int init(const IPASettings &settings, unsigned int hwRevision,
> + const ControlInfoMap &sensorControls,
> ControlInfoMap *ipaControls) override;
> int start() override;
> void stop() override;
> @@ -116,6 +117,7 @@ std::string IPARkISP1::logPrefix() const
> }
>
> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
> + const ControlInfoMap &sensorControls,
> ControlInfoMap *ipaControls)
> {
> /* \todo Add support for other revisions */
> @@ -184,6 +186,21 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
>
> /* Return the controls handled by the IPA. */
> ControlInfoMap::Map ctrlMap = rkisp1Controls;
> +
> + auto exposureTimeInfo = sensorControls.find(V4L2_CID_EXPOSURE);
> + if (exposureTimeInfo != sensorControls.end()) {
> + ctrlMap.emplace(&controls::ExposureTime,
> + ControlInfo(exposureTimeInfo->second.min(),
> + exposureTimeInfo->second.max()));
> + }
> +
> + auto analogueGainInfo = sensorControls.find(V4L2_CID_ANALOGUE_GAIN);
> + if (analogueGainInfo != sensorControls.end()) {
> + ctrlMap.emplace(&controls::AnalogueGain,
> + ControlInfo(static_cast<float>(analogueGainInfo->second.min().get<int32_t>()),
> + static_cast<float>(analogueGainInfo->second.max().get<int32_t>())));
> + }
It's likely a yak - but this is another area I've been thinking about
lately.
We're deciding what controls are supported by the algorithms at the top
level, and I think this is probably something we should push down to
each algorithm to report back. (and do any validation they require).
Yet Another Call into each algorithm for init() though?
But then we could push the algorith to validate it has the information
it needs from the sensor (and fail early otherwise) and then the ctrlMap
would only get populated with controls that are supported by the
algorithms.
I.e. - if (for some reason) AGC is disabled, no AGC controls would get
registered.
> +
> *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
>
> return 0;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 455ee2a0a711..1418dc9a47fb 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -348,7 +348,7 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
> }
>
> int ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> - &controlInfo_);
> + sensor_->controls(), &controlInfo_);
> if (ret < 0) {
> LOG(RkISP1, Error) << "IPA initialization failure";
> return ret;
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list