[libcamera-devel] [PATCH v4 7/9] ipa: rkisp1: Add enable field for AWB algorithm in IPA context

[paul.elder at ideasonboard.com]

Hi Laurent,

On Tue, Aug 16, 2022 at 04:54:12AM +0300, Laurent Pinchart via libcamera-devel wrote:
> From: Florian Sylvestre <fsylvestre at baylibre.com>
> 
> Add an enable variable in the awb struct in IPASessionConfiguration which
> indicates if the awb algorithm has been configured. This will allow other
> algorithms to retrieve this information.
> 
> Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 2 ++
>  src/ipa/rkisp1/ipa_context.cpp    | 3 +++
>  src/ipa/rkisp1/ipa_context.h      | 1 +
>  3 files changed, 6 insertions(+)
> 
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index 9f00364d12b1..d1328f011081 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -48,6 +48,8 @@ int Awb::configure(IPAContext &context,
>  	context.configuration.awb.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;
>  	context.configuration.awb.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;
>  
> +	context.configuration.awb.enabled = true;
> +
>  	return 0;
>  }
>  
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index ef8bb8e931c8..23a63f8c6e25 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -87,6 +87,9 @@ namespace libcamera::ipa::rkisp1 {
>   *
>   * \var IPASessionConfiguration::awb.measureWindow
>   * \brief AWB measure window
> + *
> + * \var IPASessionConfiguration::awb.enabled
> + * \brief Indicates if the AWB hardware is enabled to apply colour gains

Ah, so this is the enable variable that you were talking about :)

>From what I understand, this variable holds whether or not the awb
algorithm has been configured? So once Awb::configure() completes
successfully then this is set to (and stays) true?

If that's the case, I think the description could use an upgrade, as
the variable doesn't signify that *only* the hardware is enabled for awb
(actually what I parse from the the current docstring is that it's
enabled to apply /manual/ color gains).

Or maybe the docstirng is correct and I'm not parsing it correctly,
which also means that it could use an upgrade :p


Paul

>   */
>  
>  /**
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 2bdb6a81d7c9..7f7b3e4d88fa 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -29,6 +29,7 @@ struct IPASessionConfiguration {
>  
>  	struct {
>  		struct rkisp1_cif_isp_window measureWindow;
> +		bool enabled;
>  	} awb;
>  
>  	struct {