[v3,2/8] libcamera: software_isp: debayer_egl: Flag dmabuf use once per session not for every frame

27049 diff mbox series

Once we get as far a streaming if we have one dmabuf import failure, take
that failure as canonical and do not try further imports. Add a flag to
debayer_egl which gets reset on any configure() to control this logic, flip
the dmabuf bit on the first failure for all subsequent frames.

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

On 26.06.26 13:33, Bryan O'Donoghue wrote:
> Once we get as far a streaming if we have one dmabuf import failure, take
> that failure as canonical and do not try further imports. Add a flag to
> debayer_egl which gets reset on any configure() to control this logic, flip
> the dmabuf bit on the first failure for all subsequent frames.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

I would prefer to see "libcamera: egl: Drop dmabuf_import_failed_" 
squashed into this commit or at least directly follow - and rephrase the 
commit message to something along the lines of:

"Instead of tracking dmabuf import failures in eGLImage, do so in 
debayer_egl. This will avoid unnecessary import attempts once we start 
using multiple eGLImages later in the series.

Still:

Reviewed-by: Robert Mader<robert.mader@collabora.com>

> ---
>   src/libcamera/software_isp/debayer_egl.cpp | 13 ++++++-------
>   src/libcamera/software_isp/debayer_egl.h   |  2 ++
>   2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
> index 14eeb9a29..53bb67c17 100644
> --- a/src/libcamera/software_isp/debayer_egl.cpp
> +++ b/src/libcamera/software_isp/debayer_egl.cpp
> @@ -316,6 +316,7 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg,
>   	inputPixelFormat_ = inputCfg.pixelFormat;
>   	width_ = inputCfg.size.width;
>   	height_ = inputCfg.size.height;
> +	use_dmabuf_ = true;
>   
>   	if (outputCfgs.size() != 1) {
>   		LOG(Debayer, Error)
> @@ -515,21 +516,19 @@ void DebayerEGL::setShaderVariableValues(eGLImage &eglImageIn, const DebayerPara
>   
>   int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
>   {
> -	bool dmabuf_import_succeeded = false;
> -
>   	/* eGL context switch */
>   	egl_.makeCurrent();
>   
>   	/* Try to create texture for input buffer via dmabuf import */
> -	if (!eglImageBayerIn_->dmabuf_import_failed_) {
> -		if (egl_.createInputDMABufTexture2D(*eglImageBayerIn_, input->planes()[0].fd.get()) == 0)
> -			dmabuf_import_succeeded = true;
> -		else
> +	if (use_dmabuf_) {
> +		if (egl_.createInputDMABufTexture2D(*eglImageBayerIn_, 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 (!dmabuf_import_succeeded) {
> +	if (!use_dmabuf_) {
>   		inDmaSyncer->emplace(input->planes()[0].fd, DmaSyncer::SyncType::Read);
>   		inMapped->emplace(input, MappedFrameBuffer::MapFlag::Read);
>   		if (!inMapped->value().isValid()) {
> diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
> index 348d7305b..d8509e9f2 100644
> --- a/src/libcamera/software_isp/debayer_egl.h
> +++ b/src/libcamera/software_isp/debayer_egl.h
> @@ -68,6 +68,8 @@ private:
>   	void setShaderVariableValues(eGLImage &eGLImageIn, const DebayerParams &params);
>   	int debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *mappedInputBuffer, std::optional<DmaSyncer> *inputBufferDmaSyncer);
>   
> +	bool use_dmabuf_;
> +
>   	/* Shader program identifiers */
>   	GLuint vertexShaderId_ = 0;
>   	GLuint fragmentShaderId_ = 0;

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

> Once we get as far a streaming if we have one dmabuf import failure, take
> that failure as canonical and do not try further imports. Add a flag to
> debayer_egl which gets reset on any configure() to control this logic, flip
> the dmabuf bit on the first failure for all subsequent frames.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

> ---
>  src/libcamera/software_isp/debayer_egl.cpp | 13 ++++++-------
>  src/libcamera/software_isp/debayer_egl.h   |  2 ++
>  2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
> index 14eeb9a29..53bb67c17 100644
> --- a/src/libcamera/software_isp/debayer_egl.cpp
> +++ b/src/libcamera/software_isp/debayer_egl.cpp
> @@ -316,6 +316,7 @@ int DebayerEGL::configure(const StreamConfiguration &inputCfg,
>  	inputPixelFormat_ = inputCfg.pixelFormat;
>  	width_ = inputCfg.size.width;
>  	height_ = inputCfg.size.height;
> +	use_dmabuf_ = true;
>  
>  	if (outputCfgs.size() != 1) {
>  		LOG(Debayer, Error)
> @@ -515,21 +516,19 @@ void DebayerEGL::setShaderVariableValues(eGLImage &eglImageIn, const DebayerPara
>  
>  int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *inMapped, std::optional<DmaSyncer> *inDmaSyncer)
>  {
> -	bool dmabuf_import_succeeded = false;
> -
>  	/* eGL context switch */
>  	egl_.makeCurrent();
>  
>  	/* Try to create texture for input buffer via dmabuf import */
> -	if (!eglImageBayerIn_->dmabuf_import_failed_) {
> -		if (egl_.createInputDMABufTexture2D(*eglImageBayerIn_, input->planes()[0].fd.get()) == 0)
> -			dmabuf_import_succeeded = true;
> -		else
> +	if (use_dmabuf_) {
> +		if (egl_.createInputDMABufTexture2D(*eglImageBayerIn_, 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 (!dmabuf_import_succeeded) {
> +	if (!use_dmabuf_) {
>  		inDmaSyncer->emplace(input->planes()[0].fd, DmaSyncer::SyncType::Read);
>  		inMapped->emplace(input, MappedFrameBuffer::MapFlag::Read);
>  		if (!inMapped->value().isValid()) {
> diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
> index 348d7305b..d8509e9f2 100644
> --- a/src/libcamera/software_isp/debayer_egl.h
> +++ b/src/libcamera/software_isp/debayer_egl.h
> @@ -68,6 +68,8 @@ private:
>  	void setShaderVariableValues(eGLImage &eGLImageIn, const DebayerParams &params);
>  	int debayerGPU(FrameBuffer *input, FrameBuffer *output, const DebayerParams &params, std::optional<MappedFrameBuffer> *mappedInputBuffer, std::optional<DmaSyncer> *inputBufferDmaSyncer);
>  
> +	bool use_dmabuf_;
> +
>  	/* Shader program identifiers */
>  	GLuint vertexShaderId_ = 0;
>  	GLuint fragmentShaderId_ = 0;