[v2] libcamera: software_isp: debayer: Take `DebayerParams` by ref
diff mbox series

Message ID 20260205102156.2591211-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v2] libcamera: software_isp: debayer: Take `DebayerParams` by ref
Related show

Commit Message

Barnabás Pőcze Feb. 5, 2026, 10:21 a.m. UTC 
When calling `Debayer::process()` from `SoftwareIsp::process()`, the
`DebayerParams` object is copied multiple times:

  (1) call of `BoundMethodMember<...>::activate()`
      inside `Object::invokeMethod()`
  (2) constructor of `BoundMethodArgs<...>`
      inside `BoundMethodMember<...>::activate()`
  (3) call of `BoundMethodMember<...>::invoke()`
      inside `BoundMethodArgs::invokePack()`
  (4) call of the actual pointer to member function
      inside `BoundMethodMember::invoke()`

While compilers might avoid one or two of the above copies, this is still
not ideal. By making `Debayer::process()` take the parameter object by
const lvalue reference, only the copy in the `BoundMethodArgs` constructor
remains. So do that.

Before:
	[0:12:51.133836595] [12424] DEBUG SoftwareIsp software_isp.cpp:399 params=0x7d0a691f57d0
	copy from 0x7d0a691f57d0 into 0x7baa65f2bf30
	copy from 0x7baa65f2bf30 into 0x7c6a69209758
	copy from 0x7c6a69209758 into 0x7baa63223930
	copy from 0x7baa63223930 into 0x7baa63223a70
	[0:12:51.134559602] [12426] DEBUG eGL debayer_egl.cpp:538 params=0x7baa63223a70
	771.099877 (30.06 fps) cam0-stream0 seq: 000031 bytesused: 8666112

After:
	[0:13:42.861691943] [12543] DEBUG SoftwareIsp software_isp.cpp:399 params=0x7cfaad5f57d0
	copy from 0x7cfaad5f57d0 into 0x7c5aad609758
	[0:13:42.862453917] [12545] DEBUG eGL debayer_egl.cpp:538 params=0x7c5aad609758
	822.827388 (30.02 fps) cam0-stream0 seq: 000031 bytesused: 8666112

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
changes in v2:
  * rebase
  * reword and extend commit message

v1: https://patchwork.libcamera.org/patch/25898/
---
 src/libcamera/software_isp/debayer.h       | 2 +-
 src/libcamera/software_isp/debayer_cpu.cpp | 6 +++---
 src/libcamera/software_isp/debayer_cpu.h   | 6 +++---
 src/libcamera/software_isp/debayer_egl.cpp | 6 +++---
 src/libcamera/software_isp/debayer_egl.h   | 6 +++---
 5 files changed, 13 insertions(+), 13 deletions(-)

--
2.53.0

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
index 652cff4cc..ce9350b7b 100644
--- a/src/libcamera/software_isp/debayer.h
+++ b/src/libcamera/software_isp/debayer.h
@@ -47,7 +47,7 @@  public:
 	virtual std::tuple<unsigned int, unsigned int>
 	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0;

-	virtual void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params) = 0;
+	virtual void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, const DebayerParams &params) = 0;
 	virtual int start() { return 0; }
 	virtual void stop() {}

diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index af7af0a8d..d09883577 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -750,7 +750,7 @@  void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst)
 	}
 }

-void DebayerCpu::updateGammaTable(DebayerParams &params)
+void DebayerCpu::updateGammaTable(const DebayerParams &params)
 {
 	const RGB<float> blackLevel = params.blackLevel;
 	/* Take let's say the green channel black level */
@@ -780,7 +780,7 @@  void DebayerCpu::updateGammaTable(DebayerParams &params)
 		  gammaTable_[blackIndex]);
 }

-void DebayerCpu::updateLookupTables(DebayerParams &params)
+void DebayerCpu::updateLookupTables(const DebayerParams &params)
 {
 	const bool gammaUpdateNeeded =
 		params.gamma != params_.gamma ||
@@ -842,7 +842,7 @@  void DebayerCpu::updateLookupTables(DebayerParams &params)
 	params_ = params;
 }

