<div dir="auto"><div>Hi Kieran,</div><div dir="auto"><br><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">On Fri, 24 May 2024, 2:53 pm Kieran Bingham, <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The implementation of gainCode for both Exponential and Linear gain<br>
models does not generate a gainCode that matches the result of the<br>
reverse operation.<br>
<br>
This can be seen by converting sequential gainCodes to a gain<br>
and converting that back to the gainCode. The values do not<br>
translate back due to only rounding down, rather than to the closest<br>
integer.<br>
<br>
Correct the rounding error and ensure that gainCode translation<br>
produces accurate bi-directional conversions from the perspective<br>
of the gainCode.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">However the visual differences will be extremely marginal.</div><div dir="auto"><br></div><div dir="auto">Naush</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
This fixes the IMX290, IMX296, IMX327 and IMX335 which use the<br>
Exponential gain model helpers, as well as IMX219, IMX258, IMX283, and<br>
IMX477 which use the Linear gain model helpers.</blockquote></div></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
This updates the corresponding test which was previously marked as<br>
expected to fail.<br>
<br>
Fixes: 8ad9249fb09d ("libipa: camera_sensor_helper: Implement exponential gain model")<br>
Signed-off-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank" rel="noreferrer">kieran.bingham@ideasonboard.com</a>><br>
---<br>
src/ipa/libipa/camera_sensor_helper.cpp | 6 +++---<br>
test/ipa/meson.build | 3 +--<br>
2 files changed, 4 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp<br>
index 2cd61fccfbb9..4a92ead88b23 100644<br>
--- a/src/ipa/libipa/camera_sensor_helper.cpp<br>
+++ b/src/ipa/libipa/camera_sensor_helper.cpp<br>
@@ -64,13 +64,13 @@ uint32_t CameraSensorHelper::gainCode(double gain) const<br>
case AnalogueGainLinear:<br>
ASSERT(k.linear.m0 == 0 || k.linear.m1 == 0);<br>
<br>
- return (k.linear.c0 - k.linear.c1 * gain) /<br>
- (k.linear.m1 * gain - k.linear.m0);<br>
+ return std::round((k.linear.c0 - k.linear.c1 * gain) /<br>
+ (k.linear.m1 * gain - k.linear.m0));<br>
<br>
case AnalogueGainExponential:<br>
ASSERT(k.exp.a != 0 && k.exp.m != 0);<br>
<br>
- return std::log2(gain / k.exp.a) / k.exp.m;<br>
+ return std::round(std::log2(gain / k.exp.a) / k.exp.m);<br>
<br>
default:<br>
ASSERT(false);<br>
diff --git a/test/ipa/meson.build b/test/ipa/meson.build<br>
index abc2456fc341..0b22c7576e56 100644<br>
--- a/test/ipa/meson.build<br>
+++ b/test/ipa/meson.build<br>
@@ -1,8 +1,7 @@<br>
# SPDX-License-Identifier: CC0-1.0<br>
<br>
ipa_test = [<br>
- {'name': 'camera_sensor_helper', 'sources': ['camera_sensor_helper.cpp'],<br>
- 'should_fail': true},<br>
+ {'name': 'camera_sensor_helper', 'sources': ['camera_sensor_helper.cpp']},<br>
{'name': 'ipa_module_test', 'sources': ['ipa_module_test.cpp']},<br>
{'name': 'ipa_interface_test', 'sources': ['ipa_interface_test.cpp']},<br>
]<br>
-- <br>
2.34.1<br>
<br>
</blockquote></div></div></div>