[v3,8/8] libcamera: software_isp: debayer_egl: Implement input/output frame caching mechanism
diff mbox series

Message ID 20260626113325.3218045-9-bryan.odonoghue@linaro.org
State Superseded
Headers show
Series
  • libcamera: software_isp: gpu: Add go faster stripes
Related show

Commit Message

Bryan O'Donoghue June 26, 2026, 11:33 a.m. UTC 
Implement a texture caching mechanism for both input and output frames and
for both types of input frame.

The before/after on a Qualcomm x1e is:

9.737ms per frame
5.691ms per frame

The before/after on a Qualcomm sm8250 is:

21.710ms per frame
17.336ms per frame

for i in {1..20} do
cam -c /base/soc@0/cci@ac16000/i2c-bus@1/camera@10 -s width=1920,height=1080 --capture=60

Interestingly there appears to be an absolute ~ 4.x ms per frame uplift as
opposed to what intuition might suggest a proportional.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 src/libcamera/software_isp/debayer_egl.cpp | 87 +++++++++++++++++-----
 src/libcamera/software_isp/debayer_egl.h   | 10 ++-
 2 files changed, 75 insertions(+), 22 deletions(-)

Comments

Robert Mader June 26, 2026, 12:29 p.m. UTC | #1 
On 26.06.26 13:33, Bryan O'Donoghue wrote:
> Implement a texture caching mechanism for both input and output frames and
> for both types of input frame.
>
> The before/after on a Qualcomm x1e is:
>
> 9.737ms per frame
> 5.691ms per frame
>
> The before/after on a Qualcomm sm8250 is:
>
> 21.710ms per frame
> 17.336ms per frame
>
> for i in {1..20} do
> cam -c /base/soc@0/cci@ac16000/i2c-bus@1/camera@10 -s width=1920,height=1080 --capture=60
>
> Interestingly there appears to be an absolute ~ 4.x ms per frame uplift as
> opposed to what intuition might suggest a proportional.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   src/libcamera/software_isp/debayer_egl.cpp | 87 +++++++++++++++++-----
>   src/libcamera/software_isp/debayer_egl.h   | 10 ++-
>   2 files changed, 75 insertions(+), 22 deletions(-)
>
> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
> index 53bb67c17..fc37f0b75 100644
> --- a/src/libcamera/software_isp/debayer_egl.cpp
> +++ b/src/libcamera/software_isp/debayer_egl.cpp
> @@ -355,6 +355,9 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg,
>   	 */
>   	stats_->setWindow(Rectangle(window_.size()));
>   
> +	inputBufferCount_ = inputCfg.bufferCount;
> +	outputBufferCount_ = outputCfg.bufferCount;
> +
>   	return 0;
>   }
>   
> @@ -514,34 +517,84 @@ void DebayerEGL::setShaderVariableValues(eGLImage &eglImageIn, const DebayerPara
>   	return;
>   }
>   
> -int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
> +eGLImage *DebayerEGL::getCachedInputFrameBuffer(FrameBuffer *input, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
>   {
> -	/* eGL context switch */
> -	egl_.makeCurrent();
> +	auto [input_cache, cache_miss] = eglImageBayerIn_.try_emplace(input->planes()[0].fd.get());
> +	if (cache_miss) {
> +		if (eglImageBayerIn_.size() > inputBufferCount_) {
> +			LOG(Debayer, Error) << "Input count " << inputBufferCount_ << " exhausted";
> +			return nullptr;
> +		}
> +		input_cache->second = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_,
> +								 height_, inputConfig_.stride, GL_TEXTURE0, 0);
> +	}
> +	eGLImage *eglImageIn = input_cache->second.get();
>   
>   	/* Try to create texture for input buffer via dmabuf import */
> -	if (use_dmabuf_) {
> -		if (egl_.createInputDMABufTexture2D(*eglImageBayerIn_, input->planes()[0].fd.get()) != 0) {
> +	if (use_dmabuf_ && cache_miss) {
> +		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
>   			use_dmabuf_ = false;
>   			LOG(Debayer, Info) << "Importing input buffer with DMABuf import failed, falling back to upload";
>   		}
>   	}
>   
> -	/* Otherwise create texture for input buffer via upload from CPU */
> -	if (!use_dmabuf_) {
> +	if (use_dmabuf_) {
> +		/* Cache hit using dmabuf activate and bind */
> +		if (!cache_miss)
> +			egl_.activateBindTexture(*eglImageIn);
> +	} else {
> +		/* Otherwise create texture for input buffer via upload from CPU */
>   		inDmaSyncer->emplace(input->planes()[0].fd, DmaSyncer::SyncType::Read);
>   		inMapped->emplace(input, MappedFrameBuffer::MapFlag::Read);
>   		if (!inMapped->value().isValid()) {
>   			LOG(Debayer, Error) << "mmap-ing buffer(s) failed";
> -			return -ENODEV;
> +			return nullptr;
>   		}
> -		egl_.createTexture2D(*eglImageBayerIn_, inMapped->value().planes()[0].data());
> +		if (cache_miss)
> +			egl_.createTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
> +		else
> +			egl_.updateTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
>   	}
>   
> -	/* Generate the output render framebuffer as render to texture */
> -	egl_.createOutputDMABufTexture2D(*eglImageBayerOut_, output->planes()[0].fd.get());
> +	return eglImageIn;
> +}
> +
> +eGLImage *DebayerEGL::getCachedOutputFrameBuffer(FrameBuffer *output)
> +{
> +	auto [output_cache, cache_miss] = eglImageBayerOut_.try_emplace(output->planes()[0].fd.get());
> +	if (cache_miss) {
> +		if (eglImageBayerOut_.size() > outputBufferCount_) {
> +			LOG(Debayer, Error) << "Output buffer count " << outputBufferCount_ << " exhaustion";
> +			return nullptr;
> +		}
> +		output_cache->second = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width,
> +								  outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1);
> +		egl_.createOutputDMABufTexture2D(*output_cache->second, output->planes()[0].fd.get());
> +	}
Answering to my own question in the previous review: IIUC we indeed 
don't need to call "egl_.activateBindTexture(*eglImageBayerOut_);" in 
the "else" case here, it's apparently only required when setting up the 
texture. Hope that's correct.
> +	eGLImage *eglImageOut = output_cache->second.get();
> +
> +	return eglImageOut;
> +}
> +
> +int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
> +{
> +	eGLImage *eglImageIn;
> +	eGLImage *eglImageOut;
> +
> +	/* eGL context switch */
> +	egl_.makeCurrent();
> +
> +	eglImageIn = getCachedInputFrameBuffer(input, inMapped, inDmaSyncer);
> +	if (!eglImageIn)
> +		return -ENOMEM;
> +
> +	eglImageOut = getCachedOutputFrameBuffer(output);
> +	if (!eglImageOut)
> +		return -ENOMEM;
> +
> +	egl_.attachTextureToFBO(*eglImageOut);
> +	setShaderVariableValues(*eglImageIn, params);
>   
> -	setShaderVariableValues(*eglImageBayerIn_, params);
>   	glViewport(0, 0, width_, height_);
>   	glClear(GL_COLOR_BUFFER_BIT);
>   	glDrawArrays(GL_TRIANGLE_FAN, 0, DEBAYER_OPENGL_COORDS);
> @@ -623,19 +676,13 @@ int DebayerEGL::start()
>   	if (initBayerShaders(inputPixelFormat_, outputPixelFormat_))
>   		return -EINVAL;
>   
> -	/* Raw bayer input as texture */
> -	eglImageBayerIn_ = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_, height_, inputConfig_.stride, GL_TEXTURE0, 0);
> -
> -	/* Texture we will render to */
> -	eglImageBayerOut_ = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width, outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1);
> -
>   	return 0;
>   }
>   
>   void DebayerEGL::stop()
>   {
> -	eglImageBayerOut_.reset();
> -	eglImageBayerIn_.reset();
> +	eglImageBayerOut_.clear();
> +	eglImageBayerIn_.clear();
Should we also reset inputBufferCount_ and outputBufferCount_ here, just 
to ensure the state is clean and can't accidentally be used in the future?
>   
>   	if (programId_)
>   		glDeleteProgram(programId_);
> diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
> index d8509e9f2..ddb3ef378 100644
> --- a/src/libcamera/software_isp/debayer_egl.h
> +++ b/src/libcamera/software_isp/debayer_egl.h
> @@ -22,6 +22,7 @@
>   #include "libcamera/internal/mapped_framebuffer.h"
>   #include "libcamera/internal/software_isp/benchmark.h"
>   #include "libcamera/internal/software_isp/swstats_cpu.h"
> +#include "libcamera/internal/v4l2_videodevice.h"
Should be removed again I think?
>   
>   #include <EGL/egl.h>
>   #include <EGL/eglext.h>
> @@ -70,14 +71,19 @@ private:
>   
>   	bool use_dmabuf_;
>   
> +	eGLImage *getCachedInputFrameBuffer(FrameBuffer *input, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer);
> +	eGLImage *getCachedOutputFrameBuffer(FrameBuffer *output);
> +
>   	/* Shader program identifiers */
>   	GLuint vertexShaderId_ = 0;
>   	GLuint fragmentShaderId_ = 0;
>   	GLuint programId_ = 0;
>   
>   	/* Pointer to object representing input texture */
> -	std::unique_ptr<eGLImage> eglImageBayerIn_;
> -	std::unique_ptr<eGLImage> eglImageBayerOut_;
> +	std::unordered_map<int, std::unique_ptr<eGLImage>> eglImageBayerIn_;
> +	std::unordered_map<int, std::unique_ptr<eGLImage>> eglImageBayerOut_;
> +	unsigned int inputBufferCount_;
> +	unsigned int outputBufferCount_;
>   
>   	/* Shader parameters */
>   	float firstRed_x_;

This approach LGTM - with the two minor commits addressed:

Reviewed-by: Robert Mader <robert.mader@collabora.com>
Barnabás Pőcze June 26, 2026, 1:19 p.m. UTC | #2 
2026. 06. 26. 13:33 keltezéssel, Bryan O'Donoghue írta:
> Implement a texture caching mechanism for both input and output frames and
> for both types of input frame.
> 
> The before/after on a Qualcomm x1e is:
> 
> 9.737ms per frame
> 5.691ms per frame
> 
> The before/after on a Qualcomm sm8250 is:
> 
> 21.710ms per frame
> 17.336ms per frame
> 
> for i in {1..20} do
> cam -c /base/soc@0/cci@ac16000/i2c-bus@1/camera@10 -s width=1920,height=1080 --capture=60
> 
> Interestingly there appears to be an absolute ~ 4.x ms per frame uplift as
> opposed to what intuition might suggest a proportional.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   src/libcamera/software_isp/debayer_egl.cpp | 87 +++++++++++++++++-----
>   src/libcamera/software_isp/debayer_egl.h   | 10 ++-
>   2 files changed, 75 insertions(+), 22 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
> index 53bb67c17..fc37f0b75 100644
> --- a/src/libcamera/software_isp/debayer_egl.cpp
> +++ b/src/libcamera/software_isp/debayer_egl.cpp
> @@ -355,6 +355,9 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg,
>   	 */
>   	stats_->setWindow(Rectangle(window_.size()));
> 
> +	inputBufferCount_ = inputCfg.bufferCount;
> +	outputBufferCount_ = outputCfg.bufferCount;
> +
>   	return 0;
>   }
> 
> @@ -514,34 +517,84 @@ void DebayerEGL::setShaderVariableValues(eGLImage &eglImageIn, const DebayerPara
>   	return;
>   }
> 
> -int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
> +eGLImage *DebayerEGL::getCachedInputFrameBuffer(FrameBuffer *input, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
>   {
> -	/* eGL context switch */
> -	egl_.makeCurrent();
> +	auto [input_cache, cache_miss] = eglImageBayerIn_.try_emplace(input->planes()[0].fd.get());
> +	if (cache_miss) {
> +		if (eglImageBayerIn_.size() > inputBufferCount_) {
> +			LOG(Debayer, Error) << "Input count " << inputBufferCount_ << " exhausted";

   eglImageBayerIn_.erase(input_cache);

otherwise the stale entry (nullptr) might cause issues later.


> +			return nullptr;
> +		}
> +		input_cache->second = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_,
> +								 height_, inputConfig_.stride, GL_TEXTURE0, 0);
> +	}
> +	eGLImage *eglImageIn = input_cache->second.get();
> 
>   	/* Try to create texture for input buffer via dmabuf import */
> -	if (use_dmabuf_) {
> -		if (egl_.createInputDMABufTexture2D(*eglImageBayerIn_, input->planes()[0].fd.get()) != 0) {
> +	if (use_dmabuf_ && cache_miss) {
> +		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
>   			use_dmabuf_ = false;
>   			LOG(Debayer, Info) << "Importing input buffer with DMABuf import failed, falling back to upload";
>   		}
>   	}
> 
> -	/* Otherwise create texture for input buffer via upload from CPU */
> -	if (!use_dmabuf_) {
> +	if (use_dmabuf_) {
> +		/* Cache hit using dmabuf activate and bind */
> +		if (!cache_miss)
> +			egl_.activateBindTexture(*eglImageIn);
> +	} else {
> +		/* Otherwise create texture for input buffer via upload from CPU */
>   		inDmaSyncer->emplace(input->planes()[0].fd, DmaSyncer::SyncType::Read);
>   		inMapped->emplace(input, MappedFrameBuffer::MapFlag::Read);
>   		if (!inMapped->value().isValid()) {
>   			LOG(Debayer, Error) << "mmap-ing buffer(s) failed";
> -			return -ENODEV;
> +			return nullptr;
>   		}
> -		egl_.createTexture2D(*eglImageBayerIn_, inMapped->value().planes()[0].data());
> +		if (cache_miss)
> +			egl_.createTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
> +		else
> +			egl_.updateTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
>   	}
> 
> -	/* Generate the output render framebuffer as render to texture */
> -	egl_.createOutputDMABufTexture2D(*eglImageBayerOut_, output->planes()[0].fd.get());
> +	return eglImageIn;
> +}
> +
> +eGLImage *DebayerEGL::getCachedOutputFrameBuffer(FrameBuffer *output)
> +{
> +	auto [output_cache, cache_miss] = eglImageBayerOut_.try_emplace(output->planes()[0].fd.get());
> +	if (cache_miss) {
> +		if (eglImageBayerOut_.size() > outputBufferCount_) {
> +			LOG(Debayer, Error) << "Output buffer count " << outputBufferCount_ << " exhaustion";

   eglImageBayerOut_.erase(output_cache);

for the same reason as above.


> +			return nullptr;
> +		}
> +		output_cache->second = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width,
> +								  outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1);
> +		egl_.createOutputDMABufTexture2D(*output_cache->second, output->planes()[0].fd.get());
> +	}
> +	eGLImage *eglImageOut = output_cache->second.get();
> +
> +	return eglImageOut;

   return output_cache->second.get();

?


> +}
> +
> +int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
> +{
> +	eGLImage *eglImageIn;
> +	eGLImage *eglImageOut;
> +
> +	/* eGL context switch */
> +	egl_.makeCurrent();
> +
> +	eglImageIn = getCachedInputFrameBuffer(input, inMapped, inDmaSyncer);
> +	if (!eglImageIn)
> +		return -ENOMEM;
> +
> +	eglImageOut = getCachedOutputFrameBuffer(output);
> +	if (!eglImageOut)
> +		return -ENOMEM;
> +
> +	egl_.attachTextureToFBO(*eglImageOut);
> +	setShaderVariableValues(*eglImageIn, params);
> 
> -	setShaderVariableValues(*eglImageBayerIn_, params);
>   	glViewport(0, 0, width_, height_);
>   	glClear(GL_COLOR_BUFFER_BIT);
>   	glDrawArrays(GL_TRIANGLE_FAN, 0, DEBAYER_OPENGL_COORDS);
> @@ -623,19 +676,13 @@ int DebayerEGL::start()
>   	if (initBayerShaders(inputPixelFormat_, outputPixelFormat_))
>   		return -EINVAL;
> 
> -	/* Raw bayer input as texture */
> -	eglImageBayerIn_ = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_, height_, inputConfig_.stride, GL_TEXTURE0, 0);
> -
> -	/* Texture we will render to */
> -	eglImageBayerOut_ = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width, outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1);
> -
>   	return 0;
>   }
> 
>   void DebayerEGL::stop()
>   {
> -	eglImageBayerOut_.reset();
> -	eglImageBayerIn_.reset();
> +	eglImageBayerOut_.clear();
> +	eglImageBayerIn_.clear();
> 
>   	if (programId_)
>   		glDeleteProgram(programId_);
> diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
> index d8509e9f2..ddb3ef378 100644
> --- a/src/libcamera/software_isp/debayer_egl.h
> +++ b/src/libcamera/software_isp/debayer_egl.h
> @@ -22,6 +22,7 @@
>   #include "libcamera/internal/mapped_framebuffer.h"
>   #include "libcamera/internal/software_isp/benchmark.h"
>   #include "libcamera/internal/software_isp/swstats_cpu.h"
> +#include "libcamera/internal/v4l2_videodevice.h"
> 
>   #include <EGL/egl.h>
>   #include <EGL/eglext.h>
> @@ -70,14 +71,19 @@ private:
> 
>   	bool use_dmabuf_;
> 
> +	eGLImage *getCachedInputFrameBuffer(FrameBuffer *input, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer);
> +	eGLImage *getCachedOutputFrameBuffer(FrameBuffer *output);
> +
>   	/* Shader program identifiers */
>   	GLuint vertexShaderId_ = 0;
>   	GLuint fragmentShaderId_ = 0;
>   	GLuint programId_ = 0;
> 
>   	/* Pointer to object representing input texture */
> -	std::unique_ptr<eGLImage> eglImageBayerIn_;
> -	std::unique_ptr<eGLImage> eglImageBayerOut_;
> +	std::unordered_map<int, std::unique_ptr<eGLImage>> eglImageBayerIn_;
> +	std::unordered_map<int, std::unique_ptr<eGLImage>> eglImageBayerOut_;

I think we should use `SharedFD` here to avoid any fd reuse issues. I have just
realized that `V4l2BufferCache` does not use `SharedFD` either. And I think
that is kind of an issue. Well, nowadays every user just uses `FrameBufferAllocator`
with long lived `FrameBuffer`s, but the libcamera api does not require anything
like that.

Maybe this should be discussed more, what the exact schemantics should be, etc.

In any case, this should be an easy change, just needs this as well:

diff --git a/include/libcamera/base/shared_fd.h b/include/libcamera/base/shared_fd.h
index 61fe11c1d6..cec1f6036c 100644
--- a/include/libcamera/base/shared_fd.h
+++ b/include/libcamera/base/shared_fd.h
@@ -7,6 +7,7 @@
  
  #pragma once
  
+#include <functional>
  #include <memory>
  
  namespace libcamera {
@@ -57,3 +58,11 @@ static inline bool operator!=(const SharedFD &lhs, const SharedFD &rhs)
  }
  
  } /* namespace libcamera */