-void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params)
+void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, const DebayerParams &params)
 {
 	bench_.startFrame();

diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
index b5cbb5bd2..7a6517462 100644
--- a/src/libcamera/software_isp/debayer_cpu.h
+++ b/src/libcamera/software_isp/debayer_cpu.h
@@ -39,7 +39,7 @@  public:
 	std::vector<PixelFormat> formats(PixelFormat input);
 	std::tuple<unsigned int, unsigned int>
 	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size);
-	void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params);
+	void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, const DebayerParams &params);
 	SizeRange sizes(PixelFormat inputFormat, const Size &inputSize);
 	const SharedFD &getStatsFD() { return stats_->getStatsFD(); }

@@ -110,8 +110,8 @@  private:
 	void memcpyNextLine(const uint8_t *linePointers[]);
 	void process2(uint32_t frame, const uint8_t *src, uint8_t *dst);
 	void process4(uint32_t frame, const uint8_t *src, uint8_t *dst);
-	void updateGammaTable(DebayerParams &params);
-	void updateLookupTables(DebayerParams &params);
+	void updateGammaTable(const DebayerParams &params);
+	void updateLookupTables(const DebayerParams &params);

 	/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */
 	static constexpr unsigned int kMaxLineBuffers = 5;
diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
index 4d0caa6ef..93f7c6946 100644
--- a/src/libcamera/software_isp/debayer_egl.cpp
+++ b/src/libcamera/software_isp/debayer_egl.cpp
@@ -383,7 +383,7 @@  DebayerEGL::strideAndFrameSize(const PixelFormat &outputFormat, const Size &size
 	return std::make_tuple(stride, stride * size.height);
 }

-void DebayerEGL::setShaderVariableValues(DebayerParams &params)
+void DebayerEGL::setShaderVariableValues(const DebayerParams &params)
 {
 	/*
 	 * Raw Bayer 8-bit, and packed raw Bayer 10-bit/12-bit formats
@@ -506,7 +506,7 @@  void DebayerEGL::setShaderVariableValues(DebayerParams &params)
 	return;
 }

-int DebayerEGL::debayerGPU(MappedFrameBuffer &in, int out_fd, DebayerParams &params)
+int DebayerEGL::debayerGPU(MappedFrameBuffer &in, int out_fd, const DebayerParams &params)
 {
 	/* eGL context switch */
 	egl_.makeCurrent();
@@ -533,7 +533,7 @@  int DebayerEGL::debayerGPU(MappedFrameBuffer &in, int out_fd, DebayerParams &par
 	return 0;
 }

-void DebayerEGL::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params)
+void DebayerEGL::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, const DebayerParams &params)
 {
 	bench_.startFrame();

diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
index cfbaf8e9d..59aebcc81 100644
--- a/src/libcamera/software_isp/debayer_egl.h
+++ b/src/libcamera/software_isp/debayer_egl.h
@@ -50,7 +50,7 @@  public:
 	std::vector<PixelFormat> formats(PixelFormat input);
 	std::tuple<unsigned int, unsigned int> strideAndFrameSize(const PixelFormat &outputFormat, const Size &size);

-	void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params);
+	void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, const DebayerParams &params);
 	int start();
 	void stop();

@@ -72,9 +72,9 @@  private:
 				 std::vector<std::string> shaderEnv);
 	int linkShaderProgram(void);
 	int getShaderVariableLocations();
-	void setShaderVariableValues(DebayerParams &params);
+	void setShaderVariableValues(const DebayerParams &params);
 	void configureTexture(GLuint &texture);
-	int debayerGPU(MappedFrameBuffer &in, int out_fd, DebayerParams &params);
+	int debayerGPU(MappedFrameBuffer &in, int out_fd, const DebayerParams &params);

 	/* Shader program identifiers */
 	GLuint vertexShaderId_ = 0;