[v2] libcamera: software_isp: debayer_egl: Teardown the output texture

26319 diff mbox series

Destroy the EGL image associated with the output DMA-BUF texture
after processing the frame.

Without this change memory can be seen building up while running
a gstreamer pipeline using GPUISP, and on end of available memory
the stream freezes and the error message `eglCreateImageKHR fail`
is reported repeatedly.

Signed-off-by: Gianfranco Mariotti <gianfranco.mariotti94@gmail.com>
---
Changes in v2:
- use utils::scope_exit to teardown

 src/libcamera/software_isp/debayer_egl.cpp | 1 +
 1 file changed, 1 insertion(+)

Hi

2026. 03. 23. 21:10 keltezéssel, Gianfranco Mariotti írta:
> Destroy the EGL image associated with the output DMA-BUF texture
> after processing the frame.
> 
> Without this change memory can be seen building up while running
> a gstreamer pipeline using GPUISP, and on end of available memory
> the stream freezes and the error message `eglCreateImageKHR fail`
> is reported repeatedly.
> 
> Signed-off-by: Gianfranco Mariotti <gianfranco.mariotti94@gmail.com>
> ---

I think

   Fixes: f520b29fe9e6 ("libcamera: software_isp: debayer_egl: Add an eGL Debayer class")

could be added.

Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> # ThinkPad X1 Yoga Gen 7 + ov2740
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

But after thinking about this a bit, I think maybe `eGLImage::image_` could be removed altogether.
I believe the EGLImage may be deleted right after the `glEGLImageTargetTexture2DOES()` call since
it will be referenced by the texture, at least that's what I would expect based on the mesa code
and given how opengl et al generally operates. The below change appears to work.


diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h
index 8a2d96d7a..0ad2320b1 100644
--- a/include/libcamera/internal/egl.h
+++ b/include/libcamera/internal/egl.h
@@ -88,7 +88,6 @@ public:
  	GLenum texture_unit_; /**< Texture unit associated with this image eg (GL_TEXTURE0) */
  	GLuint texture_; /**< OpenGL texture object ID */
  	GLuint fbo_; /**< OpenGL frame buffer object ID */
-	EGLImageKHR image_; /**< EGL Image handle */
  
  private:
  	LIBCAMERA_DISABLE_COPY_AND_MOVE(eGLImage)
@@ -104,7 +103,6 @@ public:
  
  	int createInputDMABufTexture2D(eGLImage &eglImage, int fd);
  	int createOutputDMABufTexture2D(eGLImage &eglImage, int fd);
-	void destroyDMABufTexture(eGLImage &eglImage);
  	void createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint32_t height, void *data);
  
  	void pushEnv(std::vector<std::string> &shaderEnv, const char *str);
diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
index 5b9bbf410..d8d6e3a42 100644
--- a/src/libcamera/egl.cpp
+++ b/src/libcamera/egl.cpp
@@ -112,8 +112,6 @@ void eGL::syncOutput()
   */
  int eGL::createDMABufTexture2D(eGLImage &eglImage, int fd, bool output)
  {
-	int ret = 0;
-
  	ASSERT(tid_ == Thread::currentId());
  
  	// clang-format off
@@ -130,22 +128,23 @@ int eGL::createDMABufTexture2D(eGLImage &eglImage, int fd, bool output)
  	};
  	// clang-format on
  
-	eglImage.image_ = eglCreateImageKHR(display_, EGL_NO_CONTEXT,
-					    EGL_LINUX_DMA_BUF_EXT,
-					    NULL, image_attrs);
+	EGLImageKHR image = eglCreateImageKHR(display_, EGL_NO_CONTEXT,
+					      EGL_LINUX_DMA_BUF_EXT,
+					      NULL, image_attrs);
  
-	if (eglImage.image_ == EGL_NO_IMAGE_KHR) {
+	if (image == EGL_NO_IMAGE_KHR) {
  		LOG(eGL, Error) << "eglCreateImageKHR fail";
-		ret = -ENODEV;
-		goto done;
+		return -ENODEV;
  	}
  
+	utils::scope_exit imageGuard([&] { eglDestroyImageKHR(display_, image); });
+
  	// Bind texture unit and texture
  	glActiveTexture(eglImage.texture_unit_);
  	glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
  
  	// Generate texture with filter semantics
-	glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, eglImage.image_);
+	glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, image);
  
  	// Nearest filtering
  	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
