[PATCH 3/5] fixup: ipa: rkisp1: Remove bespoke Agc functions

Umang Jain umang.jain at ideasonboard.com
Fri Apr 12 10:24:10 CEST 2024


Hi Paul

Thank you for the patch.

On 05/04/24 8:17 pm, Paul Elder wrote:
> Remove some constants that should have been removed, and restore a
> fairly informative comment that got removed.
>
> Although the removal of one of the constants also removes a todo
> comment, this will be addressed imminently so it should be fine.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>   src/ipa/rkisp1/algorithms/agc.cpp | 32 +++++++++++++++++++++++++------
>   1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 7220f00a..1dfc4aaa 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -39,12 +39,6 @@ namespace ipa::rkisp1::algorithms {
>   
>   LOG_DEFINE_CATEGORY(RkISP1Agc)
>   
> -/* Minimum limit for analogue gain value */
> -static constexpr double kMinAnalogueGain = 1.0;
> -
> -/* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
> -static constexpr utils::Duration kMaxShutterSpeed = 60ms;
> -
>   int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
>   			    const char *prop)
>   {
> @@ -315,6 +309,32 @@ void Agc::parseStatistics(const rkisp1_stat_buffer *stats,
>   					       context.hw->numHistogramBins), 4);
>   }
>   
> +/**
> + * \brief Estimate the relative luminance of the frame with a given gain
> + * \param[in] expMeans The mean luminance values, from the RkISP1 statistics
> + * \param[in] gain The gain to apply to the frame

The documentation states two input function parameters but looking at 
this patch, estimateLuminance() has only gain parameter ?

Did you miss to update the function signature here?
> + *
> + * This function estimates the average relative luminance of the frame that
> + * would be output by the sensor if an additional \a gain was applied.
> + *
> + * The estimation is based on the AE statistics for the current frame. Y
> + * averages for all cells are first multiplied by the gain, and then saturated
> + * to approximate the sensor behaviour at high brightness values. The
> + * approximation is quite rough, as it doesn't take into account non-linearities
> + * when approaching saturation. In this case, saturating after the conversion to
> + * YUV doesn't take into account the fact that the R, G and B components
> + * contribute differently to the relative luminance.
> + *
> + * \todo Have a dedicated YUV algorithm ?
> + *
> + * The values are normalized to the [0.0, 1.0] range, where 1.0 corresponds to a
> + * theoretical perfect reflector of 100% reference white.
> + *
> + * More detailed information can be found in:
> + * https://en.wikipedia.org/wiki/Relative_luminance
> + *
> + * \return The relative luminance
> + */
>   double Agc::estimateLuminance(double gain)
>   {
>   	double ySum = 0.0;



More information about the libcamera-devel mailing list