+
+template<>
+struct std::hash<libcamera::SharedFD> : private std::hash<int> {
+       auto operator()(const libcamera::SharedFD &x) const noexcept
+       {
+               return std::hash<int>::operator()(x.get());
+       }
+};


> +	unsigned int inputBufferCount_;
> +	unsigned int outputBufferCount_;

Something like `xyzBufferCacheMaxSize_` / `maxXyzBufferCacheSize_` is more to the point in my opinion.

> 
>   	/* Shader parameters */
>   	float firstRed_x_;
> --
> 2.54.0
>
Barnabás Pőcze June 26, 2026, 1:22 p.m. UTC | #3 
2026. 06. 26. 15:19 keltezéssel, Barnabás Pőcze írta:
> 2026. 06. 26. 13:33 keltezéssel, Bryan O'Donoghue írta:
>> Implement a texture caching mechanism for both input and output frames and
>> for both types of input frame.
>>
>> The before/after on a Qualcomm x1e is:
>>
>> 9.737ms per frame
>> 5.691ms per frame
>>
>> The before/after on a Qualcomm sm8250 is:
>>
>> 21.710ms per frame
>> 17.336ms per frame
>>
>> for i in {1..20} do
>> cam -c /base/soc@0/cci@ac16000/i2c-bus@1/camera@10 -s width=1920,height=1080 --capture=60
>>
>> Interestingly there appears to be an absolute ~ 4.x ms per frame uplift as
>> opposed to what intuition might suggest a proportional.
>>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>    src/libcamera/software_isp/debayer_egl.cpp | 87 +++++++++++++++++-----
>>    src/libcamera/software_isp/debayer_egl.h   | 10 ++-
>>    2 files changed, 75 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
>> index 53bb67c17..fc37f0b75 100644
>> --- a/src/libcamera/software_isp/debayer_egl.cpp
>> +++ b/src/libcamera/software_isp/debayer_egl.cpp
>> @@ -355,6 +355,9 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg,
> [...]
>> +eGLImage *DebayerEGL::getCachedInputFrameBuffer(FrameBuffer *input, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
>>    {
>> -	/* eGL context switch */
>> -	egl_.makeCurrent();
>> +	auto [input_cache, cache_miss] = eglImageBayerIn_.try_emplace(input->planes()[0].fd.get());
>> +	if (cache_miss) {
>> +		if (eglImageBayerIn_.size() > inputBufferCount_) {
>> +			LOG(Debayer, Error) << "Input count " << inputBufferCount_ << " exhausted";

And I am not sure this should be an error, maybe "debug" at most.


> 
>     eglImageBayerIn_.erase(input_cache);
> 
> otherwise the stale entry (nullptr) might cause issues later.
> 
> 
>> +			return nullptr;
>> +		}
>> +		input_cache->second = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_,
>> +								 height_, inputConfig_.stride, GL_TEXTURE0, 0);
>> +	}
>> +	eGLImage *eglImageIn = input_cache->second.get();
>>
>>    	/* Try to create texture for input buffer via dmabuf import */
>> -	if (use_dmabuf_) {
>> -		if (egl_.createInputDMABufTexture2D(*eglImageBayerIn_, input->planes()[0].fd.get()) != 0) {
>> +	if (use_dmabuf_ && cache_miss) {
>> +		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
>>    			use_dmabuf_ = false;
>>    			LOG(Debayer, Info) << "Importing input buffer with DMABuf import failed, falling back to upload";
>>    		}
>>    	}
>>
>> -	/* Otherwise create texture for input buffer via upload from CPU */
>> -	if (!use_dmabuf_) {
>> +	if (use_dmabuf_) {
>> +		/* Cache hit using dmabuf activate and bind */
>> +		if (!cache_miss)
>> +			egl_.activateBindTexture(*eglImageIn);
>> +	} else {
>> +		/* Otherwise create texture for input buffer via upload from CPU */
>>    		inDmaSyncer->emplace(input->planes()[0].fd, DmaSyncer::SyncType::Read);
>>    		inMapped->emplace(input, MappedFrameBuffer::MapFlag::Read);
>>    		if (!inMapped->value().isValid()) {
>>    			LOG(Debayer, Error) << "mmap-ing buffer(s) failed";
>> -			return -ENODEV;
>> +			return nullptr;
>>    		}
>> -		egl_.createTexture2D(*eglImageBayerIn_, inMapped->value().planes()[0].data());
>> +		if (cache_miss)
>> +			egl_.createTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
>> +		else
>> +			egl_.updateTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
>>    	}
>>
>> -	/* Generate the output render framebuffer as render to texture */
>> -	egl_.createOutputDMABufTexture2D(*eglImageBayerOut_, output->planes()[0].fd.get());
>> +	return eglImageIn;
>> +}
>> [...]
Bryan O'Donoghue June 26, 2026, 2:10 p.m. UTC | #4 
On 26/06/2026 14:19, Barnabás Pőcze wrote:
> diff --git a/include/libcamera/base/shared_fd.h b/include/libcamera/ 
> base/shared_fd.h
> index 61fe11c1d6..cec1f6036c 100644
> --- a/include/libcamera/base/shared_fd.h
> +++ b/include/libcamera/base/shared_fd.h
> @@ -7,6 +7,7 @@
> 
>   #pragma once
> 
> +#include <functional>
>   #include <memory>
> 
>   namespace libcamera {
> @@ -57,3 +58,11 @@ static inline bool operator!=(const SharedFD &lhs, 
> const SharedFD &rhs)
>   }
> 
>   } /* namespace libcamera */
> +
> +template<>
> +struct std::hash<libcamera::SharedFD> : private std::hash<int> {
> +       auto operator()(const libcamera::SharedFD &x) const noexcept
> +       {
> +               return std::hash<int>::operator()(x.get());
> +       }
> +};

Do you want to make that into a patch yourself or for me to pick it up 
for you ?

---
bod
Barnabás Pőcze June 26, 2026, 3:01 p.m. UTC | #5 
2026. 06. 26. 16:10 keltezéssel, Bryan O'Donoghue írta:
> On 26/06/2026 14:19, Barnabás Pőcze wrote:
>> diff --git a/include/libcamera/base/shared_fd.h b/include/libcamera/ base/shared_fd.h
>> index 61fe11c1d6..cec1f6036c 100644
>> --- a/include/libcamera/base/shared_fd.h
>> +++ b/include/libcamera/base/shared_fd.h
>> @@ -7,6 +7,7 @@
>>
>>   #pragma once
>>
>> +#include <functional>
>>   #include <memory>
>>
>>   namespace libcamera {
>> @@ -57,3 +58,11 @@ static inline bool operator!=(const SharedFD &lhs, const SharedFD &rhs)
>>   }
>>
>>   } /* namespace libcamera */
>> +
>> +template<>
>> +struct std::hash<libcamera::SharedFD> : private std::hash<int> {
>> +       auto operator()(const libcamera::SharedFD &x) const noexcept
>> +       {
>> +               return std::hash<int>::operator()(x.get());
>> +       }
>> +};
> 
> Do you want to make that into a patch yourself or for me to pick it up for you ?

