[libcamera-devel,v4,00/32] ipa: Frame context queue, IPU3 & RkISP consolidation, and RkISP1 improvements

17309 mbox series

Hello,

After lots of reshuffling of the "[PATCH v3 00/17] libcamera: Align IPU3
and RKISP1 interfaces" series, here's a v4 that has grown quite a bit on
the RkISP1 side.

I've taken over the previous version of the series as I thought it would
be easier and faster to provide review as fixup commits. I ended up
reshuffling modifications around, splitting and combining commits, not
so much in an attempt to clean or fix the history, but out of a desire
to make sure I correctly understood the changes. The result should
hopefully be easy to follow.

Patches 01/32 and 02/32 are small preliminary drive-by cleanups. The
gist of the frame context queue introduction is in patches 03/32 to
08/32 that also update the IPA algorithm operations to pass the new
objects around. There shouldn't be anything really new there, small
changes are listed in individual changelogs.

The next patches, from 09/32 to 11/32, port the IPU3 IPA module to the
FCQueue class. They should be similar to the implementation in v3 that
was spread within the patches that updated the IPA algorithm operations.
I have dropped the other changes to the IPU3 IPA module, not because
they were wrong, but because the series was growing big already. I will
rebase them on top of this v4 and submit them separately (unless someone
beats me to it).

Patches 12/32 to 16/32 then performs the same for the RkISP1 IPA module.
I have also dropped the lens-related patches from v3, as I wanted to
focus on the frame context queue first. Those patches will also be
rebased on top, they're not lost.

Patch 17/32 then renames the IPAFrameContext structures to
IPU3FrameContext and RkISP1FrameContext. This was part of v3, and I'm
not too fond of it to be honest, as the other structures don't have the
same naming scheme. I would prefer dropping this from the series and
then discussing the naming scheme of IPA structures globally.

The next 7 patches, from 18/32 to 24/32, port the RkISP1 algorithms to
the frame context API. 18/32 starts by dropping the frameCount member
variable of the active state as we can now use the frame number passed
to the algorithm operations, and the next patches port the algorithms
one by one, showcasing how data can be split between the active state
and frame context. Different schemes may be possible, maybe with less
duplication of member variables between the active state and frame
context. I'm particularly interested in feedback on this. The last patch
of this group, 24/32, documents the scheme used here.

Finally, patches 25/32 to 32/32 improve the AWB implementation of the
RkISP1 IPA module. Patch 25/32 in particular shows how the frame context
allows fixing a defect of the current implementation. The rest are
improvements that are detailed in the individual commit messages.

We don't need to merge this series in one go. It is organized as three
sets of patches (01/32 to 17/32 to introduce and use the frame context
queue, 18/32 to 24/32 to showcase its usage in the RkISP1 IPA module,
and 25/32 to 32/32 to improve the RkISP1 AWB). Each set is quite
self-contained, but they are based on each other in that order.

There is still work to do, in particular in the documentation, but I
didn't want to spend more time on it before getting an approval of the
general approach. Another point I'm not completely happy about is the
naming of the FCQueue functions, and in particular the init() function
(alloc() would already be better in my opinion, and I'd like to find an
even better name) but that could also possibly be fixed on top.

Jacopo Mondi (1):
  ipa: rkisp1: Remove unused class member

Kieran Bingham (7):
  ipa: libipa: Provide a common base for frame contexts
  ipa: libipa: algorithm: prepare(): Pass frame and frame Context
  ipa: libipa: algorithm: process(): Pass frame number
  ipa: libipa: algorithm: queueRequest(): Pass frame context
  ipa: rkisp1: Rename frameContext to activeState
  ipa: rkisp1: Convert to use the FCQueue
  ipa: Rename IPAFrameContext structures

