[PATCH v5 3/4] libcamera: software_isp: Add support for contrast control

Kieran Bingham kieran.bingham at ideasonboard.com
Fri Dec 6 01:22:58 CET 2024


Quoting Milan Zamazal (2024-11-28 12:52:24)
> Software ISP is currently fully automatic and doesn't allow image
> modifications by explicitly set control values.  The user has no means
> to make the image looking better.
> 
> This patch introduces support for contrast control, which can improve
> e.g. a flat looking image.  Based on the provided contrast value, it
> applies a simple S-curve modification to the image.
> 
> The contrast algorithm just handles the provided values, while the
> S-curve is applied in the gamma algorithm on the computed gamma curve
> whenever the contrast value changes.  Since the algorithm is applied
> only on the lookup table already present, its overhead is negligible.
> 
> The contrast value range is 0..2 and corresponds to the whole range from
> a completely flat contrast to an infinite contrast, 1.0 being the normal
> value.  This makes the user visible range intuitive and easy to use in
> GUI sliders, while complying with Contrast control definition.  There is
> no unified range in the hardware pipelines, for example rkisp1 uses
> 0..1.993 range while rpi uses 0..10 range.
> 
> This is a preparation patch without actually providing the control
> itself, which is done in the following patch.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>

Well, I've just tested this - and it does indeed affect the image, and
respond correctly to a slider control in camshark ... so that's progress
for the soft-isp from my perspective, and I know more will be built on
top of this:

Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  src/ipa/simple/algorithms/lut.cpp | 40 +++++++++++++++++++++++++++----
>  src/ipa/simple/algorithms/lut.h   |  5 ++++
>  src/ipa/simple/ipa_context.h      |  7 ++++++
>  3 files changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp
> index 9744e773..dd76e117 100644
> --- a/src/ipa/simple/algorithms/lut.cpp
> +++ b/src/ipa/simple/algorithms/lut.cpp
> @@ -9,14 +9,19 @@
>  
>  #include <algorithm>
>  #include <cmath>
> +#include <optional>
>  #include <stdint.h>
>  
>  #include <libcamera/base/log.h>
>  
>  #include "simple/ipa_context.h"
>  
> +#include "control_ids.h"
> +
>  namespace libcamera {
>  
> +LOG_DEFINE_CATEGORY(IPASoftLut)
> +
>  namespace ipa::soft::algorithms {
>  
>  int Lut::configure(IPAContext &context,
> @@ -24,24 +29,48 @@ int Lut::configure(IPAContext &context,
>  {
>         /* Gamma value is fixed */
>         context.configuration.gamma = 0.5;
> +       context.activeState.knobs.contrast = std::optional<double>();
>         updateGammaTable(context);
>  
>         return 0;
>  }
>  
> +void Lut::queueRequest(typename Module::Context &context,
> +                      [[maybe_unused]] const uint32_t frame,
> +                      [[maybe_unused]] typename Module::FrameContext &frameContext,
> +                      const ControlList &controls)
> +{
> +       const auto &contrast = controls.get(controls::Contrast);
> +       if (contrast.has_value()) {
> +               context.activeState.knobs.contrast = contrast;
> +               LOG(IPASoftLut, Debug) << "Setting contrast to " << contrast.value();
> +       }
> +}
> +
>  void Lut::updateGammaTable(IPAContext &context)
>  {
>         auto &gammaTable = context.activeState.gamma.gammaTable;
> -       auto blackLevel = context.activeState.blc.level;
> +       const auto blackLevel = context.activeState.blc.level;
>         const unsigned int blackIndex = blackLevel * gammaTable.size() / 256;
> +       const auto contrast = context.activeState.knobs.contrast.value_or(1.0);
>  
>         std::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);
>         const float divisor = gammaTable.size() - blackIndex - 1.0;
> -       for (unsigned int i = blackIndex; i < gammaTable.size(); i++)
> -               gammaTable[i] = UINT8_MAX * std::pow((i - blackIndex) / divisor,
> -                                                    context.configuration.gamma);
> +       for (unsigned int i = blackIndex; i < gammaTable.size(); i++) {
> +               double normalized = (i - blackIndex) / divisor;
> +               /* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */
> +               double contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));
> +               /* Apply simple S-curve */
> +               if (normalized < 0.5)
> +                       normalized = 0.5 * std::pow(normalized / 0.5, contrastExp);
> +               else
> +                       normalized = 1.0 - 0.5 * std::pow((1.0 - normalized) / 0.5, contrastExp);
> +               gammaTable[i] = UINT8_MAX *
> +                               std::pow(normalized, context.configuration.gamma);
> +       }
>  
>         context.activeState.gamma.blackLevel = blackLevel;
> +       context.activeState.gamma.contrast = contrast;
>  }
>  
>  void Lut::prepare(IPAContext &context,
> @@ -55,7 +84,8 @@ void Lut::prepare(IPAContext &context,
>          * observed, it's not permanently prone to minor fluctuations or
>          * rounding errors.
>          */
> -       if (context.activeState.gamma.blackLevel != context.activeState.blc.level)
> +       if (context.activeState.gamma.blackLevel != context.activeState.blc.level ||
> +           context.activeState.gamma.contrast != context.activeState.knobs.contrast)
>                 updateGammaTable(context);
>  
>         auto &gains = context.activeState.gains;
> diff --git a/src/ipa/simple/algorithms/lut.h b/src/ipa/simple/algorithms/lut.h
> index b635987d..ef2df147 100644
> --- a/src/ipa/simple/algorithms/lut.h
> +++ b/src/ipa/simple/algorithms/lut.h
> @@ -20,6 +20,11 @@ public:
>         ~Lut() = default;
>  
>         int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> +       void queueRequest(typename Module::Context &context,
> +                         const uint32_t frame,
> +                         typename Module::FrameContext &frameContext,
> +                         const ControlList &controls)
> +               override;
>         void prepare(IPAContext &context,
>                      const uint32_t frame,
>                      IPAFrameContext &frameContext,
> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
> index fd7343e9..0c2f7021 100644
> --- a/src/ipa/simple/ipa_context.h
> +++ b/src/ipa/simple/ipa_context.h
> @@ -11,6 +11,8 @@
>  #include <optional>
>  #include <stdint.h>
>  
> +#include <libcamera/controls.h>
> +
>  #include <libipa/fc_queue.h>
>  
>  namespace libcamera {
> @@ -48,7 +50,12 @@ struct IPAActiveState {
>         struct {
>                 std::array<double, kGammaLookupSize> gammaTable;
>                 uint8_t blackLevel;
> +               double contrast;
>         } gamma;
> +       struct {
> +               /* 0..2 range, 1.0 = normal */
> +               std::optional<double> contrast;
> +       } knobs;
>  };
>  
>  struct IPAFrameContext : public FrameContext {
> -- 
> 2.44.2
>


More information about the libcamera-devel mailing list