Please include it if you send a new version with `SharedFD` as the map key.


> 
> ---
> bod
Barnabás Pőcze June 30, 2026, 10:16 a.m. UTC | #6 
2026. 06. 26. 13:33 keltezéssel, Bryan O'Donoghue írta:
> Implement a texture caching mechanism for both input and output frames and
> for both types of input frame.
> 
> The before/after on a Qualcomm x1e is:
> 
> 9.737ms per frame
> 5.691ms per frame
> 
> The before/after on a Qualcomm sm8250 is:
> 
> 21.710ms per frame
> 17.336ms per frame
> 
> for i in {1..20} do
> cam -c /base/soc@0/cci@ac16000/i2c-bus@1/camera@10 -s width=1920,height=1080 --capture=60
> 
> Interestingly there appears to be an absolute ~ 4.x ms per frame uplift as
> opposed to what intuition might suggest a proportional.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   src/libcamera/software_isp/debayer_egl.cpp | 87 +++++++++++++++++-----
>   src/libcamera/software_isp/debayer_egl.h   | 10 ++-
>   2 files changed, 75 insertions(+), 22 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
> index 53bb67c17..fc37f0b75 100644
> --- a/src/libcamera/software_isp/debayer_egl.cpp
> +++ b/src/libcamera/software_isp/debayer_egl.cpp
> @@ -355,6 +355,9 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg,
> [...]
> -int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
> +eGLImage *DebayerEGL::getCachedInputFrameBuffer(FrameBuffer *input, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
>   {
> -	/* eGL context switch */
> -	egl_.makeCurrent();
> +	auto [input_cache, cache_miss] = eglImageBayerIn_.try_emplace(input->planes()[0].fd.get());

With a custom cache like here, there needs some kind of eviction policy.
Just like `V4L2BufferCache` does a kind of LRU eviction.


> +	if (cache_miss) {
> +		if (eglImageBayerIn_.size() > inputBufferCount_) {
> +			LOG(Debayer, Error) << "Input count " << inputBufferCount_ << " exhausted";
> +			return nullptr;
> +		}
> +		input_cache->second = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_,
> +								 height_, inputConfig_.stride, GL_TEXTURE0, 0);
> +	}
> +	eGLImage *eglImageIn = input_cache->second.get();
>   
>   	/* Try to create texture for input buffer via dmabuf import */
> -	if (use_dmabuf_) {
> -		if (egl_.createInputDMABufTexture2D(*eglImageBayerIn_, input->planes()[0].fd.get()) != 0) {
> +	if (use_dmabuf_ && cache_miss) {
> +		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
>   			use_dmabuf_ = false;

What if some buffers were imported successfully already? Can that cause
issue later when `use_dmabuf_ == false`?


>   			LOG(Debayer, Info) << "Importing input buffer with DMABuf import failed, falling back to upload";
>   		}
>   	}
>   
> -	/* Otherwise create texture for input buffer via upload from CPU */
> -	if (!use_dmabuf_) {
> +	if (use_dmabuf_) {
> +		/* Cache hit using dmabuf activate and bind */
> +		if (!cache_miss)
> +			egl_.activateBindTexture(*eglImageIn);
> +	} else {
> +		/* Otherwise create texture for input buffer via upload from CPU */
>   		inDmaSyncer->emplace(input->planes()[0].fd, DmaSyncer::SyncType::Read);
>   		inMapped->emplace(input, MappedFrameBuffer::MapFlag::Read);
>   		if (!inMapped->value().isValid()) {
>   			LOG(Debayer, Error) << "mmap-ing buffer(s) failed";
> -			return -ENODEV;
> +			return nullptr;
>   		}
> -		egl_.createTexture2D(*eglImageBayerIn_, inMapped->value().planes()[0].data());
> +		if (cache_miss)
> +			egl_.createTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
> +		else
> +			egl_.updateTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
>   	}
>   
> -	/* Generate the output render framebuffer as render to texture */
> -	egl_.createOutputDMABufTexture2D(*eglImageBayerOut_, output->planes()[0].fd.get());
> +	return eglImageIn;
> +}
> [...]
Bryan O'Donoghue July 1, 2026, 8:35 a.m. UTC | #7 
On 30/06/2026 11:16, Barnabás Pőcze wrote:
>> +	if (use_dmabuf_ && cache_miss) {
>> +		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
>>    			use_dmabuf_ = false;
> What if some buffers were imported successfully already? Can that cause
> issue later when `use_dmabuf_ == false`?

For this specific case - its all or nothing.

There's no reason to support some input buffers using dma-buf handle and 
some not - that's indicative of a fundamental bug - all of the buffers 
come from the same place and pertain to framebuffers of the same size.

I'll include this check if you _really_ want it but if encountered it is 
certain "error out and die screaming" not "be chill and accept"

---
bod
Robert Mader July 1, 2026, 8:39 a.m. UTC | #8 
On 01.07.26 10:35, Bryan O'Donoghue wrote:
> On 30/06/2026 11:16, Barnabás Pőcze wrote:
>>> +	if (use_dmabuf_ && cache_miss) {
>>> +		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
>>>     			use_dmabuf_ = false;
>> What if some buffers were imported successfully already? Can that cause
>> issue later when `use_dmabuf_ == false`?
> For this specific case - its all or nothing.
>
> There's no reason to support some input buffers using dma-buf handle and
> some not - that's indicative of a fundamental bug - all of the buffers
> come from the same place and pertain to framebuffers of the same size.
>
> I'll include this check if you _really_ want it but if encountered it is
> certain "error out and die screaming" not "be chill and accept"
>
> ---
> bod
Fully agree, that's how things are usually handled in Wayland as well 
(if the compositor suddenly can't import a buffer from a dmabuf client 
any more).
Barnabás Pőcze July 1, 2026, 9:06 a.m. UTC | #9 
2026. 07. 01. 10:35 keltezéssel, Bryan O'Donoghue írta:
> On 30/06/2026 11:16, Barnabás Pőcze wrote:
>>> +	if (use_dmabuf_ && cache_miss) {
>>> +		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
>>>     			use_dmabuf_ = false;
>> What if some buffers were imported successfully already? Can that cause
>> issue later when `use_dmabuf_ == false`?
> 
> For this specific case - its all or nothing.
> 
> There's no reason to support some input buffers using dma-buf handle and
> some not - that's indicative of a fundamental bug - all of the buffers
> come from the same place and pertain to framebuffers of the same size.

But my point is that this is not necessarily the case. An application can create and
use its own `FrameBuffer`s, without ever touching a `FrameBufferAllocator`; so they
don't necessarily come from the same place. Arguably this is not a commonly
used thing today, but the API allows this, and it should be handled accordingly.
E.g. if you look at the android component, you'll see it has its own
`HALFrameBuffer` thing, it does not use `libcamera::FrameBufferAllocator` at all.


> 
> I'll include this check if you _really_ want it but if encountered it is
> certain "error out and die screaming" not "be chill and accept"
> 
> ---
> bod
>
Bryan O'Donoghue July 1, 2026, 9:55 a.m. UTC | #10 
On 01/07/2026 10:06, Barnabás Pőcze wrote:
>>> What if some buffers were imported successfully already? Can that cause
>>> issue later when `use_dmabuf_ == false`?
>>
>> For this specific case - its all or nothing.
>>
>> There's no reason to support some input buffers using dma-buf handle and
>> some not - that's indicative of a fundamental bug - all of the buffers
>> come from the same place and pertain to framebuffers of the same size.
> 
> But my point is that this is not necessarily the case. An application 
> can create and
> use its own `FrameBuffer`s, without ever touching a 
> `FrameBufferAllocator`; so they
> don't necessarily come from the same place. Arguably this is not a commonly
> used thing today, but the API allows this, and it should be handled 
> accordingly.
> E.g. if you look at the android component, you'll see it has its own
> `HALFrameBuffer` thing, it does not use 
> `libcamera::FrameBufferAllocator` at all.

Hmm.

In this case we will need to stick to evaluating each eglImage instead 
of having a policy per session, my idea about promoting (demoting?) the 
logic to debayer_egl from eglImage is not a flier.

---
bod
Robert Mader July 1, 2026, 11:40 a.m. UTC | #11 
On 01.07.26 11:55, Bryan O'Donoghue wrote:
> On 01/07/2026 10:06, Barnabás Pőcze wrote:
>>>> What if some buffers were imported successfully already? Can that 
>>>> cause
>>>> issue later when `use_dmabuf_ == false`?
>>>
>>> For this specific case - its all or nothing.
>>>
>>> There's no reason to support some input buffers using dma-buf handle 
>>> and
>>> some not - that's indicative of a fundamental bug - all of the buffers
>>> come from the same place and pertain to framebuffers of the same size.
>>
>> But my point is that this is not necessarily the case. An application 
>> can create and
>> use its own `FrameBuffer`s, without ever touching a 
>> `FrameBufferAllocator`; so they
>> don't necessarily come from the same place. Arguably this is not a 
>> commonly
>> used thing today, but the API allows this, and it should be handled 
>> accordingly.
>> E.g. if you look at the android component, you'll see it has its own
>> `HALFrameBuffer` thing, it does not use 
>> `libcamera::FrameBufferAllocator` at all.
>
> Hmm.
>
> In this case we will need to stick to evaluating each eglImage instead 
> of having a policy per session, my idea about promoting (demoting?) 
> the logic to debayer_egl from eglImage is not a flier.
>
> ---
> bod

Even though the API allows it, would it be acceptable if we explicitly 
disallow it in debayer_egl? I.e. something along the lines of

		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
+
+			if (eglImageBayerOut_.size() > 1)
+				LOG(Debayer, Info) << "Importing input buffer with DMABuf import failed after previously succeeding. This is likely caused by mixing incompatible input buffers and not currently supported.";
+				return nullptr;
+			}
+
			use_dmabuf_ = false;
			LOG(Debayer, Info) << "Importing input buffer with DMABuf import failed, falling back to upload";
		}
Laurent Pinchart July 1, 2026, 11:50 a.m. UTC | #12 
On Wed, Jul 01, 2026 at 01:40:38PM +0200, Robert Mader wrote:
> 
> On 01.07.26 11:55, Bryan O'Donoghue wrote:
> > On 01/07/2026 10:06, Barnabás Pőcze wrote:
> >>>> What if some buffers were imported successfully already? Can that 
> >>>> cause
> >>>> issue later when `use_dmabuf_ == false`?
> >>>
> >>> For this specific case - its all or nothing.
> >>>
> >>> There's no reason to support some input buffers using dma-buf handle 
> >>> and
> >>> some not - that's indicative of a fundamental bug - all of the buffers
> >>> come from the same place and pertain to framebuffers of the same size.
> >>
> >> But my point is that this is not necessarily the case. An application 
> >> can create and
> >> use its own `FrameBuffer`s, without ever touching a 
> >> `FrameBufferAllocator`; so they
> >> don't necessarily come from the same place. Arguably this is not a 
> >> commonly
> >> used thing today, but the API allows this, and it should be handled 
> >> accordingly.
> >> E.g. if you look at the android component, you'll see it has its own
> >> `HALFrameBuffer` thing, it does not use 
> >> `libcamera::FrameBufferAllocator` at all.
> >
> > Hmm.
> >
> > In this case we will need to stick to evaluating each eglImage instead 
> > of having a policy per session, my idea about promoting (demoting?) 
> > the logic to debayer_egl from eglImage is not a flier.
> >
> > ---
> > bod
> 
> Even though the API allows it, would it be acceptable if we explicitly 
> disallow it in debayer_egl? I.e. something along the lines of

Nice try, but sorry :-) We're accumulated a lot of technical debt in the
soft ISP already, I'm increasingly feeling uncomfortable adding more
without addressing the existing one.

> 		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
> +
> +			if (eglImageBayerOut_.size() > 1)
> +				LOG(Debayer, Info) << "Importing input buffer with DMABuf import failed after previously succeeding. This is likely caused by mixing incompatible input buffers and not currently supported.";
> +				return nullptr;
> +			}
> +
> 			use_dmabuf_ = false;
> 			LOG(Debayer, Info) << "Importing input buffer with DMABuf import failed, falling back to upload";
> 		}
Robert Mader July 1, 2026, 11:56 a.m. UTC | #13 
On 01.07.26 13:50, Laurent Pinchart wrote:
> On Wed, Jul 01, 2026 at 01:40:38PM +0200, Robert Mader wrote:
>> On 01.07.26 11:55, Bryan O'Donoghue wrote:
>>> On 01/07/2026 10:06, Barnabás Pőcze wrote:
>>>>>> What if some buffers were imported successfully already? Can that
>>>>>> cause
>>>>>> issue later when `use_dmabuf_ == false`?
>>>>> For this specific case - its all or nothing.
>>>>>
>>>>> There's no reason to support some input buffers using dma-buf handle
>>>>> and
>>>>> some not - that's indicative of a fundamental bug - all of the buffers
>>>>> come from the same place and pertain to framebuffers of the same size.
>>>> But my point is that this is not necessarily the case. An application
>>>> can create and
>>>> use its own `FrameBuffer`s, without ever touching a
>>>> `FrameBufferAllocator`; so they
>>>> don't necessarily come from the same place. Arguably this is not a
>>>> commonly
>>>> used thing today, but the API allows this, and it should be handled
>>>> accordingly.
>>>> E.g. if you look at the android component, you'll see it has its own
>>>> `HALFrameBuffer` thing, it does not use
>>>> `libcamera::FrameBufferAllocator` at all.
>>> Hmm.
>>>
>>> In this case we will need to stick to evaluating each eglImage instead
>>> of having a policy per session, my idea about promoting (demoting?)
>>> the logic to debayer_egl from eglImage is not a flier.
>>>
>>> ---
>>> bod
>> Even though the API allows it, would it be acceptable if we explicitly
>> disallow it in debayer_egl? I.e. something along the lines of
> Nice try, but sorry :-) We're accumulated a lot of technical debt in the
> soft ISP already, I'm increasingly feeling uncomfortable adding more
> without addressing the existing one.
I see. In that case I guess we can simply keep using 
eglImageIn->dmabuf_import_failed_ - accepting the negligible overhead 
for the 3 additional failing imports in the common cases - and should 
probably ensure that the "Importing input buffer with DMABuf import 
failed, falling back to upload" log is only printed once. Shouldn't be a 
big deal AFAICS.
Bryan O'Donoghue July 1, 2026, 3:20 p.m. UTC | #14 
On 01/07/2026 12:56, Robert Mader wrote:
>> Nice try, but sorry 🙂 We're accumulated a lot of technical debt in the
>> soft ISP already, I'm increasingly feeling uncomfortable adding more
>> without addressing the existing one.
> I see. In that case I guess we can simply keep using
> eglImageIn->dmabuf_import_failed_ - accepting the negligible overhead
> for the 3 additional failing imports in the common cases - and should
> probably ensure that the "Importing input buffer with DMABuf import
> failed, falling back to upload" log is only printed once. Shouldn't be a
> big deal AFAICS.