Laurent Pinchart (22):
  ipa: ipu3: Fix style of Doxygen comment blocks
  ipa: ipu3: af: Pass context reference to afIsOutOfFocus()
  ipa: libipa: Pass a reference instead of pointer to
    Algorithm::process()
  ipa: ipu3: Use base FrameContext class
  ipa: ipu3: Use the FCQueue
  ipa: ipu3: Pass controls to algorithm's queueRequest() handler
  ipa: rkisp1: Sort documentation of the IPA context
  ipa: rkisp1: Use base FrameContext class
  ipa: rkisp1: Use frame number passed to Algorithm::prepare()
  ipa: rkisp1: agc: Store per-frame information in frame context
  ipa: rkisp1: awb: Store per-frame information in frame context
  ipa: rkisp1: cproc: Store per-frame information in frame context
  ipa: rkisp1: dpf: Store per-frame information in frame context
  ipa: rkisp1: filter: Store per-frame information in frame context
  ipa: rkisp1: Document the active state and frame context
  ipa: rkisp1: awb: Use frame context to fix gains calculations
  ipa: rkisp1: awb: Store color temperature as an integer
  ipa: rkisp1: awb: Log means, gains and temperature in debug message
  ipa: rkisp1: awb: Prevent RGB means from being negative
  ipa: rkisp1: awb: Clamp gains to prevent divisions by zero
  ipa: rkisp1: awb: Freeze AWB when means are too small
  ipa: rkisp1: awb: Remove bias from gain calculation

Quentin Schulz (1):
  ipa: rkisp1: awb: Add support for RGB means

Umang Jain (1):
  ipa: libipa: Introduce FrameContextQueue

 src/ipa/ipu3/algorithms/af.cpp           |  34 +--
 src/ipa/ipu3/algorithms/af.h             |   9 +-
 src/ipa/ipu3/algorithms/agc.cpp          |  10 +-
 src/ipa/ipu3/algorithms/agc.h            |   5 +-
 src/ipa/ipu3/algorithms/awb.cpp          |   8 +-
 src/ipa/ipu3/algorithms/awb.h            |   7 +-
 src/ipa/ipu3/algorithms/blc.cpp          |  10 +-
 src/ipa/ipu3/algorithms/blc.h            |   4 +-
 src/ipa/ipu3/algorithms/tone_mapping.cpp |  18 +-
 src/ipa/ipu3/algorithms/tone_mapping.h   |   6 +-
 src/ipa/ipu3/ipa_context.cpp             |  45 +---
 src/ipa/ipu3/ipa_context.h               |  18 +-
 src/ipa/ipu3/ipu3.cpp                    |  39 +++-
 src/ipa/ipu3/module.h                    |   2 +-
 src/ipa/libipa/algorithm.cpp             |   4 +
 src/ipa/libipa/algorithm.h               |   7 +-
 src/ipa/libipa/fc_queue.cpp              | 135 ++++++++++++
 src/ipa/libipa/fc_queue.h                | 118 ++++++++++
 src/ipa/libipa/meson.build               |   2 +
 src/ipa/rkisp1/algorithms/agc.cpp        |  36 +--
 src/ipa/rkisp1/algorithms/agc.h          |  10 +-
 src/ipa/rkisp1/algorithms/awb.cpp        | 269 ++++++++++++++++-------
 src/ipa/rkisp1/algorithms/awb.h          |  12 +-
 src/ipa/rkisp1/algorithms/blc.cpp        |   6 +-
 src/ipa/rkisp1/algorithms/blc.h          |   4 +-
 src/ipa/rkisp1/algorithms/cproc.cpp      |  52 +++--
 src/ipa/rkisp1/algorithms/cproc.h        |   5 +-
 src/ipa/rkisp1/algorithms/dpcc.cpp       |   6 +-
 src/ipa/rkisp1/algorithms/dpcc.h         |   4 +-
 src/ipa/rkisp1/algorithms/dpf.cpp        |  25 ++-
 src/ipa/rkisp1/algorithms/dpf.h          |   5 +-
 src/ipa/rkisp1/algorithms/filter.cpp     |  32 +--
 src/ipa/rkisp1/algorithms/filter.h       |   5 +-
 src/ipa/rkisp1/algorithms/gsl.cpp        |   6 +-
 src/ipa/rkisp1/algorithms/gsl.h          |   4 +-
 src/ipa/rkisp1/algorithms/lsc.cpp        |   5 +-
 src/ipa/rkisp1/algorithms/lsc.h          |   4 +-
 src/ipa/rkisp1/ipa_context.cpp           | 265 ++++++++++++++++------
 src/ipa/rkisp1/ipa_context.h             |  58 ++++-
 src/ipa/rkisp1/module.h                  |   2 +-
 src/ipa/rkisp1/rkisp1.cpp                |  52 +++--
 41 files changed, 982 insertions(+), 366 deletions(-)
 create mode 100644 src/ipa/libipa/fc_queue.cpp
 create mode 100644 src/ipa/libipa/fc_queue.h