[libcamera-devel,v2,00/12] Improve ImgU statistics usage

13983 mbox series

Hello everybody !

Here is a series, solving quite a few issues and clarifying most of the
ImgU statistics format.

Main changes in v2:
- stride is now a IPASessionConfiguration parameter
- Optical Black Correction is an algorithm

The first part has already been sent before and was titled "Move and
improve AWB structures" and those did not change since the latest v8 of
it:
- Patch 1/12 moves the AWB structures to be able to use those from the
ipa::ipu3::algorithms namespace (by AGC at least).
- Patch 2/12 renames the stats region structure to make it clear it is an
accumulator structure.
- Patch 3/12 is improving the Accumulator structure to have the same layout
as the IPAFrameContext::awb structure.
- Patch 4/12 is now only focusing on AWB name usage.

We worked, together with Laurent (thanks !) on the AWB grid limits, and
ImgU statistics format. There was differences between public API [0] and
the kernel doc which needed clarifications (who is correct ?). After
quite a few investigations, we discovered a bug in the kernel, for low
resolutions [1] and this leads to patches 5/12 and 6/12, the latest
reusing the proposal from Laurent in "ipa: ipu3: Split width and height
loops in calculateBdsGrid()" but adapting it to the limit names.

Next patches are here to solve bugs in AWB by configuring the proportion
of unsaturated zones (7/12), using the right gain multipliers (8/12) and
taking care of padding in the AWB loop (9/12).

As we are improving AWB, I cherry-picked patch 10/12 from another branch
already proposed before to use the black level correction in the ImgU
with default values (probably good enough for now).

Next, as AGC is using the AWB statistics for its algorithm, rewrite the
loop to simplify it and take care of padding too (11/12).

And we can finally move the Ipu3AwbCell out from the Awb class, to use
it with the (yet to be in v2) patch from the kernel [2].

[0] https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/awb_public.h
[1] https://lore.kernel.org/linux-media/20210916172504.677919-1-jeanmichel.hautbois@ideasonboard.com/
[2] https://lore.kernel.org/linux-media/20210831185140.77400-1-jeanmichel.hautbois@ideasonboard.com/

Jean-Michel Hautbois (12):
  ipa: ipu3: Move the AWB stats structures
  ipa: ipu3: Rename IspStatsRegion to Accumulator
  ipa: ipu3: Change Accumulator structure layout
  ipa: ipu3: awb: Make the naming consistent
  ipa: ipu3: Change the limits of the AWB stats
  ipa: ipu3: Change limits and split loops in calculateBdsGrid()
  ipa: ipu3: awb: Correct the relevant zones proportion
  ipa: ipu3: awb: Correct the gain multipliers
  ipa: ipu3: awb: Use the line stride for the stats
  ipa: ipu3: awb: Introduce Black Level Correction
  ipa: ipu3: agc: Rewrite and simplify the brightness loop
  ipa: ipu3: Replace ipa::ipu3::algorithms::Ipu3AwbCell

 include/linux/intel-ipu3.h                   |  31 ++-
 src/ipa/ipu3/algorithms/agc.cpp              |  53 ++---
 src/ipa/ipu3/algorithms/agc.h                |   2 +
 src/ipa/ipu3/algorithms/awb.cpp              | 195 +++++++++++--------
 src/ipa/ipu3/algorithms/awb.h                |  42 ++--
 src/ipa/ipu3/algorithms/black_correction.cpp |  67 +++++++
 src/ipa/ipu3/algorithms/black_correction.h   |  28 +++
 src/ipa/ipu3/algorithms/meson.build          |   1 +
 src/ipa/ipu3/ipa_context.h                   |   1 +
 src/ipa/ipu3/ipu3.cpp                        |  75 ++++---
 10 files changed, 325 insertions(+), 170 deletions(-)
 create mode 100644 src/ipa/ipu3/algorithms/black_correction.cpp
 create mode 100644 src/ipa/ipu3/algorithms/black_correction.h