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

[Dan Scally]

Hello Umang, Paul

On 12/04/2024 09:24, Umang Jain wrote:
> 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?

Thank you for the fixes - I squashed this into v2 of the centralised AGC series, but dropped the 
expMeans parameter from the doxygen comment 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;
>