[v2,07/16] libipa: agc_mean_luminance: Configure the exposure mode helpers
diff mbox series

Message ID 20250808141315.413839-8-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Implement WDR algorithm
Related show

Commit Message

Stefan Klug Aug. 8, 2025, 2:12 p.m. UTC 
Add a function to configure the exposure mode helpers with the line
length and sensor helper to take quantization effects into account.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/libipa/agc_mean_luminance.cpp | 15 +++++++++++++++
 src/ipa/libipa/agc_mean_luminance.h   |  1 +
 2 files changed, 16 insertions(+)

Comments

Dan Scally Aug. 11, 2025, 3:07 p.m. UTC | #1 
Hi Stefan

On 08/08/2025 15:12, Stefan Klug wrote:
> Add a function to configure the exposure mode helpers with the line
> length and sensor helper to take quantization effects into account.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>   src/ipa/libipa/agc_mean_luminance.cpp | 15 +++++++++++++++
>   src/ipa/libipa/agc_mean_luminance.h   |  1 +
>   2 files changed, 16 insertions(+)
> 
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index ff96a381ffce..58c4dfc9add2 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -311,6 +311,21 @@ int AgcMeanLuminance::parseExposureModes(const YamlObject &tuningData)
>   	return 0;
>   }
>   
> +/**
> + * \brief Configure the exposure mode helpers
> + * \param[in] lineLength The sensor line length
> + * \param[in] sensorHelper The sensor helper
> + *
> + * This function configures the exposure mode helpers so they can correctly
> + * take quantization effects into account.
> + */
> +void AgcMeanLuminance::configure(utils::Duration lineLength,

Very very mild dislike of the word "length" to name a Duration which my 
personality compels me to mention, but with this name or any other:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

> +				 const CameraSensorHelper *sensorHelper)
> +{
> +	for (auto &[id, helper] : exposureModeHelpers_)
> +		helper->configure(lineLength, sensorHelper);
> +}
> +
>   /**
>    * \brief Parse tuning data for AeConstraintMode and AeExposureMode controls
>    * \param[in] tuningData the YamlObject representing the tuning data
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index cad7ef845487..a7c7e006af42 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -42,6 +42,7 @@ public:
>   		double yTarget;
>   	};
>   
> +	void configure(utils::Duration lineLength, const CameraSensorHelper *sensorHelper);
>   	int parseTuningData(const YamlObject &tuningData);
>   
>   	void setExposureCompensation(double gain)
Stefan Klug Aug. 13, 2025, 8:46 a.m. UTC | #2 
Hi Dan,

Thank you for the review.

Quoting Dan Scally (2025-08-11 17:07:11)
> Hi Stefan
> 
> On 08/08/2025 15:12, Stefan Klug wrote:
> > Add a function to configure the exposure mode helpers with the line
> > length and sensor helper to take quantization effects into account.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >   src/ipa/libipa/agc_mean_luminance.cpp | 15 +++++++++++++++
> >   src/ipa/libipa/agc_mean_luminance.h   |  1 +
> >   2 files changed, 16 insertions(+)
> > 
> > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> > index ff96a381ffce..58c4dfc9add2 100644
> > --- a/src/ipa/libipa/agc_mean_luminance.cpp
> > +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> > @@ -311,6 +311,21 @@ int AgcMeanLuminance::parseExposureModes(const YamlObject &tuningData)
> >       return 0;
> >   }
> >   
> > +/**
> > + * \brief Configure the exposure mode helpers
> > + * \param[in] lineLength The sensor line length
> > + * \param[in] sensorHelper The sensor helper
> > + *
> > + * This function configures the exposure mode helpers so they can correctly
> > + * take quantization effects into account.
> > + */
> > +void AgcMeanLuminance::configure(utils::Duration lineLength,
> 
> Very very mild dislike of the word "length" to name a Duration which my 
> personality compels me to mention, but with this name or any other:

Indeed lineDuration sound better. I'll rename that.

> 
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

Thanks,
Stefan

> 
> > +                              const CameraSensorHelper *sensorHelper)
> > +{
> > +     for (auto &[id, helper] : exposureModeHelpers_)
> > +             helper->configure(lineLength, sensorHelper);
> > +}
> > +
> >   /**
> >    * \brief Parse tuning data for AeConstraintMode and AeExposureMode controls
> >    * \param[in] tuningData the YamlObject representing the tuning data
> > diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> > index cad7ef845487..a7c7e006af42 100644
> > --- a/src/ipa/libipa/agc_mean_luminance.h
> > +++ b/src/ipa/libipa/agc_mean_luminance.h
> > @@ -42,6 +42,7 @@ public:
> >               double yTarget;
> >       };
> >   
> > +     void configure(utils::Duration lineLength, const CameraSensorHelper *sensorHelper);
> >       int parseTuningData(const YamlObject &tuningData);
> >   
> >       void setExposureCompensation(double gain)
>

Patch
diff mbox series 

diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
index ff96a381ffce..58c4dfc9add2 100644
--- a/src/ipa/libipa/agc_mean_luminance.cpp
+++ b/src/ipa/libipa/agc_mean_luminance.cpp
@@ -311,6 +311,21 @@  int AgcMeanLuminance::parseExposureModes(const YamlObject &tuningData)
 	return 0;
 }
 
+/**
+ * \brief Configure the exposure mode helpers
+ * \param[in] lineLength The sensor line length
+ * \param[in] sensorHelper The sensor helper
+ *
+ * This function configures the exposure mode helpers so they can correctly
+ * take quantization effects into account.
+ */
+void AgcMeanLuminance::configure(utils::Duration lineLength,
+				 const CameraSensorHelper *sensorHelper)
+{
+	for (auto &[id, helper] : exposureModeHelpers_)
+		helper->configure(lineLength, sensorHelper);
+}
+
 /**
  * \brief Parse tuning data for AeConstraintMode and AeExposureMode controls
  * \param[in] tuningData the YamlObject representing the tuning data
diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
index cad7ef845487..a7c7e006af42 100644
--- a/src/ipa/libipa/agc_mean_luminance.h
+++ b/src/ipa/libipa/agc_mean_luminance.h
@@ -42,6 +42,7 @@  public:
 		double yTarget;
 	};
 
+	void configure(utils::Duration lineLength, const CameraSensorHelper *sensorHelper);
 	int parseTuningData(const YamlObject &tuningData);
 
 	void setExposureCompensation(double gain)