[libcamera-devel,1/2] ipa: rpi: black_level: Add an initialValues method
diff mbox series

Message ID 20231206103839.16607-2-david.plowman@raspberrypi.com
State Accepted
Commit 8892d937c55ce2d62a5b0ee24c2b23b64366e73c
Headers show
Series
  • IPA initial values for Raspberry Pi
Related show

Commit Message

David Plowman Dec. 6, 2023, 10:38 a.m. UTC 
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@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

Comments

Naushir Patuck Dec. 7, 2023, 10:43 a.m. UTC | #1 
Hi David,

Thank you for your work.

On Wed, 6 Dec 2023 at 10:38, David Plowman via libcamera-devel
<libcamera-devel@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@raspberrypi.com>

Looks reasonable to me!

Reviewed-by: Naushir Patuck <naush@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
>

Patch
diff mbox series 

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: