From 46c76534bcc39727b91c9d29b96c029124ceccad Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Sun, 4 Jan 2026 18:06:24 -0600 Subject: [PATCH] Use AVBufferRefs to keep EGLImages and DRM FDs around for the lifetime of the frame --- app/streaming/video/ffmpeg-renderers/drm.cpp | 19 ---- app/streaming/video/ffmpeg-renderers/drm.h | 1 - .../ffmpeg-renderers/eglimagefactory.cpp | 97 +++++++++---------- .../video/ffmpeg-renderers/eglimagefactory.h | 18 +--- .../video/ffmpeg-renderers/eglvid.cpp | 3 - .../video/ffmpeg-renderers/renderer.h | 5 - .../video/ffmpeg-renderers/vaapi.cpp | 19 ++-- app/streaming/video/ffmpeg-renderers/vaapi.h | 6 +- 8 files changed, 68 insertions(+), 100 deletions(-) diff --git a/app/streaming/video/ffmpeg-renderers/drm.cpp b/app/streaming/video/ffmpeg-renderers/drm.cpp index b1df008f..2af5eb5b 100644 --- a/app/streaming/video/ffmpeg-renderers/drm.cpp +++ b/app/streaming/video/ffmpeg-renderers/drm.cpp @@ -1266,11 +1266,6 @@ bool DrmRenderer::addFbForFrame(AVFrame *frame, uint32_t* newFbId, bool testMode "Selected DRM plane doesn't support chosen decoding format and modifier: " FOURCC_FMT " %016" PRIx64, FOURCC_FMT_ARGS(drmFrame->layers[0].format), drmFrame->objects[0].format_modifier); - - if (m_DrmPrimeBackend) { - SDL_assert(drmFrame == &mappedFrame); - m_BackendRenderer->unmapDrmPrimeFrame(drmFrame); - } return false; } } @@ -1293,10 +1288,6 @@ bool DrmRenderer::addFbForFrame(AVFrame *frame, uint32_t* newFbId, bool testMode SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "drmPrimeFDToHandle() failed: %d", errno); - if (m_DrmPrimeBackend) { - SDL_assert(drmFrame == &mappedFrame); - m_BackendRenderer->unmapDrmPrimeFrame(drmFrame); - } return false; } @@ -1318,12 +1309,6 @@ bool DrmRenderer::addFbForFrame(AVFrame *frame, uint32_t* newFbId, bool testMode handles, pitches, offsets, (flags & DRM_MODE_FB_MODIFIERS) ? modifiers : NULL, newFbId, flags); - - if (m_DrmPrimeBackend) { - SDL_assert(drmFrame == &mappedFrame); - m_BackendRenderer->unmapDrmPrimeFrame(drmFrame); - } - if (err < 0) { SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "drmModeAddFB2[WithModifiers]() failed: %d", @@ -1552,8 +1537,4 @@ ssize_t DrmRenderer::exportEGLImages(AVFrame *frame, EGLDisplay dpy, return m_EglImageFactory.exportDRMImages(frame, dpy, images); } -void DrmRenderer::freeEGLImages() { - m_EglImageFactory.freeEGLImages(); -} - #endif diff --git a/app/streaming/video/ffmpeg-renderers/drm.h b/app/streaming/video/ffmpeg-renderers/drm.h index 237a7ba3..bc34eb4f 100644 --- a/app/streaming/video/ffmpeg-renderers/drm.h +++ b/app/streaming/video/ffmpeg-renderers/drm.h @@ -500,7 +500,6 @@ public: virtual AVPixelFormat getEGLImagePixelFormat() override; virtual bool initializeEGL(EGLDisplay dpy, const EGLExtensions &ext) override; virtual ssize_t exportEGLImages(AVFrame *frame, EGLDisplay dpy, EGLImage images[EGL_MAX_PLANES]) override; - virtual void freeEGLImages() override; #endif private: diff --git a/app/streaming/video/ffmpeg-renderers/eglimagefactory.cpp b/app/streaming/video/ffmpeg-renderers/eglimagefactory.cpp index 92635016..c9aee153 100644 --- a/app/streaming/video/ffmpeg-renderers/eglimagefactory.cpp +++ b/app/streaming/video/ffmpeg-renderers/eglimagefactory.cpp @@ -69,17 +69,12 @@ bool EglImageFactory::initializeEGL(EGLDisplay, void EglImageFactory::resetCache() { - // Cannot reset cache while in the middle of rendering - SDL_assert(!m_LastImageCtx.has_value()); } #ifdef HAVE_DRM ssize_t EglImageFactory::exportDRMImages(AVFrame* frame, EGLDisplay dpy, EGLImage images[EGL_MAX_PLANES]) { - // freeEGLImages() must be called before exporting again - SDL_assert(!m_LastImageCtx.has_value()); - SDL_assert(frame->format == AV_PIX_FMT_DRM_PRIME); AVDRMFrameDescriptor* drmFrame = (AVDRMFrameDescriptor*)frame->data[0]; @@ -222,14 +217,12 @@ ssize_t EglImageFactory::exportDRMImages(AVFrame* frame, EGLDisplay dpy, EGLImag attribs[attribIndex++] = EGL_NONE; SDL_assert(attribIndex <= MAX_ATTRIB_COUNT); - EglImageContext imgCtx(dpy, m_eglDestroyImage, m_eglDestroyImageKHR); - // Our EGLImages are non-planar, so we only populate the first entry if (m_eglCreateImage) { - imgCtx.images[0] = m_eglCreateImage(dpy, EGL_NO_CONTEXT, - EGL_LINUX_DMA_BUF_EXT, - nullptr, attribs); - if (!imgCtx.images[0]) { + images[0] = m_eglCreateImage(dpy, EGL_NO_CONTEXT, + EGL_LINUX_DMA_BUF_EXT, + nullptr, attribs); + if (!images[0]) { SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "eglCreateImage() Failed: %d", eglGetError()); return -1; @@ -242,25 +235,27 @@ ssize_t EglImageFactory::exportDRMImages(AVFrame* frame, EGLDisplay dpy, EGLImag intAttribs[i] = (EGLint)attribs[i]; } - imgCtx.images[0] = m_eglCreateImageKHR(dpy, EGL_NO_CONTEXT, - EGL_LINUX_DMA_BUF_EXT, - nullptr, intAttribs); - if (!imgCtx.images[0]) { + images[0] = m_eglCreateImageKHR(dpy, EGL_NO_CONTEXT, + EGL_LINUX_DMA_BUF_EXT, + nullptr, intAttribs); + if (!images[0]) { SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "eglCreateImageKHR() Failed: %d", eglGetError()); return -1; } } - imgCtx.count = 1; + auto imgCtx = new EglImageContext(dpy, m_eglDestroyImage, m_eglDestroyImageKHR); + imgCtx->images[0] = images[0]; + imgCtx->count = 1; - // Copy the output from the image context before we move it - images[0] = imgCtx.images[0]; - - // Store this image context - m_LastImageCtx.emplace(std::move(imgCtx)); - - return 1; + // Add a buffer reference to the frame to automatically destroy the EGLImages + // when the frame is no longer referenced. + frame->opaque_ref = av_buffer_create((uint8_t*)imgCtx, sizeof(imgCtx), + freeEglImageContextBuffer, + frame->opaque_ref, // Chain any existing buffer + AV_BUFFER_FLAG_READONLY); + return imgCtx->count; } #endif @@ -269,9 +264,6 @@ ssize_t EglImageFactory::exportDRMImages(AVFrame* frame, EGLDisplay dpy, EGLImag ssize_t EglImageFactory::exportVAImages(AVFrame *frame, uint32_t exportFlags, EGLDisplay dpy, EGLImage images[EGL_MAX_PLANES]) { - // freeEGLImages() must be called before exporting again - SDL_assert(!m_LastImageCtx.has_value()); - SDL_assert(frame->format == AV_PIX_FMT_VAAPI); auto hwFrameCtx = (AVHWFramesContext*)frame->hw_frames_ctx->data; AVVAAPIDeviceContext* vaDeviceContext = (AVVAAPIDeviceContext*)hwFrameCtx->device_ctx->hwctx; @@ -285,8 +277,6 @@ ssize_t EglImageFactory::exportVAImages(AVFrame *frame, uint32_t exportFlags, EG return -1; } - EglImageContext imgCtx(dpy, m_eglDestroyImage, m_eglDestroyImageKHR); - VADRMPRIMESurfaceDescriptor vaFrame; st = vaExportSurfaceHandle(vaDeviceContext->display, surface_id, @@ -299,6 +289,8 @@ ssize_t EglImageFactory::exportVAImages(AVFrame *frame, uint32_t exportFlags, EG return -1; } + auto imgCtx = new EglImageContext(dpy, m_eglDestroyImage, m_eglDestroyImageKHR); + SDL_assert(vaFrame.num_layers <= EGL_MAX_PLANES); for (size_t i = 0; i < vaFrame.num_layers; ++i) { @@ -443,10 +435,10 @@ ssize_t EglImageFactory::exportVAImages(AVFrame *frame, uint32_t exportFlags, EG SDL_assert(attribIndex <= EGL_ATTRIB_COUNT); if (m_eglCreateImage) { - imgCtx.images[i] = m_eglCreateImage(dpy, EGL_NO_CONTEXT, - EGL_LINUX_DMA_BUF_EXT, - nullptr, attribs); - if (!imgCtx.images[i]) { + imgCtx->images[i] = m_eglCreateImage(dpy, EGL_NO_CONTEXT, + EGL_LINUX_DMA_BUF_EXT, + nullptr, attribs); + if (!imgCtx->images[i]) { SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "eglCreateImage() Failed: %d", eglGetError()); break; @@ -459,17 +451,17 @@ ssize_t EglImageFactory::exportVAImages(AVFrame *frame, uint32_t exportFlags, EG intAttribs[i] = (EGLint)attribs[i]; } - imgCtx.images[i] = m_eglCreateImageKHR(dpy, EGL_NO_CONTEXT, - EGL_LINUX_DMA_BUF_EXT, - nullptr, intAttribs); - if (!imgCtx.images[i]) { + imgCtx->images[i] = m_eglCreateImageKHR(dpy, EGL_NO_CONTEXT, + EGL_LINUX_DMA_BUF_EXT, + nullptr, intAttribs); + if (!imgCtx->images[i]) { SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "eglCreateImageKHR() Failed: %d", eglGetError()); break; } } - imgCtx.count++; + imgCtx->count++; } // Always close the exported FDs @@ -478,20 +470,24 @@ ssize_t EglImageFactory::exportVAImages(AVFrame *frame, uint32_t exportFlags, EG } // Check for failure - if (vaFrame.num_layers != imgCtx.count) { + if (vaFrame.num_layers != imgCtx->count) { + delete imgCtx; return -1; } - // Copy the output from the image context before we move it - ssize_t count = imgCtx.count; - memcpy(images, imgCtx.images, sizeof(EGLImage) * imgCtx.count); + // Add a buffer reference to the frame to automatically destroy the EGLImages + // when the frame is no longer referenced. + frame->opaque_ref = av_buffer_create((uint8_t*)imgCtx, sizeof(imgCtx), + freeEglImageContextBuffer, + frame->opaque_ref, // Chain any existing buffer + AV_BUFFER_FLAG_READONLY); - // Store this image context - m_LastImageCtx.emplace(std::move(imgCtx)); - - return count; + memcpy(images, imgCtx->images, sizeof(EGLImage) * imgCtx->count); + return imgCtx->count; } +#endif + bool EglImageFactory::supportsImportingFormat(EGLDisplay dpy, EGLint format) { if (!m_eglQueryDmaBufFormatsEXT) { @@ -573,9 +569,12 @@ bool EglImageFactory::supportsImportingModifier(EGLDisplay dpy, EGLint format, E return false; } -#endif +void EglImageFactory::freeEglImageContextBuffer(void* opaque, uint8_t* data) +{ + auto imgCtx = (EglImageContext*)data; + delete imgCtx; -void EglImageFactory::freeEGLImages() { - SDL_assert(m_LastImageCtx.has_value()); - m_LastImageCtx.reset(); + // Free any chained buffers + av_buffer_unref((AVBufferRef**)&opaque); } + diff --git a/app/streaming/video/ffmpeg-renderers/eglimagefactory.h b/app/streaming/video/ffmpeg-renderers/eglimagefactory.h index f0315f71..7828c2bd 100644 --- a/app/streaming/video/ffmpeg-renderers/eglimagefactory.h +++ b/app/streaming/video/ffmpeg-renderers/eglimagefactory.h @@ -18,18 +18,7 @@ class EglImageFactory m_eglDestroyImageKHR(fn_eglDestroyImageKHR) {} EglImageContext(const EglImageContext& other) = delete; - - EglImageContext(EglImageContext&& other) { - // Copy the basic EGL state - m_Display = other.m_Display; - m_eglDestroyImage = other.m_eglDestroyImage; - m_eglDestroyImageKHR = other.m_eglDestroyImageKHR; - - // Transfer ownership of the EGLImages - memcpy(images, other.images, other.count * sizeof(EGLImage)); - count = other.count; - other.count = 0; - } + EglImageContext(EglImageContext&& other) = delete; ~EglImageContext() { for (ssize_t i = 0; i < count; ++i) { @@ -64,14 +53,13 @@ public: ssize_t exportVAImages(AVFrame* frame, uint32_t exportFlags, EGLDisplay dpy, EGLImage images[EGL_MAX_PLANES]); #endif - void freeEGLImages(); - bool supportsImportingFormat(EGLDisplay dpy, EGLint format); bool supportsImportingModifier(EGLDisplay dpy, EGLint format, EGLuint64KHR modifier); private: + static void freeEglImageContextBuffer(void* opaque, uint8_t* data); + IFFmpegRenderer* m_Renderer; - std::optional m_LastImageCtx; bool m_EGLExtDmaBuf; PFNEGLCREATEIMAGEPROC m_eglCreateImage; PFNEGLDESTROYIMAGEPROC m_eglDestroyImage; diff --git a/app/streaming/video/ffmpeg-renderers/eglvid.cpp b/app/streaming/video/ffmpeg-renderers/eglvid.cpp index b59798f4..ab9e80e6 100644 --- a/app/streaming/video/ffmpeg-renderers/eglvid.cpp +++ b/app/streaming/video/ffmpeg-renderers/eglvid.cpp @@ -880,8 +880,6 @@ void EGLRenderer::renderFrame(AVFrame* frame) } } - m_Backend->freeEGLImages(); - // Free the DMA-BUF backing the last frame now that it is definitely // no longer being used anymore. While the PRIME FD stays around until // EGL is done with it, the memory backing it may be reused by FFmpeg @@ -905,6 +903,5 @@ bool EGLRenderer::testRenderFrame(AVFrame* frame) return false; } - m_Backend->freeEGLImages(); return true; } diff --git a/app/streaming/video/ffmpeg-renderers/renderer.h b/app/streaming/video/ffmpeg-renderers/renderer.h index 80154d65..8cae946c 100644 --- a/app/streaming/video/ffmpeg-renderers/renderer.h +++ b/app/streaming/video/ffmpeg-renderers/renderer.h @@ -512,9 +512,6 @@ public: EGLImage[EGL_MAX_PLANES]) { return -1; } - - // Free the resources allocated during the last `exportEGLImages` call - virtual void freeEGLImages() {} #endif #ifdef HAVE_DRM @@ -526,8 +523,6 @@ public: virtual bool mapDrmPrimeFrame(AVFrame*, AVDRMFrameDescriptor*) { return false; } - - virtual void unmapDrmPrimeFrame(AVDRMFrameDescriptor*) {} #endif protected: diff --git a/app/streaming/video/ffmpeg-renderers/vaapi.cpp b/app/streaming/video/ffmpeg-renderers/vaapi.cpp index ae7c5da9..1777e0c2 100644 --- a/app/streaming/video/ffmpeg-renderers/vaapi.cpp +++ b/app/streaming/video/ffmpeg-renderers/vaapi.cpp @@ -1162,11 +1162,6 @@ VAAPIRenderer::exportEGLImages(AVFrame *frame, EGLDisplay dpy, return m_EglImageFactory.exportVAImages(frame, exportFlags, dpy, images); } -void -VAAPIRenderer::freeEGLImages() { - m_EglImageFactory.freeEGLImages(); -} - #endif #ifdef HAVE_DRM @@ -1224,14 +1219,26 @@ bool VAAPIRenderer::mapDrmPrimeFrame(AVFrame* frame, AVDRMFrameDescriptor* drmDe } } + // Add a buffer reference to the frame to automatically close the + // mapped FDs when the frame is no longer referenced. + frame->opaque_ref = av_buffer_create((uint8_t*)drmDescriptor, sizeof(*drmDescriptor), + freeDrmDescriptorBuffer, + frame->opaque_ref, // Chain any existing buffer + AV_BUFFER_FLAG_READONLY); + return true; } -void VAAPIRenderer::unmapDrmPrimeFrame(AVDRMFrameDescriptor* drmDescriptor) +void VAAPIRenderer::freeDrmDescriptorBuffer(void* opaque, uint8_t* data) { + auto drmDescriptor = (AVDRMFrameDescriptor*)data; + for (int i = 0; i < drmDescriptor->nb_objects; i++) { close(drmDescriptor->objects[i].fd); } + + // Free any chained buffers + av_buffer_unref((AVBufferRef**)&opaque); } #endif diff --git a/app/streaming/video/ffmpeg-renderers/vaapi.h b/app/streaming/video/ffmpeg-renderers/vaapi.h index f2d99c57..f391fdaf 100644 --- a/app/streaming/video/ffmpeg-renderers/vaapi.h +++ b/app/streaming/video/ffmpeg-renderers/vaapi.h @@ -73,13 +73,11 @@ public: virtual AVPixelFormat getEGLImagePixelFormat() override; virtual bool initializeEGL(EGLDisplay dpy, const EGLExtensions &ext) override; virtual ssize_t exportEGLImages(AVFrame *frame, EGLDisplay dpy, EGLImage images[EGL_MAX_PLANES]) override; - virtual void freeEGLImages() override; #endif #ifdef HAVE_DRM virtual bool canExportDrmPrime() override; virtual bool mapDrmPrimeFrame(AVFrame* frame, AVDRMFrameDescriptor* drmDescriptor) override; - virtual void unmapDrmPrimeFrame(AVDRMFrameDescriptor* drmDescriptor) override; #endif private: @@ -91,6 +89,10 @@ private: bool canExportSurfaceHandle(int layerTypeFlag, VADRMPRIMESurfaceDescriptor* descriptor); #endif +#ifdef HAVE_DRM + static void freeDrmDescriptorBuffer(void* opaque, uint8_t* data); +#endif + int m_DecoderSelectionPass; int m_WindowSystem; AVBufferRef* m_HwContext;