Yes but - even though we are told @ configure() time of how many buffers 
to keep in the cache - I'm told those buffers can change.

The dma-buf handle must be unique but the application can feed in its 
own buffer => import on a per-frame as opposed to per-session policy 
should be supported.

We also agreed if we allocated space for four buffers and then we see a 
new fifth buffer to evict the oldest buffer in the cache.

---
bod
Robert Mader July 1, 2026, 3:34 p.m. UTC | #15 
On 01.07.26 17:20, Bryan O'Donoghue wrote:
> On 01/07/2026 12:56, Robert Mader wrote:
>>> Nice try, but sorry 🙂 We're accumulated a lot of technical debt in the
>>> soft ISP already, I'm increasingly feeling uncomfortable adding more
>>> without addressing the existing one.
>> I see. In that case I guess we can simply keep using
>> eglImageIn->dmabuf_import_failed_ - accepting the negligible overhead
>> for the 3 additional failing imports in the common cases - and should
>> probably ensure that the "Importing input buffer with DMABuf import
>> failed, falling back to upload" log is only printed once. Shouldn't be a
>> big deal AFAICS.
> Yes but - even though we are told @ configure() time of how many buffers
> to keep in the cache - I'm told those buffers can change.
>
> The dma-buf handle must be unique but the application can feed in its
> own buffer => import on a per-frame as opposed to per-session policy
> should be supported.
>
> We also agreed if we allocated space for four buffers and then we see a
> new fifth buffer to evict the oldest buffer in the cache.
Yes - AFAICS for each unique FrameBuffer (or rather: unique dmabuf 
handle within that FrameBuffer) we already create a unique eGLImage. So 
using eGLImage::dmabuf_import_failed_ ensures that for each FrameBuffer 
we try the dmabuf import exactly once.
Robert Mader July 4, 2026, 9:46 a.m. UTC | #16 
Hey hey, I just took the freedom to try to address the feedback from the 
last series / apply all the requested changes and IMO it went quite 
well. I pushed it here:

