[libcamera-devel] [PATCH 1/2] libipa: Round gain code before returning the value

[Jean-Michel Hautbois]

On 19/11/2021 12:11, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-11-19 10:25:58)
>> We can have the case where the gain is very close to 1, as coded in
>> double, and after gainCode() returns, the value as an uint32_t is 0. In
>> order to avoid that, always round the value halfway cases away from zero
>> before returning.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> 
> This seems to make sense.
> 
> Are there any issues with other values being rounded? I presume not.
> Is there any requirement to further protect or warn if gainCode is
> returned as zero? or can that still be valid in some situations?

That is a tough question... Can we have a gainCode set to 0 in a linear 
analogue gain controlled sensor... I am not sure of that at all...

BTW, investigating it (before introducing std::round) I found that using 
float instead of double made it return 1...

> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
>> ---
>>   src/ipa/libipa/camera_sensor_helper.cpp | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
>> index 0b0eb503..67cf6913 100644
>> --- a/src/ipa/libipa/camera_sensor_helper.cpp
>> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
>> @@ -7,6 +7,8 @@
>>    */
>>   #include "camera_sensor_helper.h"
>>   
>> +#include <cmath>
>> +
>>   #include <libcamera/base/log.h>
>>   
>>   /**
>> @@ -61,8 +63,8 @@ uint32_t CameraSensorHelper::gainCode(double gain) const
>>          ASSERT(analogueGainConstants_.m0 == 0 || analogueGainConstants_.m1 == 0);
>>          ASSERT(analogueGainConstants_.type == AnalogueGainLinear);
>>   
>> -       return (analogueGainConstants_.c0 - analogueGainConstants_.c1 * gain) /
>> -              (analogueGainConstants_.m1 * gain - analogueGainConstants_.m0);
>> +       return std::round((analogueGainConstants_.c0 - analogueGainConstants_.c1 * gain) /
>> +                         (analogueGainConstants_.m1 * gain - analogueGainConstants_.m0));
>>   }
>>   
>>   /**
>> -- 
>> 2.32.0
>>