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

17411 mbox series

Hello,

Here's the v5 of the "frame context queue" series. It looks very much
like v4, with review comments taken into account, and a few additional
changes:

- The rename of IPAFrameContext to IPU3FrameContext and
  RkISP1FrameContext got dropped. I'm not convinced it's a good idea,
  and while I could change my mind, I think we should then rename all
  the other structures accordingly in one go.
- Patches 17/33 and 18/33 have been added. They integrate review
  comments from v4, and I've kept them separate to ease review and
  discussions.

Patches 01/33 and 02/33 are small preliminary drive-by cleanups. The
gist of the frame context queue introduction is in patches 03/33 to
08/33 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/33 to 11/33, 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/33 to 16/33 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.

As mentioned above, the next two patches are new in v5. Patch 17/33
passes the FCQueue size as a template argument instead of a constructor
argument. I've kept it separate as I'm not entirely sure it's the right
option, I have a feeling we may have use cases for computing the queue
size at runtime instead of compile time, and using a template argument
doesn't bring that much as far as I can tell. Patch 18/33 disables
copy-construction of the frame contexts, which shouldn't be
controversial.

The next 7 patches, from 19/33 to 25/34, port the RkISP1 algorithms to
the frame context API. 19/33 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, 25/33, documents the scheme used here.

Finally, patches 26/33 to 33/33 improve the AWB implementation of the
RkISP1 IPA module. Patch 26/33 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/33 to 18/33 to introduce and use the frame context
queue, 19/33 to 25/33 to showcase its usage in the RkISP1 IPA module,
and 26/33 to 33/33 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 how
the FCQueue alloc() and get() functions handle underruns, and actually
how the whole underrun mechanism will be implemented. We need to
experiment on top of this series to figure it out.

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

Kieran Bingham (6):
  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

Laurent Pinchart (24):
  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: libipa: Pass FCQueue size as template argument
  ipa: Disable copy-construction of IPAContext
  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             |  37 +---
 src/ipa/ipu3/ipa_context.h               |  25 +--
 src/ipa/ipu3/ipu3.cpp                    |  37 ++--
 src/ipa/libipa/algorithm.cpp             |   4 +
 src/ipa/libipa/algorithm.h               |   7 +-
 src/ipa/libipa/fc_queue.cpp              | 140 ++++++++++++
 src/ipa/libipa/fc_queue.h                | 117 ++++++++++
 src/ipa/libipa/meson.build               |   2 +
 src/ipa/rkisp1/algorithms/agc.cpp        |  46 ++--
 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        |  33 +--
 src/ipa/rkisp1/algorithms/dpf.h          |   5 +-
 src/ipa/rkisp1/algorithms/filter.cpp     |  49 +++--
 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           | 219 +++++++++++++-----
 src/ipa/rkisp1/ipa_context.h             |  64 +++++-
 src/ipa/rkisp1/rkisp1.cpp                |  55 +++--
 39 files changed, 998 insertions(+), 345 deletions(-)
 create mode 100644 src/ipa/libipa/fc_queue.cpp
 create mode 100644 src/ipa/libipa/fc_queue.h