[libcamera-devel] [PATCH 1/2] ipa: rpi: black_level: Add an initialValues method

[Naushir Patuck]

Hi David,

Thank you for your work.

On Wed, 6 Dec 2023 at 10:38, David Plowman via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> This allows the IPA to discover the correct black level values even
> before any frames have been processed. This is important on the PiSP
> platform where the front end black level blocks must be programmed in
> advance.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>

Looks reasonable to me!

Reviewed-by: Naushir Patuck <naush at raspberrypi.com>

> ---
>  .../rpi/controller/black_level_algorithm.h    | 23 +++++++++++++++++++
>  src/ipa/rpi/controller/rpi/black_level.cpp    | 10 +++++++-
>  src/ipa/rpi/controller/rpi/black_level.h      |  6 +++--
>  3 files changed, 36 insertions(+), 3 deletions(-)
>  create mode 100644 src/ipa/rpi/controller/black_level_algorithm.h
>
> diff --git a/src/ipa/rpi/controller/black_level_algorithm.h b/src/ipa/rpi/controller/black_level_algorithm.h
> new file mode 100644
> index 00000000..c2cff2f5
> --- /dev/null
> +++ b/src/ipa/rpi/controller/black_level_algorithm.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2023, Raspberry Pi Ltd
> + *
> + * black_level_algorithm.h - black level control algorithm interface
> + */
> +#pragma once
> +
> +#include "algorithm.h"
> +
> +namespace RPiController {
> +
> +class BlackLevelAlgorithm : public Algorithm
> +{
> +public:
> +       BlackLevelAlgorithm(Controller *controller)
> +               : Algorithm(controller) {}
> +       /* A black level algorithm must provide the following: */
> +       virtual void initialValues(uint16_t &blackLevelR, uint16_t &blackLevelG,
> +                                  uint16_t &blackLevelB) = 0;
> +};
> +
> +} /* namespace RPiController */
> diff --git a/src/ipa/rpi/controller/rpi/black_level.cpp b/src/ipa/rpi/controller/rpi/black_level.cpp
> index 85baec3f..2e3db51f 100644
> --- a/src/ipa/rpi/controller/rpi/black_level.cpp
> +++ b/src/ipa/rpi/controller/rpi/black_level.cpp
> @@ -22,7 +22,7 @@ LOG_DEFINE_CATEGORY(RPiBlackLevel)
>  #define NAME "rpi.black_level"
>
>  BlackLevel::BlackLevel(Controller *controller)
> -       : Algorithm(controller)
> +       : BlackLevelAlgorithm(controller)
>  {
>  }
>
> @@ -45,6 +45,14 @@ int BlackLevel::read(const libcamera::YamlObject &params)
>         return 0;
>  }
>
> +void BlackLevel::initialValues(uint16_t &blackLevelR, uint16_t &blackLevelG,
> +                              uint16_t &blackLevelB)
> +{
> +       blackLevelR = blackLevelR_;
> +       blackLevelG = blackLevelG_;
> +       blackLevelB = blackLevelB_;
> +}
> +
>  void BlackLevel::prepare(Metadata *imageMetadata)
>  {
>         /*
> diff --git a/src/ipa/rpi/controller/rpi/black_level.h b/src/ipa/rpi/controller/rpi/black_level.h
> index 2403f7f7..d8c41c62 100644
> --- a/src/ipa/rpi/controller/rpi/black_level.h
> +++ b/src/ipa/rpi/controller/rpi/black_level.h
> @@ -6,19 +6,21 @@
>   */
>  #pragma once
>
> -#include "../algorithm.h"
> +#include "../black_level_algorithm.h"
>  #include "../black_level_status.h"
>
>  /* This is our implementation of the "black level algorithm". */
>
>  namespace RPiController {
>
> -class BlackLevel : public Algorithm
> +class BlackLevel : public BlackLevelAlgorithm
>  {
>  public:
>         BlackLevel(Controller *controller);
>         char const *name() const override;
>         int read(const libcamera::YamlObject &params) override;
> +       void initialValues(uint16_t &blackLevelR, uint16_t &blackLevelG,
> +                          uint16_t &blackLevelB) override;
>         void prepare(Metadata *imageMetadata) override;
>
>  private:
> --
> 2.39.2
>