[08/13] ipa: libipa: Awb: Provide common context storage

26490 diff mbox series

Expose a common structure for storing Awb Session configuration, Active
state and Frame Context data.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/ipa/libipa/awb.cpp       | 47 ++++++++++++++++++++++++++++++++++++++++++++
 src/ipa/libipa/awb.h         | 26 ++++++++++++++++++++++++
 src/ipa/rkisp1/ipa_context.h | 28 ++++++++------------------
 3 files changed, 81 insertions(+), 20 deletions(-)

Hi Kieran

On Tue, Apr 07, 2026 at 11:01:11PM +0100, Kieran Bingham wrote:
> Expose a common structure for storing Awb Session configuration, Active
> state and Frame Context data.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/ipa/libipa/awb.cpp       | 47 ++++++++++++++++++++++++++++++++++++++++++++
>  src/ipa/libipa/awb.h         | 26 ++++++++++++++++++++++++
>  src/ipa/rkisp1/ipa_context.h | 28 ++++++++------------------
>  3 files changed, 81 insertions(+), 20 deletions(-)
>
> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp
> index 214bac8b56a6ca7b0ac9954f0aa742cd613e6141..f215bea0b59483eadf95572f073928a4eb1275f4 100644
> --- a/src/ipa/libipa/awb.cpp
> +++ b/src/ipa/libipa/awb.cpp
> @@ -22,6 +22,53 @@ LOG_DEFINE_CATEGORY(Awb)
>
>  namespace ipa {
>
> +/**
> + * \struct awb::Session
> + * \brief Session-wide AWB configuration
> + *
> + * \var awb::Session::enabled
> + * \brief True when AWB processing is enabled for the session

This is easy to confuse with "auto awb" enabled.

I would say
        \brief True when AWB support is enabled

I wonder if this a "per-session" parameter or a global one (it depends
on the config file content, doesn't it?). But this is already like
this, we can change it on top if needed

> + */
> +
> +/**
> + * \struct awb::ActiveState
> + * \brief Active AWB state shared across frames

I would drop "Active" and use something like "most recent" ?

> + *
> + * \var awb::ActiveState::manual
> + * \brief The most recent manually requested AWB state
> + *
> + * \var awb::ActiveState::automatic
> + * \brief The most recent automatically calculated AWB state

And drop most recent from these two

> + *
> + * \var awb::ActiveState::autoEnabled
> + * \brief True when automatic AWB is currently selected
> + */
> +
> +/**
> + * \struct awb::ActiveState::AwbState
> + * \brief AWB gains and colour temperature for one operating mode

and drop "for one operating mode"

> + *
> + * \var awb::ActiveState::AwbState::gains
> + * \brief The white balance gains for this AWB state
> + *
> + * \var awb::ActiveState::AwbState::temperatureK
> + * \brief The colour temperature for this AWB state, in Kelvin

and "for this AWB state"

> + */
> +
> +/**
> + * \struct awb::FrameContext
> + * \brief Per-frame AWB state applied to a captured frame

Either "Per-frame" or "applied to a captured frame". Don't they mean
the same thing ?

> + *
> + * \var awb::FrameContext::gains
> + * \brief The white balance gains applied to the frame
> + *
> + * \var awb::FrameContext::autoEnabled
> + * \brief True when the frame uses automatic AWB

True when the gains have been computed by the AWB algorithm ?

> + *
> + * \var awb::FrameContext::temperatureK
> + * \brief The colour temperature used for the frame, in Kelvin

And drop "to the frame"/"for the frame" ?

> + */
> +
>  /**
>   * \class AwbResult
>   * \brief The result of an AWB calculation
> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h
> index f4a86038635f984ac03b1e466c0fcd4e6d5e22bd..764be849270bcd42ecdf67aea3d13afa170c7499 100644
> --- a/src/ipa/libipa/awb.h
> +++ b/src/ipa/libipa/awb.h
> @@ -20,6 +20,32 @@ namespace libcamera {
>
>  namespace ipa {
>
> +namespace awb {
> +
> +struct Session {
> +	bool enabled;
> +};
> +
> +struct ActiveState {
> +	struct AwbState {
> +		RGB<double> gains;
> +		unsigned int temperatureK;
> +	};
> +
> +	AwbState manual;
> +	AwbState automatic;
> +
> +	bool autoEnabled;
> +};
> +
> +struct FrameContext {
> +	RGB<double> gains;
> +	bool autoEnabled;
> +	unsigned int temperatureK;
> +};
> +
> +} /* namespace awb */
> +
>  struct AwbResult {
>  	RGB<double> gains;
>  	double colourTemperature;
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 63a6f2b7d884bfff2553175fa32e45ae747fb4c7..85a8ea3acc1fd75ff6b49576800ab7615cebce2c 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -25,6 +25,7 @@
>  #include "libcamera/internal/vector.h"
>
>  #include "libipa/agc_mean_luminance.h"
> +#include "libipa/awb.h"
>  #include "libipa/camera_sensor_helper.h"
>  #include "libipa/fc_queue.h"
>  #include "libipa/fixedpoint.h"
> @@ -49,15 +50,16 @@ struct IPAHwSettings {
>  	bool compand;
>  };
>
> +struct RKISP1AwbSession : public ipa::awb::Session {
> +	struct rkisp1_cif_isp_window measureWindow;
> +};
> +
>  struct IPASessionConfiguration {
>  	struct {
>  		struct rkisp1_cif_isp_window measureWindow;
>  	} agc;
>
> -	struct {
> -		struct rkisp1_cif_isp_window measureWindow;
> -		bool enabled;
> -	} awb;
> +	struct RKISP1AwbSession awb;
>
>  	struct {
>  		bool supported;
> @@ -103,17 +105,7 @@ struct IPAActiveState {
>  		utils::Duration maxFrameDuration;
>  	} agc;
>
> -	struct {
> -		struct AwbState {
> -			RGB<double> gains;
> -			unsigned int temperatureK;
> -		};
> -
> -		AwbState manual;
> -		AwbState automatic;
> -
> -		bool autoEnabled;
> -	} awb;
> +	ipa::awb::ActiveState awb;
>
>  	struct {
>  		Matrix<float, 3, 3> manual;
> @@ -175,11 +167,7 @@ struct IPAFrameContext : public FrameContext {
>  		bool autoGainModeChange;
>  	} agc;
>
> -	struct {
> -		RGB<double> gains;
> -		bool autoEnabled;
> -		unsigned int temperatureK;
> -	} awb;
> +	ipa::awb::FrameContext awb;

All minors or suggestions

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>
>  	struct {
>  		BrightnessQ brightness;
>
> --
> 2.53.0
>