[PATCH v2 2/2] libipa: camera_sensor_helper: Fix rounding of gainCode

[Naushir Patuck]

Hi Kieran,

On Fri, 24 May 2024, 2:53 pm Kieran Bingham, <
kieran.bingham at ideasonboard.com> wrote:

> The implementation of gainCode for both Exponential and Linear gain
> models does not generate a gainCode that matches the result of the
> reverse operation.
>
> This can be seen by converting sequential gainCodes to a gain
> and converting that back to the gainCode. The values do not
> translate back due to only rounding down, rather than to the closest
> integer.
>
> Correct the rounding error and ensure that gainCode translation
> produces accurate bi-directional conversions from the perspective
> of the gainCode.
>

Just to cause a bit of controversy, some IPAs might prefer to round down
the gain code. This would allow a stable total exposure by compensating
with digital gain. If we round up, our total exposure will be higher than
what was requested, which is harder to compensate for.

However the visual differences will be extremely marginal.

Naush




> This fixes the IMX290, IMX296, IMX327 and IMX335 which use the
> Exponential gain model helpers, as well as IMX219, IMX258, IMX283, and
> IMX477 which use the Linear gain model helpers.


> This updates the corresponding test which was previously marked as
> expected to fail.
>
> Fixes: 8ad9249fb09d ("libipa: camera_sensor_helper: Implement exponential
> gain model")
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp | 6 +++---
>  test/ipa/meson.build                    | 3 +--
>  2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp
> b/src/ipa/libipa/camera_sensor_helper.cpp
> index 2cd61fccfbb9..4a92ead88b23 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -64,13 +64,13 @@ uint32_t CameraSensorHelper::gainCode(double gain)
> const
>         case AnalogueGainLinear:
>                 ASSERT(k.linear.m0 == 0 || k.linear.m1 == 0);
>
> -               return (k.linear.c0 - k.linear.c1 * gain) /
> -                      (k.linear.m1 * gain - k.linear.m0);
> +               return std::round((k.linear.c0 - k.linear.c1 * gain) /
> +                                 (k.linear.m1 * gain - k.linear.m0));
>
>         case AnalogueGainExponential:
>                 ASSERT(k.exp.a != 0 && k.exp.m != 0);
>
> -               return std::log2(gain / k.exp.a) / k.exp.m;
> +               return std::round(std::log2(gain / k.exp.a) / k.exp.m);
>
>         default:
>                 ASSERT(false);
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> index abc2456fc341..0b22c7576e56 100644
> --- a/test/ipa/meson.build
> +++ b/test/ipa/meson.build
> @@ -1,8 +1,7 @@
>  # SPDX-License-Identifier: CC0-1.0
>
>  ipa_test = [
> -    {'name': 'camera_sensor_helper', 'sources':
> ['camera_sensor_helper.cpp'],
> -     'should_fail': true},
> +    {'name': 'camera_sensor_helper', 'sources':
> ['camera_sensor_helper.cpp']},
>      {'name': 'ipa_module_test', 'sources': ['ipa_module_test.cpp']},
>      {'name': 'ipa_interface_test', 'sources': ['ipa_interface_test.cpp']},
>  ]
> --
> 2.34.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20240524/94dff06d/attachment.htm>