[v3,10/39] libcamera: software_isp: Move Bayer params init from DebayerCpu to Debayer
diff mbox series

Message ID 20251015012251.17508-11-bryan.odonoghue@linaro.org
State New
Headers show
Series
  • Add GLES 2.0 GPUISP to libcamera
Related show

Commit Message

Bryan O'Donoghue Oct. 15, 2025, 1:22 a.m. UTC
Move the initialisation of Bayer params and CCM to a new constructor in the
Debayer class.

Ensure we call the base class constructor from DebayerCpu's constructor in
the expected constructor order Debayer then DebayerCpu.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 src/libcamera/software_isp/debayer.cpp     | 11 +++++++++++
 src/libcamera/software_isp/debayer.h       |  2 +-
 src/libcamera/software_isp/debayer_cpu.cpp |  8 --------
 3 files changed, 12 insertions(+), 9 deletions(-)

Comments

Kieran Bingham Oct. 15, 2025, 9:27 p.m. UTC | #1
Quoting Bryan O'Donoghue (2025-10-15 02:22:22)
> Move the initialisation of Bayer params and CCM to a new constructor in the
> Debayer class.
> 
> Ensure we call the base class constructor from DebayerCpu's constructor in
> the expected constructor order Debayer then DebayerCpu.
> 

Looks like a straight move.

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

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  src/libcamera/software_isp/debayer.cpp     | 11 +++++++++++
>  src/libcamera/software_isp/debayer.h       |  2 +-
>  src/libcamera/software_isp/debayer_cpu.cpp |  8 --------
>  3 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
> index e9e18c48..937d5e2e 100644
> --- a/src/libcamera/software_isp/debayer.cpp
> +++ b/src/libcamera/software_isp/debayer.cpp
> @@ -103,6 +103,17 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(Debayer)
>  
> +Debayer::Debayer(const GlobalConfiguration &configuration) : bench_(configuration)
> +{
> +       /* Initialize color lookup tables */
> +       for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
> +               red_[i] = green_[i] = blue_[i] = i;
> +               redCcm_[i] = { static_cast<int16_t>(i), 0, 0 };
> +               greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 };
> +               blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) };
> +       }
> +}
> +
>  Debayer::~Debayer()
>  {
>  }
> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
> index 1b195d29..2cbf0823 100644
> --- a/src/libcamera/software_isp/debayer.h
> +++ b/src/libcamera/software_isp/debayer.h
> @@ -33,7 +33,7 @@ LOG_DECLARE_CATEGORY(Debayer)
>  class Debayer : public Object
>  {
>  public:
> -       Debayer (const GlobalConfiguration &configuration) : bench_(configuration) {};
> +       Debayer (const GlobalConfiguration &configuration);
>         virtual ~Debayer() = 0;
>  
>         virtual int configure(const StreamConfiguration &inputCfg,
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index 612f01a6..42a0f374 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -57,14 +57,6 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfigurat
>          */
>         enableInputMemcpy_ =
>                 configuration.option<bool>({ "software_isp", "copy_input_buffer" }).value_or(true);
> -
> -       /* Initialize color lookup tables */
> -       for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
> -               red_[i] = green_[i] = blue_[i] = i;
> -               redCcm_[i] = { static_cast<int16_t>(i), 0, 0 };
> -               greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 };
> -               blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) };
> -       }
>  }
>  
>  DebayerCpu::~DebayerCpu() = default;
> -- 
> 2.51.0
>
Milan Zamazal Oct. 16, 2025, 8:17 a.m. UTC | #2
Hi Bryan,

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> Move the initialisation of Bayer params and CCM to a new constructor in the
> Debayer class.
>
> Ensure we call the base class constructor from DebayerCpu's constructor in
> the expected constructor order Debayer then DebayerCpu.

This is already ensured, isn't it?

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

