From e2d388000c5133dd877270ac410fe9f0c4b20716 Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Fri, 22 Nov 2024 18:59:53 +1100 Subject: [PATCH] qt(6)/material: ensure that we always update the context in setBuffer() Scenario is that there are two (or more) GstGLContext's wrapping Qt's GL context from either multiple qml(6)glsink or qml(6)glsrc elements. Call flow is this: 1. material 1 setBuffer() 2. material 1 bind() 3. material 2 setBuffer() 4. material 2 bind() If the call to setBuffer() reuses the same buffer as previous call, then the qt context is not updated in the material. If however the previously used qt context by the material had been deactivated or freed, then bind() would fail and could result in a critical like so: gst_gl_context_thread_add: assertion 'context->priv->active_thread == g_thread_self ()' failed Part-of: --- .../gst-plugins-good/ext/qt/gstqsgmaterial.cc | 31 ++++++++++++------- .../ext/qt6/gstqsg6material.cc | 22 ++++++++----- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/subprojects/gst-plugins-good/ext/qt/gstqsgmaterial.cc b/subprojects/gst-plugins-good/ext/qt/gstqsgmaterial.cc index 3dfc325a90..4459a92c2f 100644 --- a/subprojects/gst-plugins-good/ext/qt/gstqsgmaterial.cc +++ b/subprojects/gst-plugins-good/ext/qt/gstqsgmaterial.cc @@ -451,16 +451,23 @@ GstQSGMaterial::setCaps (GstCaps * caps) gboolean GstQSGMaterial::setBuffer (GstBuffer * buffer) { - GST_LOG ("%p setBuffer %" GST_PTR_FORMAT, this, buffer); + GstGLContext *qt_context; + gboolean ret = FALSE; + /* FIXME: update more state here */ - if (!gst_buffer_replace (&this->buffer_, buffer)) - return FALSE; + if (gst_buffer_replace (&this->buffer_, buffer)) { + GST_LOG ("%p setBuffer new buffer %" GST_PTR_FORMAT, this, buffer); - this->buffer_was_bound = FALSE; + this->buffer_was_bound = FALSE; + ret = TRUE; + } - g_weak_ref_set (&this->qt_context_ref_, gst_gl_context_get_current ()); + qt_context = gst_gl_context_get_current (); + GST_DEBUG ("%p setBuffer with qt context %" GST_PTR_FORMAT, this, qt_context); - return TRUE; + g_weak_ref_set (&this->qt_context_ref_, qt_context); + + return ret; } /* only called from the streaming thread with scene graph thread blocked */ @@ -481,7 +488,7 @@ void GstQSGMaterial::bind(GstQSGMaterialShader *shader, GstVideoFormat v_format) { const GstGLFuncs *gl; - GstGLContext *context, *qt_context; + GstGLContext *context, *qt_context = NULL; GstGLSyncMeta *sync_meta; GstMemory *mem; GstGLMemory *gl_mem; @@ -492,10 +499,6 @@ GstQSGMaterial::bind(GstQSGMaterialShader *shader, GstVideoFormat v_format) memset (&this->v_frame, 0, sizeof (this->v_frame)); } - qt_context = GST_GL_CONTEXT (g_weak_ref_get (&this->qt_context_ref_)); - if (!qt_context) - goto out; - if (!this->buffer_) goto out; if (GST_VIDEO_INFO_FORMAT (&this->v_info) == GST_VIDEO_FORMAT_UNKNOWN) @@ -505,6 +508,10 @@ GstQSGMaterial::bind(GstQSGMaterialShader *shader, GstVideoFormat v_format) if (!this->mem_) goto out; + qt_context = GST_GL_CONTEXT (g_weak_ref_get (&this->qt_context_ref_)); + if (!qt_context) + goto out; + gl = qt_context->gl_vtable; /* FIXME: should really lock the memory to prevent write access */ @@ -514,6 +521,8 @@ GstQSGMaterial::bind(GstQSGMaterialShader *shader, GstVideoFormat v_format) goto out; } + GST_DEBUG ("%p attempting to bind with context %" GST_PTR_FORMAT, this, qt_context); + mem = gst_buffer_peek_memory (this->buffer_, 0); g_assert (gst_is_gl_memory (mem)); gl_mem = (GstGLMemory *)mem; diff --git a/subprojects/gst-plugins-good/ext/qt6/gstqsg6material.cc b/subprojects/gst-plugins-good/ext/qt6/gstqsg6material.cc index 3848f1cab8..d230ec804d 100644 --- a/subprojects/gst-plugins-good/ext/qt6/gstqsg6material.cc +++ b/subprojects/gst-plugins-good/ext/qt6/gstqsg6material.cc @@ -473,15 +473,19 @@ GstQSG6Material::setCaps (GstCaps * caps) gboolean GstQSG6Material::setBuffer (GstBuffer * buffer) { - GST_LOG ("%p setBuffer %" GST_PTR_FORMAT, this, buffer); + GstGLContext *qt_context = gst_gl_context_get_current (); + + GST_LOG ("%p setBuffer %" GST_PTR_FORMAT " with qt context %" GST_PTR_FORMAT, + this, buffer, qt_context); /* FIXME: update more state here */ + + g_weak_ref_set (&this->qt_context_ref_, qt_context); + if (!gst_buffer_replace (&this->buffer_, buffer)) return FALSE; this->buffer_was_bound = false; - g_weak_ref_set (&this->qt_context_ref_, gst_gl_context_get_current ()); - if (this->v_frame.buffer) { gst_video_frame_unmap (&this->v_frame); memset (&this->v_frame, 0, sizeof (this->v_frame)); @@ -567,7 +571,7 @@ video_format_to_texel_size (GstVideoFormat format, guint plane) QSGTexture * GstQSG6Material::bind(GstQSG6MaterialShader *shader, QRhi * rhi, QRhiResourceUpdateBatch *res_updates, guint plane, GstVideoFormat v_format) { - GstGLContext *qt_context, *context; + GstGLContext *qt_context = NULL, *context; GstMemory *mem; GstGLMemory *gl_mem; GstGLSyncMeta *sync_meta; @@ -578,15 +582,17 @@ GstQSG6Material::bind(GstQSG6MaterialShader *shader, QRhi * rhi, QRhiResourceUpd QSize tex_size; QRhiTexture::Flags flags = {}; - qt_context = GST_GL_CONTEXT (g_weak_ref_get (&this->qt_context_ref_)); - if (!qt_context) - goto out; - if (!this->buffer_) goto out; if (GST_VIDEO_INFO_FORMAT (&this->v_info) == GST_VIDEO_FORMAT_UNKNOWN) goto out; + qt_context = GST_GL_CONTEXT (g_weak_ref_get (&this->qt_context_ref_)); + if (!qt_context) + goto out; + + GST_DEBUG ("%p attempting to bind with context %" GST_PTR_FORMAT, this, qt_context); + mem = gst_buffer_peek_memory (this->buffer_, plane); g_assert (gst_is_gl_memory (mem)); gl_mem = (GstGLMemory *) mem;