[v2,11/12] ipa: ipu3: Add IPACameraSensorInfo to IPAContext
diff mbox series

Message ID 20260626-ipu3-libipa-rework-v2-11-41546e23de3e@ideasonboard.com
State New
Headers show
Series
  • libipa: Re-work IPU3 IPA to use libipa algorithms
Related show

Commit Message

Dan Scally June 26, 2026, 1:05 p.m. UTC 
Add an instance of IPACameraSensorInfo to the IPAContext and fill it
during IPAIPU3::init(). This will allow us to pass sensor information
to algorithms at init time. This means that IPAIPU3::sensorInfo_ is
no longer needed, as the instance stored in the context is accessible
everywhere that that was previously used. Drop it.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v2:

        - Dropped the sensorInfo_ class member, since this replaces it
---
 src/ipa/ipu3/ipa_context.cpp |  3 +++
 src/ipa/ipu3/ipa_context.h   |  3 +++
 src/ipa/ipu3/ipu3.cpp        | 20 +++++++++-----------
 3 files changed, 15 insertions(+), 11 deletions(-)

Comments

Kieran Bingham June 29, 2026, 10 a.m. UTC | #1 
Quoting Daniel Scally (2026-06-26 14:05:58)
> Add an instance of IPACameraSensorInfo to the IPAContext and fill it
> during IPAIPU3::init(). This will allow us to pass sensor information
> to algorithms at init time. This means that IPAIPU3::sensorInfo_ is
> no longer needed, as the instance stored in the context is accessible
> everywhere that that was previously used. Drop it.

aha, this kind of superceeds my comment/worries on the previous patch
and means that's just a preparatory - and it's fine.

