[libcamera-devel] [PATCH v3 04/13] ipa: rkisp1: Add support for manual gain and exposure
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Nov 23 10:39:54 CET 2022
Hi Paul,
Thank you for the patch.
On Mon, Oct 24, 2022 at 03:03:47AM +0300, Laurent Pinchart via libcamera-devel wrote:
> 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;
s/://
and below too.
> + }
> +
> + 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()) {
No need for curly braces.
> + ctrlMap.emplace(&controls::ExposureTime,
> + ControlInfo(exposureTimeInfo->second.min(),
> + exposureTimeInfo->second.max()));
To really benefit from emplace, you should write
ctrlMap.emplace(std::piecewise_construct,
std::forward_as_tuple(&controls::ExposureTime),
std::forward_as_tuple(exposureTimeInfo->second.min(),
exposureTimeInfo->second.max()));
to avoid the construction of a temporary ControlInfo.
The units are not right for this control, you need to convert it from
V4L2 units (lines) to libcamera units (time). It may be easier to handle
this on top of "[PATCH 4/4] ipa: rkisp1: add FrameDurationLimits
control" which I hope we'll merge quickly.
This applies to the AnalogueGain control below.
I can handle these issues in v4.
> + }
> +
> + 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>())));
> + }
> +
> *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