https://gitlab.freedesktop.org/rmader/libcamera/-/tree/swisp-dmabuf-import-v3-fixes

Please feel free to pick that up for v4 if you agree with the changes or 
change things again as you prefer. Note that I also did a slight 
reordering in getCachedInputFrameBuffer(), IMO making it easier to follow.

CI succeeds apart from one doxygen issue around the new SharedFD hash 
(need to investigate further, apparently enabling BUILTIN_STL_SUPPORT 
should fix it)

On 01.07.26 17:34, Robert Mader wrote:
> On 01.07.26 17:20, Bryan O'Donoghue wrote:
>> On 01/07/2026 12:56, Robert Mader wrote:
>>>> Nice try, but sorry 🙂 We're accumulated a lot of technical debt in 
>>>> the
>>>> soft ISP already, I'm increasingly feeling uncomfortable adding more
>>>> without addressing the existing one.
>>> I see. In that case I guess we can simply keep using
>>> eglImageIn->dmabuf_import_failed_ - accepting the negligible overhead
>>> for the 3 additional failing imports in the common cases - and should
>>> probably ensure that the "Importing input buffer with DMABuf import
>>> failed, falling back to upload" log is only printed once. Shouldn't 
>>> be a
>>> big deal AFAICS.
>> Yes but - even though we are told @ configure() time of how many buffers
>> to keep in the cache - I'm told those buffers can change.
>>
>> The dma-buf handle must be unique but the application can feed in its
>> own buffer => import on a per-frame as opposed to per-session policy
>> should be supported.
>>
>> We also agreed if we allocated space for four buffers and then we see a
>> new fifth buffer to evict the oldest buffer in the cache.
> Yes - AFAICS for each unique FrameBuffer (or rather: unique dmabuf 
> handle within that FrameBuffer) we already create a unique eGLImage. 
> So using eGLImage::dmabuf_import_failed_ ensures that for each 
> FrameBuffer we try the dmabuf import exactly once.
>
Robert Mader July 5, 2026, 11:49 a.m. UTC | #17 
On 04.07.26 11:46, Robert Mader wrote:
> Hey hey, I just took the freedom to try to address the feedback from 
> the last series / apply all the requested changes and IMO it went 
> quite well. I pushed it here:
>
> https://gitlab.freedesktop.org/rmader/libcamera/-/tree/swisp-dmabuf-import-v3-fixes 
>
>
> Please feel free to pick that up for v4 if you agree with the changes 
> or change things again as you prefer. Note that I also did a slight 
> reordering in getCachedInputFrameBuffer(), IMO making it easier to 
> follow.
>
> CI succeeds apart from one doxygen issue around the new SharedFD hash 
> (need to investigate further, apparently enabling BUILTIN_STL_SUPPORT 
> should fix it)
Yep, it does. Added and pushed, together with another small fix. CI 
succeeds now and I won't touch the branch any more.

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
index 53bb67c17..fc37f0b75 100644
--- a/src/libcamera/software_isp/debayer_egl.cpp
+++ b/src/libcamera/software_isp/debayer_egl.cpp
@@ -355,6 +355,9 @@  int DebayerEGL::configure(const StreamConfiguration &inputCfg,
 	 */
 	stats_->setWindow(Rectangle(window_.size()));
 
+	inputBufferCount_ = inputCfg.bufferCount;
+	outputBufferCount_ = outputCfg.bufferCount;
+
 	return 0;
 }
 