And yes, this looks like how I would expect to store it.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v2:
> 
>         - Dropped the sensorInfo_ class member, since this replaces it
> ---
>  src/ipa/ipu3/ipa_context.cpp |  3 +++
>  src/ipa/ipu3/ipa_context.h   |  3 +++
>  src/ipa/ipu3/ipu3.cpp        | 20 +++++++++-----------
>  3 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index d3bcd7a81fe010d17d1437bf7f6cedce83a9be5c..9537802ceca5018118bdf4c71ef361c20fc44bfb 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -54,6 +54,9 @@ namespace libcamera::ipa::ipu3 {
>   *
>   * \var IPAContext::ctrlMap
>   * \brief A ControlInfoMap::Map of controls populated by the algorithms
> + *
> + * \var IPAContext::sensorInfo
> + * \brief An IPACameraSensorInfo instance holding information about the sensor
>   */
>  
>  /**
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index ce496ed477f4cbe2653aedada3381258396d92c8..d650f2fe1ad8eab91b7128a47c9690e42b1595f1 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -15,6 +15,8 @@
>  #include <libcamera/controls.h>
>  #include <libcamera/geometry.h>
>  
> +#include <libcamera/ipa/core_ipa_interface.h>
> +
>  #include <libipa/awb.h>
>  #include <libipa/ccm.h>
>  #include <libipa/fc_queue.h>
> @@ -91,6 +93,7 @@ struct IPAContext {
>         FCQueue<IPAFrameContext> frameContexts;
>  
>         ControlInfoMap::Map ctrlMap;
> +       IPACameraSensorInfo sensorInfo;
>  };
>  
>  } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index b1c176e68098b24d8fd37e7a0e278cabf0e1c3b2..c2d3e62d12f4aba9e3d008025c8922a71d45d36f 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -177,8 +177,6 @@ private:
>         ControlInfoMap sensorCtrls_;
>         ControlInfoMap lensCtrls_;
>  
> -       IPACameraSensorInfo sensorInfo_;
> -
>         /* Interface to the Camera Helper */
>         std::unique_ptr<CameraSensorHelper> camHelper_;
>  
> @@ -265,19 +263,19 @@ void IPAIPU3::updateControls(const ControlInfoMap &sensorControls,
>          */
>         const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;
>         uint32_t hblank = v4l2HBlank.def().get<int32_t>();
> -       uint32_t lineLength = sensorInfo_.outputSize.width + hblank;
> +       uint32_t lineLength = context_.sensorInfo.outputSize.width + hblank;
>  
>         const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
>         std::array<uint32_t, 3> frameHeights{
> -               v4l2VBlank.min().get<int32_t>() + sensorInfo_.outputSize.height,
> -               v4l2VBlank.max().get<int32_t>() + sensorInfo_.outputSize.height,
> -               v4l2VBlank.def().get<int32_t>() + sensorInfo_.outputSize.height,
> +               v4l2VBlank.min().get<int32_t>() + context_.sensorInfo.outputSize.height,
> +               v4l2VBlank.max().get<int32_t>() + context_.sensorInfo.outputSize.height,
> +               v4l2VBlank.def().get<int32_t>() + context_.sensorInfo.outputSize.height,
>         };
>  
>         std::array<int64_t, 3> frameDurations;
>         for (unsigned int i = 0; i < frameHeights.size(); ++i) {
>                 uint64_t frameSize = lineLength * frameHeights[i];
> -               frameDurations[i] = frameSize / (sensorInfo_.pixelRate / 1000000U);
> +               frameDurations[i] = frameSize / (context_.sensorInfo.pixelRate / 1000000U);
>         }
>         controls[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
>                                                                frameDurations[1],
> @@ -311,7 +309,7 @@ int IPAIPU3::init(const IPASettings &settings,
>         context_.configuration = {};
>         context_.configuration.sensor.lineDuration =
>                 sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;
> -       sensorInfo_ = sensorInfo;
> +       context_.sensorInfo = sensorInfo;
>  
>         /* Load the tuning data file. */
>         File file(settings.configurationFile);
> @@ -464,7 +462,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>                 return -ENODATA;
>         }
>  
> -       sensorInfo_ = configInfo.sensorInfo;
> +       context_.sensorInfo = configInfo.sensorInfo;
>  
>         lensCtrls_ = configInfo.lensControls;
>  
> @@ -475,8 +473,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>  
>         /* Initialise the sensor configuration. */
>         context_.configuration.sensor.lineDuration =
> -               sensorInfo_.minLineLength * 1.0s / sensorInfo_.pixelRate;
> -       context_.configuration.sensor.size = sensorInfo_.outputSize;
> +               context_.sensorInfo.minLineLength * 1.0s / context_.sensorInfo.pixelRate;
> +       context_.configuration.sensor.size = context_.sensorInfo.outputSize;
>  
>         /*
>          * Compute the sensor V4L2 controls to be used by the algorithms and
> 
> -- 
> 2.43.0
>
Barnabás Pőcze June 30, 2026, 11:36 a.m. UTC | #2 
2026. 06. 26. 15:05 keltezéssel, Daniel Scally írta:
> Add an instance of IPACameraSensorInfo to the IPAContext and fill it
> during IPAIPU3::init(). This will allow us to pass sensor information
> to algorithms at init time. This means that IPAIPU3::sensorInfo_ is
> no longer needed, as the instance stored in the context is accessible
> everywhere that that was previously used. Drop it.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v2:
> 
>          - Dropped the sensorInfo_ class member, since this replaces it
> ---

Looks ok to me.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   src/ipa/ipu3/ipa_context.cpp |  3 +++
>   src/ipa/ipu3/ipa_context.h   |  3 +++
>   src/ipa/ipu3/ipu3.cpp        | 20 +++++++++-----------
>   3 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index d3bcd7a81fe010d17d1437bf7f6cedce83a9be5c..9537802ceca5018118bdf4c71ef361c20fc44bfb 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -54,6 +54,9 @@ namespace libcamera::ipa::ipu3 {
>    *
>    * \var IPAContext::ctrlMap
>    * \brief A ControlInfoMap::Map of controls populated by the algorithms
> + *
> + * \var IPAContext::sensorInfo
> + * \brief An IPACameraSensorInfo instance holding information about the sensor
>    */
>   
>   /**
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index ce496ed477f4cbe2653aedada3381258396d92c8..d650f2fe1ad8eab91b7128a47c9690e42b1595f1 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -15,6 +15,8 @@
>   #include <libcamera/controls.h>
>   #include <libcamera/geometry.h>
>   
> +#include <libcamera/ipa/core_ipa_interface.h>
> +
>   #include <libipa/awb.h>
>   #include <libipa/ccm.h>
>   #include <libipa/fc_queue.h>
> @@ -91,6 +93,7 @@ struct IPAContext {
>   	FCQueue<IPAFrameContext> frameContexts;
>   
>   	ControlInfoMap::Map ctrlMap;
> +	IPACameraSensorInfo sensorInfo;
>   };
>   
>   } /* namespace ipa::ipu3 */
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index b1c176e68098b24d8fd37e7a0e278cabf0e1c3b2..c2d3e62d12f4aba9e3d008025c8922a71d45d36f 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -177,8 +177,6 @@ private:
>   	ControlInfoMap sensorCtrls_;
>   	ControlInfoMap lensCtrls_;
>   
> -	IPACameraSensorInfo sensorInfo_;
> -
>   	/* Interface to the Camera Helper */
>   	std::unique_ptr<CameraSensorHelper> camHelper_;
>   
> @@ -265,19 +263,19 @@ void IPAIPU3::updateControls(const ControlInfoMap &sensorControls,
>   	 */
>   	const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;
>   	uint32_t hblank = v4l2HBlank.def().get<int32_t>();
> -	uint32_t lineLength = sensorInfo_.outputSize.width + hblank;
> +	uint32_t lineLength = context_.sensorInfo.outputSize.width + hblank;
>   
>   	const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
>   	std::array<uint32_t, 3> frameHeights{
> -		v4l2VBlank.min().get<int32_t>() + sensorInfo_.outputSize.height,
> -		v4l2VBlank.max().get<int32_t>() + sensorInfo_.outputSize.height,
> -		v4l2VBlank.def().get<int32_t>() + sensorInfo_.outputSize.height,
> +		v4l2VBlank.min().get<int32_t>() + context_.sensorInfo.outputSize.height,
> +		v4l2VBlank.max().get<int32_t>() + context_.sensorInfo.outputSize.height,
> +		v4l2VBlank.def().get<int32_t>() + context_.sensorInfo.outputSize.height,
>   	};
>   
>   	std::array<int64_t, 3> frameDurations;
>   	for (unsigned int i = 0; i < frameHeights.size(); ++i) {
>   		uint64_t frameSize = lineLength * frameHeights[i];
> -		frameDurations[i] = frameSize / (sensorInfo_.pixelRate / 1000000U);
> +		frameDurations[i] = frameSize / (context_.sensorInfo.pixelRate / 1000000U);
>   	}
>   	controls[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
>   							       frameDurations[1],
> @@ -311,7 +309,7 @@ int IPAIPU3::init(const IPASettings &settings,
>   	context_.configuration = {};
>   	context_.configuration.sensor.lineDuration =
>   		sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;
> -	sensorInfo_ = sensorInfo;
> +	context_.sensorInfo = sensorInfo;
>   
>   	/* Load the tuning data file. */
>   	File file(settings.configurationFile);
> @@ -464,7 +462,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>   		return -ENODATA;
>   	}
>   
> -	sensorInfo_ = configInfo.sensorInfo;
> +	context_.sensorInfo = configInfo.sensorInfo;
>   
>   	lensCtrls_ = configInfo.lensControls;
>   
> @@ -475,8 +473,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>   
>   	/* Initialise the sensor configuration. */
>   	context_.configuration.sensor.lineDuration =
> -		sensorInfo_.minLineLength * 1.0s / sensorInfo_.pixelRate;
> -	context_.configuration.sensor.size = sensorInfo_.outputSize;
> +		context_.sensorInfo.minLineLength * 1.0s / context_.sensorInfo.pixelRate;
> +	context_.configuration.sensor.size = context_.sensorInfo.outputSize;
>   
>   	/*
>   	 * Compute the sensor V4L2 controls to be used by the algorithms and
>

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
index d3bcd7a81fe010d17d1437bf7f6cedce83a9be5c..9537802ceca5018118bdf4c71ef361c20fc44bfb 100644
--- a/src/ipa/ipu3/ipa_context.cpp
+++ b/src/ipa/ipu3/ipa_context.cpp
@@ -54,6 +54,9 @@  namespace libcamera::ipa::ipu3 {
  *
  * \var IPAContext::ctrlMap
  * \brief A ControlInfoMap::Map of controls populated by the algorithms
+ *
+ * \var IPAContext::sensorInfo
+ * \brief An IPACameraSensorInfo instance holding information about the sensor
  */
 
 /**
diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
index ce496ed477f4cbe2653aedada3381258396d92c8..d650f2fe1ad8eab91b7128a47c9690e42b1595f1 100644
--- a/src/ipa/ipu3/ipa_context.h
+++ b/src/ipa/ipu3/ipa_context.h
@@ -15,6 +15,8 @@ 
 #include <libcamera/controls.h>
 #include <libcamera/geometry.h>
 
+#include <libcamera/ipa/core_ipa_interface.h>
+
 #include <libipa/awb.h>
 #include <libipa/ccm.h>
 #include <libipa/fc_queue.h>
@@ -91,6 +93,7 @@  struct IPAContext {
 	FCQueue<IPAFrameContext> frameContexts;
 
 	ControlInfoMap::Map ctrlMap;
+	IPACameraSensorInfo sensorInfo;
 };
 
 } /* namespace ipa::ipu3 */
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index b1c176e68098b24d8fd37e7a0e278cabf0e1c3b2..c2d3e62d12f4aba9e3d008025c8922a71d45d36f 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -177,8 +177,6 @@  private:
 	ControlInfoMap sensorCtrls_;
 	ControlInfoMap lensCtrls_;
 
-	IPACameraSensorInfo sensorInfo_;
-
 	/* Interface to the Camera Helper */
 	std::unique_ptr<CameraSensorHelper> camHelper_;
 
@@ -265,19 +263,19 @@  void IPAIPU3::updateControls(const ControlInfoMap &sensorControls,
 	 */
 	const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;
 	uint32_t hblank = v4l2HBlank.def().get<int32_t>();
-	uint32_t lineLength = sensorInfo_.outputSize.width + hblank;
+	uint32_t lineLength = context_.sensorInfo.outputSize.width + hblank;
 
 	const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
 	std::array<uint32_t, 3> frameHeights{
-		v4l2VBlank.min().get<int32_t>() + sensorInfo_.outputSize.height,
-		v4l2VBlank.max().get<int32_t>() + sensorInfo_.outputSize.height,
-		v4l2VBlank.def().get<int32_t>() + sensorInfo_.outputSize.height,
+		v4l2VBlank.min().get<int32_t>() + context_.sensorInfo.outputSize.height,
+		v4l2VBlank.max().get<int32_t>() + context_.sensorInfo.outputSize.height,
+		v4l2VBlank.def().get<int32_t>() + context_.sensorInfo.outputSize.height,
 	};
 
 	std::array<int64_t, 3> frameDurations;
 	for (unsigned int i = 0; i < frameHeights.size(); ++i) {
 		uint64_t frameSize = lineLength * frameHeights[i];
-		frameDurations[i] = frameSize / (sensorInfo_.pixelRate / 1000000U);
+		frameDurations[i] = frameSize / (context_.sensorInfo.pixelRate / 1000000U);
 	}
 	controls[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
 							       frameDurations[1],
@@ -311,7 +309,7 @@  int IPAIPU3::init(const IPASettings &settings,
 	context_.configuration = {};
 	context_.configuration.sensor.lineDuration =
 		sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;
-	sensorInfo_ = sensorInfo;
+	context_.sensorInfo = sensorInfo;
 
 	/* Load the tuning data file. */
 	File file(settings.configurationFile);
@@ -464,7 +462,7 @@  int IPAIPU3::configure(const IPAConfigInfo &configInfo,
 		return -ENODATA;
 	}
 
-	sensorInfo_ = configInfo.sensorInfo;
+	context_.sensorInfo = configInfo.sensorInfo;
 
 	lensCtrls_ = configInfo.lensControls;
 
@@ -475,8 +473,8 @@  int IPAIPU3::configure(const IPAConfigInfo &configInfo,
 
 	/* Initialise the sensor configuration. */
 	context_.configuration.sensor.lineDuration =
-		sensorInfo_.minLineLength * 1.0s / sensorInfo_.pixelRate;
-	context_.configuration.sensor.size = sensorInfo_.outputSize;
+		context_.sensorInfo.minLineLength * 1.0s / context_.sensorInfo.pixelRate;
+	context_.configuration.sensor.size = context_.sensorInfo.outputSize;
 
 	/*
 	 * Compute the sensor V4L2 controls to be used by the algorithms and