[PATCH 00/10] Centralise Agc into libipa

[Stefan Klug]

Hi Daniel,

On Fri, Mar 22, 2024 at 01:14:41PM +0000, Daniel Scally wrote:
> Hello all
> 
> The IPU3 and RkISP1 IPAs do Agc the same way (following the Rpi method closely),
> but the implementations are wholly separate. This introduces the potential for
> divergence and also makes both maintenance and improvement more difficult. This
> series unifies them as derivations of a new MeanLuminanceAgc class within
> libipa. The algorithm itself is done through functions of the base class
> with the IPA's derived classes providing a function through which the mean
> luminance input to the algorithm can be calculated from their specific stats
> formats.
> 
> The old implementations effectively hard coded values for the AeConstraintMode
> and AeExposureMode controls; they adhered to a single lower-bound constraint of
> 0.5 across the top 2% of a Histogram and acted as though the AeExposureMode
> expected shutter time to be driven to its maximum before touching gain at all.
> The base class additionally adds infrastructure to discover the supported values
> for AeConstraintMode and AeExposureMode controls from a camera sensor tuning
> file and uses the values defined in those files with the algorithm instead,
> though as no tuning files are modified in this series the behaviour currently
> remains as it was before.
> 
> The series **does** alter the calculated shutter time and gain for both the
> IPU3 and RkISP1 compared to their bespoke implementations, for two reasons:
> 
> 1. A bug in the way they were implemented meant that an over-exposed image was
>    corrected more slowly than an under-exposed one. This is fixed and will
>    improve both IPAs' response to a too-bright image.
> 2. The default kRelativeLuminanceTarget is centrally set to 0.16 which matches
>    the value from the IPU3 implementation. In the RkISP1 implementation that
>    value was set to 0.4 with a \todo to see why they were different. My belief
>    is that they were different because they were implemented in different
>    lighting conditions. In my very imperfect testing setup the 0.16 target
>    produced reasonable images on both.

Just a wild guess (and the numbers fit nicely). This might be dependent on
whether gamma is included in the calculation or not. A value of 0.16
corresponds to 0.4 for gamma=2.

Cheers,
Stefan

> 
> Without those two changes, the shutter time and gain calculated after this
> series matches those calculated by their independent implementations.
> 
> Thanks
> Dan
> 
> Daniel Scally (9):
>   ipa: libipa: Allow creation of empty Histogram
>   libcamera: controls: Generate enum value-name maps
>   ipa: libipa: Add MeanLuminanceAgc base class
>   ipa: ipu3: Parse and store Agc stats
>   ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc
>   ipa: ipu3: Remove bespoke AGC functions from IPU3
>   ipa: rkisp1: Add parseStatistics() to Agc
>   ipa: rkisp1: Derive rkisp1::algorithms::Agc from MeanLuminanceAgc
>   ipa: rkisp1: Remove bespoke Agc functions
> 
> Paul Elder (1):
>   ipa: libipa: Add ExposureModeHelper
> 
>  include/libcamera/control_ids.h.in      |   2 +
>  include/libcamera/property_ids.h.in     |   2 +
>  src/ipa/ipu3/algorithms/agc.cpp         | 306 ++++----------
>  src/ipa/ipu3/algorithms/agc.h           |  27 +-
>  src/ipa/ipu3/ipa_context.cpp            |   3 +
>  src/ipa/ipu3/ipa_context.h              |   5 +
>  src/ipa/ipu3/ipu3.cpp                   |   2 +-
>  src/ipa/libipa/agc.cpp                  | 526 ++++++++++++++++++++++++
>  src/ipa/libipa/agc.h                    |  82 ++++
>  src/ipa/libipa/exposure_mode_helper.cpp | 307 ++++++++++++++
>  src/ipa/libipa/exposure_mode_helper.h   |  61 +++
>  src/ipa/libipa/histogram.h              |   1 +
>  src/ipa/libipa/meson.build              |   4 +
>  src/ipa/rkisp1/algorithms/agc.cpp       | 303 ++++----------
>  src/ipa/rkisp1/algorithms/agc.h         |  19 +-
>  src/ipa/rkisp1/ipa_context.h            |   5 +
>  src/ipa/rkisp1/rkisp1.cpp               |   2 +-
>  utils/gen-controls.py                   |  19 +
>  18 files changed, 1219 insertions(+), 457 deletions(-)
>  create mode 100644 src/ipa/libipa/agc.cpp
>  create mode 100644 src/ipa/libipa/agc.h
>  create mode 100644 src/ipa/libipa/exposure_mode_helper.cpp
>  create mode 100644 src/ipa/libipa/exposure_mode_helper.h
> 
> -- 
> 2.34.1
>