@@ -163,13 +162,11 @@ int eGL::createDMABufTexture2D(eGLImage &eglImage, int fd, bool output)
  		GLenum err = glCheckFramebufferStatus(GL_FRAMEBUFFER);
  		if (err != GL_FRAMEBUFFER_COMPLETE) {
  			LOG(eGL, Error) << "glFrameBufferTexture2D error " << err;
-			eglDestroyImageKHR(display_, eglImage.image_);
-			ret = -ENODEV;
-			goto done;
+			return -ENODEV;
  		}
  	}
-done:
-	return ret;
+
+	return 0;
  }
  
  /**
@@ -209,19 +206,6 @@ int eGL::createOutputDMABufTexture2D(eGLImage &eglImage, int fd)
  	return createDMABufTexture2D(eglImage, fd, true);
  }
  
-/**
- * \brief Destroy a DMA-BUF texture's EGL image
- * \param[in,out] eglImage EGL image to destroy
- *
- * Destroys the EGL image associated with a DMA-BUF texture. The OpenGL
- * texture and framebuffer objects are destroyed separately in the
- * eGLImage destructor.
- */
-void eGL::destroyDMABufTexture(eGLImage &eglImage)
-{
-	eglDestroyImage(display_, std::exchange(eglImage.image_, EGL_NO_IMAGE_KHR));
-}
-
  /**
   * \brief Create a 2D texture from a memory buffer
   * \param[in,out] eglImage EGL image to associate with the texture


> Changes in v2:
> - use utils::scope_exit to teardown
> 
>   src/libcamera/software_isp/debayer_egl.cpp | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
> index 8147eca1..14ea98eb 100644
> --- a/src/libcamera/software_isp/debayer_egl.cpp
> +++ b/src/libcamera/software_isp/debayer_egl.cpp
> @@ -506,6 +506,7 @@ int DebayerEGL::debayerGPU(MappedFrameBuffer &in, int out_fd, const DebayerParam
>   
>   	/* Generate the output render framebuffer as render to texture */
>   	egl_.createOutputDMABufTexture2D(*eglImageBayerOut_, out_fd);
> +	utils::scope_exit outImageGuard([&] { egl_.destroyDMABufTexture(*eglImageBayerOut_); });
>   
>   	setShaderVariableValues(params);
>   	glViewport(0, 0, width_, height_);

On 24/03/2026 08:39, Barnabás Pőcze wrote:
> +	EGLImageKHR image = eglCreateImageKHR(display_, EGL_NO_CONTEXT,
> +					      EGL_LINUX_DMA_BUF_EXT,
> +					      NULL, image_attrs);
> 
> -	if (eglImage.image_ == EGL_NO_IMAGE_KHR) {
> +	if (image == EGL_NO_IMAGE_KHR) {
>    		LOG(eGL, Error) << "eglCreateImageKHR fail";
> -		ret = -ENODEV;
> -		goto done;
> +		return -ENODEV;
>    	}
> 
> +	utils::scope_exit imageGuard([&] { eglDestroyImageKHR(display_, image); });

Part of storing the image_ was the thought we might use it for 
multi-pass but I don't think that will be necessary.

Why ssope guard this at all though ?

Just delete direction after glEGLImageTargetTexture2DOES() as you're 
right the texture will be resident in VRAM.

---
bod

Hi

2026. 03. 25. 16:12 keltezéssel, Bryan O'Donoghue írta:
> On 24/03/2026 08:39, Barnabás Pőcze wrote:
>> +    EGLImageKHR image = eglCreateImageKHR(display_, EGL_NO_CONTEXT,
>> +                          EGL_LINUX_DMA_BUF_EXT,
>> +                          NULL, image_attrs);
>>
>> -    if (eglImage.image_ == EGL_NO_IMAGE_KHR) {
>> +    if (image == EGL_NO_IMAGE_KHR) {
>>            LOG(eGL, Error) << "eglCreateImageKHR fail";
>> -        ret = -ENODEV;
>> -        goto done;
>> +        return -ENODEV;
>>        }
>>
>> +    utils::scope_exit imageGuard([&] { eglDestroyImageKHR(display_, image); });
> 
> Part of storing the image_ was the thought we might use it for multi-pass but I don't think that will be necessary.

Even if it is used, I think it's easy to bring it back. But currently it is not needed,
so I think it makes sense to remove it (for now at least).

> 
> Why ssope guard this at all though ?
> 
> Just delete direction after glEGLImageTargetTexture2DOES() as you're right the texture will be resident in VRAM.

Good question... I was testing some things and did not know how the code
is going to end up. But it's probably simpler to just delete it after `glEGLImageTargetTexture2DOES()`.


Regards,
Barnabás Pőcze


> 
> ---
> bod