qt(6)/material: ensure that we always update the context in setBuffer()
authorMatthew Waters <matthew@centricular.com>
Fri, 22 Nov 2024 07:59:53 +0000 (18:59 +1100)
committerBackport Bot <gitlab-backport-bot@gstreamer-foundation.org>
Wed, 27 Nov 2024 01:19:27 +0000 (01:19 +0000)
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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7978>

subprojects/gst-plugins-good/ext/qt/gstqsgmaterial.cc
subprojects/gst-plugins-good/ext/qt6/gstqsg6material.cc

index f708667cdd8e2ee23bd3a9968fee1d467c81790f..5a2b48e69032acf57e18b559254f25a2fa08de8e 100644 (file)
@@ -383,16 +383,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 */
@@ -413,7 +420,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;
   gboolean use_dummy_tex = TRUE;
@@ -423,10 +430,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)
@@ -436,6 +439,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 */
@@ -445,6 +452,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));
 
index a4e012f301258cf97f4b60fc607c1c530acf71f6..9e64d4ab46988bbc67b0c33922b61ee4bcb3faf4 100644 (file)
@@ -447,15 +447,19 @@ GstQSGMaterial::setCaps (GstCaps * caps)
 gboolean
 GstQSGMaterial::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));
@@ -537,7 +541,7 @@ video_format_to_texel_size (GstVideoFormat format, guint plane)
 QSGTexture *
 GstQSGMaterial::bind(GstQSGMaterialShader *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;
@@ -547,15 +551,17 @@ GstQSGMaterial::bind(GstQSGMaterialShader *shader, QRhi * rhi, QRhiResourceUpdat
   QRhiTexture *rhi_tex;
   QSize tex_size;
 
-  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;