@@ -514,34 +517,84 @@  void DebayerEGL::setShaderVariableValues(eGLImage &eglImageIn, const DebayerPara
 	return;
 }
 
-int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
+eGLImage *DebayerEGL::getCachedInputFrameBuffer(FrameBuffer *input, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
 {
-	/* eGL context switch */
-	egl_.makeCurrent();
+	auto [input_cache, cache_miss] = eglImageBayerIn_.try_emplace(input->planes()[0].fd.get());
+	if (cache_miss) {
+		if (eglImageBayerIn_.size() > inputBufferCount_) {
+			LOG(Debayer, Error) << "Input count " << inputBufferCount_ << " exhausted";
+			return nullptr;
+		}
+		input_cache->second = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_,
+								 height_, inputConfig_.stride, GL_TEXTURE0, 0);
+	}
+	eGLImage *eglImageIn = input_cache->second.get();
 
 	/* Try to create texture for input buffer via dmabuf import */
-	if (use_dmabuf_) {
-		if (egl_.createInputDMABufTexture2D(*eglImageBayerIn_, input->planes()[0].fd.get()) != 0) {
+	if (use_dmabuf_ && cache_miss) {
+		if (egl_.createInputDMABufTexture2D(*eglImageIn, input->planes()[0].fd.get()) != 0) {
 			use_dmabuf_ = false;
 			LOG(Debayer, Info) << "Importing input buffer with DMABuf import failed, falling back to upload";
 		}
 	}
 
-	/* Otherwise create texture for input buffer via upload from CPU */
-	if (!use_dmabuf_) {
+	if (use_dmabuf_) {
+		/* Cache hit using dmabuf activate and bind */
+		if (!cache_miss)
+			egl_.activateBindTexture(*eglImageIn);
+	} else {
+		/* Otherwise create texture for input buffer via upload from CPU */
 		inDmaSyncer->emplace(input->planes()[0].fd, DmaSyncer::SyncType::Read);
 		inMapped->emplace(input, MappedFrameBuffer::MapFlag::Read);
 		if (!inMapped->value().isValid()) {
 			LOG(Debayer, Error) << "mmap-ing buffer(s) failed";
-			return -ENODEV;
+			return nullptr;
 		}
-		egl_.createTexture2D(*eglImageBayerIn_, inMapped->value().planes()[0].data());
+		if (cache_miss)
+			egl_.createTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
+		else
+			egl_.updateTexture2D(*eglImageIn, inMapped->value().planes()[0].data());
 	}
 
-	/* Generate the output render framebuffer as render to texture */
-	egl_.createOutputDMABufTexture2D(*eglImageBayerOut_, output->planes()[0].fd.get());
+	return eglImageIn;
+}
+
+eGLImage *DebayerEGL::getCachedOutputFrameBuffer(FrameBuffer *output)
+{
+	auto [output_cache, cache_miss] = eglImageBayerOut_.try_emplace(output->planes()[0].fd.get());
+	if (cache_miss) {
+		if (eglImageBayerOut_.size() > outputBufferCount_) {
+			LOG(Debayer, Error) << "Output buffer count " << outputBufferCount_ << " exhaustion";
+			return nullptr;
+		}
+		output_cache->second = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width,
+								  outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1);
+		egl_.createOutputDMABufTexture2D(*output_cache->second, output->planes()[0].fd.get());
+	}
+	eGLImage *eglImageOut = output_cache->second.get();
+
+	return eglImageOut;
+}
+
+int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
+{
+	eGLImage *eglImageIn;
+	eGLImage *eglImageOut;
+
+	/* eGL context switch */
+	egl_.makeCurrent();
+
+	eglImageIn = getCachedInputFrameBuffer(input, inMapped, inDmaSyncer);
+	if (!eglImageIn)
+		return -ENOMEM;
+
+	eglImageOut = getCachedOutputFrameBuffer(output);
+	if (!eglImageOut)
+		return -ENOMEM;
+
+	egl_.attachTextureToFBO(*eglImageOut);
+	setShaderVariableValues(*eglImageIn, params);
 
-	setShaderVariableValues(*eglImageBayerIn_, params);
 	glViewport(0, 0, width_, height_);
 	glClear(GL_COLOR_BUFFER_BIT);
 	glDrawArrays(GL_TRIANGLE_FAN, 0, DEBAYER_OPENGL_COORDS);
@@ -623,19 +676,13 @@  int DebayerEGL::start()
 	if (initBayerShaders(inputPixelFormat_, outputPixelFormat_))
 		return -EINVAL;
 
-	/* Raw bayer input as texture */
-	eglImageBayerIn_ = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_, height_, inputConfig_.stride, GL_TEXTURE0, 0);
-
-	/* Texture we will render to */
-	eglImageBayerOut_ = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width, outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1);
-
 	return 0;
 }
 
 void DebayerEGL::stop()
 {
-	eglImageBayerOut_.reset();
-	eglImageBayerIn_.reset();
+	eglImageBayerOut_.clear();
+	eglImageBayerIn_.clear();
 
 	if (programId_)
 		glDeleteProgram(programId_);
diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
index d8509e9f2..ddb3ef378 100644
--- a/src/libcamera/software_isp/debayer_egl.h
+++ b/src/libcamera/software_isp/debayer_egl.h
@@ -22,6 +22,7 @@ 
 #include "libcamera/internal/mapped_framebuffer.h"
 #include "libcamera/internal/software_isp/benchmark.h"
 #include "libcamera/internal/software_isp/swstats_cpu.h"
+#include "libcamera/internal/v4l2_videodevice.h"
 
 #include <EGL/egl.h>
 #include <EGL/eglext.h>
@@ -70,14 +71,19 @@  private:
 
 	bool use_dmabuf_;
 
+	eGLImage *getCachedInputFrameBuffer(FrameBuffer *input, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer);
+	eGLImage *getCachedOutputFrameBuffer(FrameBuffer *output);
+
 	/* Shader program identifiers */
 	GLuint vertexShaderId_ = 0;
 	GLuint fragmentShaderId_ = 0;
 	GLuint programId_ = 0;
 
 	/* Pointer to object representing input texture */
-	std::unique_ptr<eGLImage> eglImageBayerIn_;
-	std::unique_ptr<eGLImage> eglImageBayerOut_;
+	std::unordered_map<int, std::unique_ptr<eGLImage>> eglImageBayerIn_;
+	std::unordered_map<int, std::unique_ptr<eGLImage>> eglImageBayerOut_;
+	unsigned int inputBufferCount_;
+	unsigned int outputBufferCount_;
 
 	/* Shader parameters */
 	float firstRed_x_;