[v2,02/10] ipa: mali-c55: Move CameraHelper to context
diff mbox series

Message ID 20251028-exposure-limits-v2-2-a8b5a318323e@ideasonboard.com
State New
Headers show
Series
  • libipa: agc: Calculate exposure limits
Related show

Commit Message

Jacopo Mondi Oct. 28, 2025, 9:31 a.m. UTC 
From: Kieran Bingham <kieran.bingham@ideasonboard.com>

Move the CameraHelper sensor to the Context so that it can be
used by the algorithms directly.

While at it, document the undocumented ctrlMap member of IPAContext.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/mali-c55/ipa_context.cpp |  6 ++++++
 src/ipa/mali-c55/ipa_context.h   |  4 ++++
 src/ipa/mali-c55/mali-c55.cpp    | 27 ++++++++++++---------------
 3 files changed, 22 insertions(+), 15 deletions(-)

Comments

Laurent Pinchart Nov. 2, 2025, 7:52 p.m. UTC | #1 
On Tue, Oct 28, 2025 at 10:31:48AM +0100, Jacopo Mondi wrote:
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Move the CameraHelper sensor to the Context so that it can be
> used by the algorithms directly.
> 
> While at it, document the undocumented ctrlMap member of IPAContext.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/ipa/mali-c55/ipa_context.cpp |  6 ++++++
>  src/ipa/mali-c55/ipa_context.h   |  4 ++++
>  src/ipa/mali-c55/mali-c55.cpp    | 27 ++++++++++++---------------
>  3 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp
> index 1b203e2b260592cda945855d54f1aceefaaece91..a1e4c39f38704d3a4b5948ab789ffb9773f216c9 100644
> --- a/src/ipa/mali-c55/ipa_context.cpp
> +++ b/src/ipa/mali-c55/ipa_context.cpp
> @@ -96,6 +96,12 @@ namespace libcamera::ipa::mali_c55 {
>   *
>   * \var IPAContext::frameContexts
>   * \brief Ring buffer of per-frame contexts
> + *
> + * \var IPAContext::ctrlMap
> + * \brief A ControlInfoMap::Map of controls populated by the algorithms
> + *
> + * \var IPAContext::camHelper
> + * \brief The camera sensor helper
>   */
>  
>  } /* namespace libcamera::ipa::mali_c55 */
> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> index 13885eb83b5c4d7c7af6c4f8d5673890834fed58..bfa805c7b93f313dda2497365e83542bbc39e291 100644
> --- a/src/ipa/mali-c55/ipa_context.h
> +++ b/src/ipa/mali-c55/ipa_context.h
> @@ -12,6 +12,7 @@
>  
>  #include "libcamera/internal/bayer_format.h"
>  
> +#include <libipa/camera_sensor_helper.h>
>  #include <libipa/fc_queue.h>
>  
>  namespace libcamera {
> @@ -83,6 +84,9 @@ struct IPAContext {
>  	FCQueue<IPAFrameContext> frameContexts;
>  
>  	ControlInfoMap::Map ctrlMap;
> +
> +	/* Interface to the Camera Helper */
> +	std::unique_ptr<CameraSensorHelper> camHelper;
>  };
>  
>  } /* namespace ipa::mali_c55 */
> diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
> index 7d45e7310aecdae0e47655e6d2e8830e776d74cd..0751513dc584ca84dd212bf8c1469dd4b40c053d 100644
> --- a/src/ipa/mali-c55/mali-c55.cpp
> +++ b/src/ipa/mali-c55/mali-c55.cpp
> @@ -74,9 +74,6 @@ private:
>  
>  	ControlInfoMap sensorControls_;
>  
> -	/* Interface to the Camera Helper */
> -	std::unique_ptr<CameraSensorHelper> camHelper_;
> -
>  	/* Local parameter storage */
>  	struct IPAContext context_;
>  };
> @@ -98,8 +95,8 @@ std::string IPAMaliC55::logPrefix() const
>  int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
>  		     ControlInfoMap *ipaControls)
>  {
> -	camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> -	if (!camHelper_) {
> +	context_.camHelper = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> +	if (!context_.camHelper) {
>  		LOG(IPAMaliC55, Error)
>  			<< "Failed to create camera sensor helper for "
>  			<< settings.sensorModel;
> @@ -142,10 +139,10 @@ void IPAMaliC55::setControls()
>  
>  	if (activeState.agc.autoEnabled) {
>  		exposure = activeState.agc.automatic.exposure;
> -		gain = camHelper_->gainCode(activeState.agc.automatic.sensorGain);
> +		gain = context_.camHelper->gainCode(activeState.agc.automatic.sensorGain);
>  	} else {
>  		exposure = activeState.agc.manual.exposure;
> -		gain = camHelper_->gainCode(activeState.agc.manual.sensorGain);
> +		gain = context_.camHelper->gainCode(activeState.agc.manual.sensorGain);
>  	}
>  
>  	ControlList ctrls(sensorControls_);
> @@ -191,17 +188,17 @@ void IPAMaliC55::updateSessionConfiguration(const IPACameraSensorInfo &info,
>  	context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
>  	context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
>  	context_.configuration.agc.defaultExposure = defExposure;
> -	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
> -	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
> +	context_.configuration.agc.minAnalogueGain = context_.camHelper->gain(minGain);
> +	context_.configuration.agc.maxAnalogueGain = context_.camHelper->gain(maxGain);
>  
> -	if (camHelper_->blackLevel().has_value()) {
> +	if (context_.camHelper->blackLevel().has_value()) {
>  		/*
>  		 * The black level from CameraSensorHelper is a 16-bit value.
>  		 * The Mali-C55 ISP expects 20-bit settings, so we shift it to
>  		 * the appropriate width
>  		 */
>  		context_.configuration.sensor.blackLevel =
> -			camHelper_->blackLevel().value() << 4;
> +			context_.camHelper->blackLevel().value() << 4;
>  	}
>  }
>  
> @@ -252,9 +249,9 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,
>  
>  	/* Compute the analogue gain limits. */
>  	const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
> -	float minGain = camHelper_->gain(v4l2Gain.min().get<int32_t>());
> -	float maxGain = camHelper_->gain(v4l2Gain.max().get<int32_t>());
> -	float defGain = camHelper_->gain(v4l2Gain.def().get<int32_t>());
> +	float minGain = context_.camHelper->gain(v4l2Gain.min().get<int32_t>());
> +	float maxGain = context_.camHelper->gain(v4l2Gain.max().get<int32_t>());
> +	float defGain = context_.camHelper->gain(v4l2Gain.def().get<int32_t>());
>  	ctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);
>  
>  	/*
> @@ -362,7 +359,7 @@ void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
>  	frameContext.agc.exposure =
>  		sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>  	frameContext.agc.sensorGain =
> -		camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> +		context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>  
>  	ControlList metadata(controls::controls);
>
Stefan Klug Nov. 5, 2025, 5:53 a.m. UTC | #2 
Hi Jacopo,

Thank you for the patch.

Quoting Jacopo Mondi (2025-10-28 10:31:48)
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Move the CameraHelper sensor to the Context so that it can be
> used by the algorithms directly.
> 
> While at it, document the undocumented ctrlMap member of IPAContext.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

No we had three people doing that, it must be right :-)

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

Regards,
Stefan

> ---
>  src/ipa/mali-c55/ipa_context.cpp |  6 ++++++
>  src/ipa/mali-c55/ipa_context.h   |  4 ++++
>  src/ipa/mali-c55/mali-c55.cpp    | 27 ++++++++++++---------------
>  3 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp
> index 1b203e2b260592cda945855d54f1aceefaaece91..a1e4c39f38704d3a4b5948ab789ffb9773f216c9 100644
> --- a/src/ipa/mali-c55/ipa_context.cpp
> +++ b/src/ipa/mali-c55/ipa_context.cpp
> @@ -96,6 +96,12 @@ namespace libcamera::ipa::mali_c55 {
>   *
>   * \var IPAContext::frameContexts
>   * \brief Ring buffer of per-frame contexts
> + *
> + * \var IPAContext::ctrlMap
> + * \brief A ControlInfoMap::Map of controls populated by the algorithms
> + *
> + * \var IPAContext::camHelper
> + * \brief The camera sensor helper
>   */
>  
>  } /* namespace libcamera::ipa::mali_c55 */
> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> index 13885eb83b5c4d7c7af6c4f8d5673890834fed58..bfa805c7b93f313dda2497365e83542bbc39e291 100644
> --- a/src/ipa/mali-c55/ipa_context.h
> +++ b/src/ipa/mali-c55/ipa_context.h
> @@ -12,6 +12,7 @@
>  
>  #include "libcamera/internal/bayer_format.h"
>  
> +#include <libipa/camera_sensor_helper.h>
>  #include <libipa/fc_queue.h>
>  
>  namespace libcamera {
> @@ -83,6 +84,9 @@ struct IPAContext {
>         FCQueue<IPAFrameContext> frameContexts;
>  
>         ControlInfoMap::Map ctrlMap;
> +
> +       /* Interface to the Camera Helper */
> +       std::unique_ptr<CameraSensorHelper> camHelper;
>  };
>  
>  } /* namespace ipa::mali_c55 */
> diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
> index 7d45e7310aecdae0e47655e6d2e8830e776d74cd..0751513dc584ca84dd212bf8c1469dd4b40c053d 100644
> --- a/src/ipa/mali-c55/mali-c55.cpp
> +++ b/src/ipa/mali-c55/mali-c55.cpp
> @@ -74,9 +74,6 @@ private:
>  
>         ControlInfoMap sensorControls_;
>  
> -       /* Interface to the Camera Helper */
> -       std::unique_ptr<CameraSensorHelper> camHelper_;
> -
>         /* Local parameter storage */
>         struct IPAContext context_;
>  };
> @@ -98,8 +95,8 @@ std::string IPAMaliC55::logPrefix() const
>  int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
>                      ControlInfoMap *ipaControls)
>  {
> -       camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> -       if (!camHelper_) {
> +       context_.camHelper = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> +       if (!context_.camHelper) {
>                 LOG(IPAMaliC55, Error)
>                         << "Failed to create camera sensor helper for "
>                         << settings.sensorModel;
> @@ -142,10 +139,10 @@ void IPAMaliC55::setControls()
>  
>         if (activeState.agc.autoEnabled) {
>                 exposure = activeState.agc.automatic.exposure;
> -               gain = camHelper_->gainCode(activeState.agc.automatic.sensorGain);
> +               gain = context_.camHelper->gainCode(activeState.agc.automatic.sensorGain);
>         } else {
>                 exposure = activeState.agc.manual.exposure;
> -               gain = camHelper_->gainCode(activeState.agc.manual.sensorGain);
> +               gain = context_.camHelper->gainCode(activeState.agc.manual.sensorGain);
>         }
>  
>         ControlList ctrls(sensorControls_);
> @@ -191,17 +188,17 @@ void IPAMaliC55::updateSessionConfiguration(const IPACameraSensorInfo &info,
>         context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
>         context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
>         context_.configuration.agc.defaultExposure = defExposure;
> -       context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
> -       context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
> +       context_.configuration.agc.minAnalogueGain = context_.camHelper->gain(minGain);
> +       context_.configuration.agc.maxAnalogueGain = context_.camHelper->gain(maxGain);
>  
> -       if (camHelper_->blackLevel().has_value()) {
> +       if (context_.camHelper->blackLevel().has_value()) {
>                 /*
>                  * The black level from CameraSensorHelper is a 16-bit value.
>                  * The Mali-C55 ISP expects 20-bit settings, so we shift it to
>                  * the appropriate width
>                  */
>                 context_.configuration.sensor.blackLevel =
> -                       camHelper_->blackLevel().value() << 4;
> +                       context_.camHelper->blackLevel().value() << 4;
>         }
>  }
>  
> @@ -252,9 +249,9 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,
>  
>         /* Compute the analogue gain limits. */
>         const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
> -       float minGain = camHelper_->gain(v4l2Gain.min().get<int32_t>());
> -       float maxGain = camHelper_->gain(v4l2Gain.max().get<int32_t>());
> -       float defGain = camHelper_->gain(v4l2Gain.def().get<int32_t>());
> +       float minGain = context_.camHelper->gain(v4l2Gain.min().get<int32_t>());
> +       float maxGain = context_.camHelper->gain(v4l2Gain.max().get<int32_t>());
> +       float defGain = context_.camHelper->gain(v4l2Gain.def().get<int32_t>());
>         ctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);
>  
>         /*
> @@ -362,7 +359,7 @@ void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
>         frameContext.agc.exposure =
>                 sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>         frameContext.agc.sensorGain =
> -               camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> +               context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>  
>         ControlList metadata(controls::controls);
>  
> 
> -- 
> 2.51.0
>

Patch
diff mbox series 

diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp
index 1b203e2b260592cda945855d54f1aceefaaece91..a1e4c39f38704d3a4b5948ab789ffb9773f216c9 100644
--- a/src/ipa/mali-c55/ipa_context.cpp
+++ b/src/ipa/mali-c55/ipa_context.cpp
@@ -96,6 +96,12 @@  namespace libcamera::ipa::mali_c55 {
  *
  * \var IPAContext::frameContexts
  * \brief Ring buffer of per-frame contexts
+ *
+ * \var IPAContext::ctrlMap
+ * \brief A ControlInfoMap::Map of controls populated by the algorithms
+ *
+ * \var IPAContext::camHelper
+ * \brief The camera sensor helper
  */
 
 } /* namespace libcamera::ipa::mali_c55 */
diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
index 13885eb83b5c4d7c7af6c4f8d5673890834fed58..bfa805c7b93f313dda2497365e83542bbc39e291 100644
--- a/src/ipa/mali-c55/ipa_context.h
+++ b/src/ipa/mali-c55/ipa_context.h
@@ -12,6 +12,7 @@ 
 
 #include "libcamera/internal/bayer_format.h"
 
+#include <libipa/camera_sensor_helper.h>
 #include <libipa/fc_queue.h>
 
 namespace libcamera {
@@ -83,6 +84,9 @@  struct IPAContext {
 	FCQueue<IPAFrameContext> frameContexts;
 
 	ControlInfoMap::Map ctrlMap;
+
+	/* Interface to the Camera Helper */
+	std::unique_ptr<CameraSensorHelper> camHelper;
 };
 
 } /* namespace ipa::mali_c55 */
diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
index 7d45e7310aecdae0e47655e6d2e8830e776d74cd..0751513dc584ca84dd212bf8c1469dd4b40c053d 100644
--- a/src/ipa/mali-c55/mali-c55.cpp
+++ b/src/ipa/mali-c55/mali-c55.cpp
@@ -74,9 +74,6 @@  private:
 
 	ControlInfoMap sensorControls_;
 
-	/* Interface to the Camera Helper */
-	std::unique_ptr<CameraSensorHelper> camHelper_;
-
 	/* Local parameter storage */
 	struct IPAContext context_;
 };
@@ -98,8 +95,8 @@  std::string IPAMaliC55::logPrefix() const
 int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
 		     ControlInfoMap *ipaControls)
 {
-	camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
-	if (!camHelper_) {
+	context_.camHelper = CameraSensorHelperFactoryBase::create(settings.sensorModel);
+	if (!context_.camHelper) {
 		LOG(IPAMaliC55, Error)
 			<< "Failed to create camera sensor helper for "
 			<< settings.sensorModel;
@@ -142,10 +139,10 @@  void IPAMaliC55::setControls()
 
 	if (activeState.agc.autoEnabled) {
 		exposure = activeState.agc.automatic.exposure;
-		gain = camHelper_->gainCode(activeState.agc.automatic.sensorGain);
+		gain = context_.camHelper->gainCode(activeState.agc.automatic.sensorGain);
 	} else {
 		exposure = activeState.agc.manual.exposure;
-		gain = camHelper_->gainCode(activeState.agc.manual.sensorGain);
+		gain = context_.camHelper->gainCode(activeState.agc.manual.sensorGain);
 	}
 
 	ControlList ctrls(sensorControls_);
@@ -191,17 +188,17 @@  void IPAMaliC55::updateSessionConfiguration(const IPACameraSensorInfo &info,
 	context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
 	context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
 	context_.configuration.agc.defaultExposure = defExposure;
-	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
-	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
+	context_.configuration.agc.minAnalogueGain = context_.camHelper->gain(minGain);
+	context_.configuration.agc.maxAnalogueGain = context_.camHelper->gain(maxGain);
 
-	if (camHelper_->blackLevel().has_value()) {
+	if (context_.camHelper->blackLevel().has_value()) {
 		/*
 		 * The black level from CameraSensorHelper is a 16-bit value.
 		 * The Mali-C55 ISP expects 20-bit settings, so we shift it to
 		 * the appropriate width
 		 */
 		context_.configuration.sensor.blackLevel =
-			camHelper_->blackLevel().value() << 4;
+			context_.camHelper->blackLevel().value() << 4;
 	}
 }
 
@@ -252,9 +249,9 @@  void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,
 
 	/* Compute the analogue gain limits. */
 	const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
-	float minGain = camHelper_->gain(v4l2Gain.min().get<int32_t>());
-	float maxGain = camHelper_->gain(v4l2Gain.max().get<int32_t>());
-	float defGain = camHelper_->gain(v4l2Gain.def().get<int32_t>());
+	float minGain = context_.camHelper->gain(v4l2Gain.min().get<int32_t>());
+	float maxGain = context_.camHelper->gain(v4l2Gain.max().get<int32_t>());
+	float defGain = context_.camHelper->gain(v4l2Gain.def().get<int32_t>());
 	ctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);
 
 	/*
@@ -362,7 +359,7 @@  void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
 	frameContext.agc.exposure =
 		sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
 	frameContext.agc.sensorGain =
-		camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
+		context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
 
 	ControlList metadata(controls::controls);