It looks like it can be easily reverted once we decide to get rid of the
lookup tables, good.

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> ---
>  src/libcamera/software_isp/debayer.cpp     | 11 +++++++++++
>  src/libcamera/software_isp/debayer.h       |  2 +-
>  src/libcamera/software_isp/debayer_cpu.cpp |  8 --------
>  3 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
> index e9e18c48..937d5e2e 100644
> --- a/src/libcamera/software_isp/debayer.cpp
> +++ b/src/libcamera/software_isp/debayer.cpp
> @@ -103,6 +103,17 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(Debayer)
>  
> +Debayer::Debayer(const GlobalConfiguration &configuration) : bench_(configuration)
> +{
> +	/* Initialize color lookup tables */
> +	for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
> +		red_[i] = green_[i] = blue_[i] = i;
> +		redCcm_[i] = { static_cast<int16_t>(i), 0, 0 };
> +		greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 };
> +		blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) };
> +	}
> +}
> +
>  Debayer::~Debayer()
>  {
>  }
> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
> index 1b195d29..2cbf0823 100644
> --- a/src/libcamera/software_isp/debayer.h
> +++ b/src/libcamera/software_isp/debayer.h
> @@ -33,7 +33,7 @@ LOG_DECLARE_CATEGORY(Debayer)
>  class Debayer : public Object
>  {
>  public:
> -	Debayer (const GlobalConfiguration &configuration) : bench_(configuration) {};
> +	Debayer (const GlobalConfiguration &configuration);
>  	virtual ~Debayer() = 0;
>  
>  	virtual int configure(const StreamConfiguration &inputCfg,
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index 612f01a6..42a0f374 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -57,14 +57,6 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfigurat
>  	 */
>  	enableInputMemcpy_ =
>  		configuration.option<bool>({ "software_isp", "copy_input_buffer" }).value_or(true);
> -
> -	/* Initialize color lookup tables */
> -	for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
> -		red_[i] = green_[i] = blue_[i] = i;
> -		redCcm_[i] = { static_cast<int16_t>(i), 0, 0 };
> -		greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 };
> -		blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) };
> -	}
>  }
>  
>  DebayerCpu::~DebayerCpu() = default;

Patch
diff mbox series

diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
index e9e18c48..937d5e2e 100644
--- a/src/libcamera/software_isp/debayer.cpp
+++ b/src/libcamera/software_isp/debayer.cpp
@@ -103,6 +103,17 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(Debayer)
 
+Debayer::Debayer(const GlobalConfiguration &configuration) : bench_(configuration)
+{
+	/* Initialize color lookup tables */
+	for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
+		red_[i] = green_[i] = blue_[i] = i;
+		redCcm_[i] = { static_cast<int16_t>(i), 0, 0 };
+		greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 };
+		blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) };
+	}
+}
+
 Debayer::~Debayer()
 {
 }
diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
index 1b195d29..2cbf0823 100644
--- a/src/libcamera/software_isp/debayer.h
+++ b/src/libcamera/software_isp/debayer.h
@@ -33,7 +33,7 @@  LOG_DECLARE_CATEGORY(Debayer)
 class Debayer : public Object
 {
 public:
-	Debayer (const GlobalConfiguration &configuration) : bench_(configuration) {};
+	Debayer (const GlobalConfiguration &configuration);
 	virtual ~Debayer() = 0;
 
 	virtual int configure(const StreamConfiguration &inputCfg,
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index 612f01a6..42a0f374 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -57,14 +57,6 @@  DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfigurat
 	 */
 	enableInputMemcpy_ =
 		configuration.option<bool>({ "software_isp", "copy_input_buffer" }).value_or(true);
-
-	/* Initialize color lookup tables */
-	for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
-		red_[i] = green_[i] = blue_[i] = i;
-		redCcm_[i] = { static_cast<int16_t>(i), 0, 0 };
-		greenCcm_[i] = { 0, static_cast<int16_t>(i), 0 };
-		blueCcm_[i] = { 0, 0, static_cast<int16_t>(i) };
-	}
 }
 
 DebayerCpu::~DebayerCpu() = default;