[264/906] Rewrite gstglwindow_x11.c because X API is not thread safe.

gst-libs/gst/gl/gstglwindow_x11.c

@@ -37,6 +37,11 @@ enum
 
 struct _GstGLWindowPrivate
 {
+  GMutex *glwin_lock;
+  GMutex *x_lock;
+  GCond *cond_send_message;
+  gboolean running;
+
   gchar *display_name;
   Display *device;
   gint screen;
@@ -57,11 +62,6 @@ struct _GstGLWindowPrivate
   gpointer resize_data;
   GstGLWindowCB close_cb;
   gpointer close_data;
-
-  gboolean is_closed;
-
-  GMutex *mutex;
-  GCond *cond_send_message;
 };
 
 G_DEFINE_TYPE (GstGLWindow, gst_gl_window, G_TYPE_OBJECT);
@@ -79,6 +79,9 @@ gst_gl_window_finalize (GObject * object)
   GstGLWindowPrivate *priv = window->priv;
   XEvent event;
 
+  g_mutex_lock (priv->glwin_lock);
+  g_mutex_lock (priv->x_lock);
+
   g_debug ("gl window finalizing\n");
 
   XUnmapWindow (priv->device, priv->internal_win_id);
@@ -97,6 +100,7 @@ gst_gl_window_finalize (GObject * object)
   }
 
   XFlush (priv->device);
+  XSync (priv->device, FALSE);
   while(XPending (priv->device))
   {
     g_debug ("one more last pending x msg\n");
@@ -109,11 +113,17 @@ gst_gl_window_finalize (GObject * object)
 
   g_debug ("display closed\n");
 
+  g_mutex_unlock (priv->glwin_lock);
+  g_mutex_unlock (priv->x_lock);
+
   if (priv->cond_send_message)
     g_cond_free (priv->cond_send_message);
 
-  if (priv->mutex)
+  if (priv->x_lock)
-    g_mutex_free (priv->mutex);
+    g_mutex_free (priv->x_lock);
+
+  if (priv->glwin_lock)
+    g_mutex_free (priv->glwin_lock);
 
   G_OBJECT_CLASS (gst_gl_window_parent_class)->finalize (object);
 
@@ -230,13 +240,17 @@ gst_gl_window_new (gint width, gint height)
   static gint x = 0;
   static gint y = 0;
 
+  priv->glwin_lock = g_mutex_new ();
+  priv->x_lock = g_mutex_new ();
+  priv->cond_send_message = g_cond_new ();
+  priv->running = TRUE;
+
+  g_mutex_lock (priv->glwin_lock);
+  g_mutex_lock (priv->x_lock);
+
   x += 20;
   y += 20;
 
-  priv->mutex = g_mutex_new ();
-  priv->cond_send_message = g_cond_new ();
-  priv->is_closed = FALSE;
-
   priv->device = XOpenDisplay (priv->display_name);
 
   g_debug ("gl device id: %ld\n", (gulong) priv->device);
@@ -268,6 +282,8 @@ gst_gl_window_new (gint width, gint height)
     width, height, 0, priv->visual_info->depth, InputOutput,
     priv->visual_info->visual, mask, &win_attr);
 
+  XSync (priv->device, FALSE);
+
   g_debug ("gl window id: %lld\n", (guint64) priv->internal_win_id);
 
   priv->gl_context = glXCreateContext (priv->device, priv->visual_info, NULL, TRUE);
@@ -300,6 +316,9 @@ gst_gl_window_new (gint width, gint height)
   if (!ret)
     g_debug ("failed to make opengl context current\n");
 
+  g_mutex_unlock (priv->x_lock);
+  g_mutex_unlock (priv->glwin_lock);
+
   return window;
 }
 
@@ -343,8 +362,12 @@ gst_gl_window_set_draw_callback (GstGLWindow *window, GstGLWindowCB callback, gp
 {
   GstGLWindowPrivate *priv = window->priv;
 
+  g_mutex_lock (priv->glwin_lock);
+
   priv->draw_cb = callback;
   priv->draw_data = data;
+
+  g_mutex_unlock (priv->glwin_lock);
 }
 
 /* Must be called in the gl thread */
@@ -353,8 +376,12 @@ gst_gl_window_set_resize_callback (GstGLWindow *window, GstGLWindowCB2 callback
 {
   GstGLWindowPrivate *priv = window->priv;
 
+  g_mutex_lock (priv->glwin_lock);
+
   priv->resize_cb = callback;
   priv->resize_data = data;
+
+  g_mutex_unlock (priv->glwin_lock);
 }
 
 /* Must be called in the gl thread */
@@ -363,158 +390,207 @@ gst_gl_window_set_close_callback (GstGLWindow *window, GstGLWindowCB callback, g
 {
   GstGLWindowPrivate *priv = window->priv;
 
+  g_mutex_lock (priv->glwin_lock);
+
   priv->close_cb = callback;
   priv->close_data = data;
+
+  g_mutex_unlock (priv->glwin_lock);
 }
 
 /* Thread safe */
 void
 gst_gl_window_visible (GstGLWindow *window, gboolean visible)
 {
-  GstGLWindowPrivate *priv = window->priv;
+  if (window)
+  {
+    GstGLWindowPrivate *priv = window->priv;
 
-  g_debug ("set visible %lld\n", (guint64) priv->internal_win_id);
+    g_mutex_lock (priv->glwin_lock);
 
-  if (visible)
+    if (priv->running)
-    XMapWindow (priv->device, priv->internal_win_id);
+    {
-  else
+      g_mutex_lock (priv->x_lock);
-    XUnmapWindow (priv->device, priv->internal_win_id);
 
-  XSync(priv->device, FALSE);
+      g_debug ("set visible %lld\n", (guint64) priv->internal_win_id);
+
+      if (visible)
+        XMapWindow (priv->device, priv->internal_win_id);
+      else
+        XUnmapWindow (priv->device, priv->internal_win_id);
+
+      XSync(priv->device, FALSE);
+
+      g_mutex_unlock (priv->x_lock);
+    }
+
+    g_mutex_unlock (priv->glwin_lock);
+  }
 }
 
 /* Thread safe */
 void
 gst_gl_window_draw (GstGLWindow *window)
 {
-  GstGLWindowPrivate *priv = window->priv;
+  g_debug ("DRAW IN\n");
-  XEvent event;
+  if (window)
-  XWindowAttributes attr;
+  {
+    GstGLWindowPrivate *priv = window->priv;
 
-  XGetWindowAttributes (priv->device, priv->internal_win_id, &attr);
+    g_mutex_lock (priv->glwin_lock);
 
-  event.xexpose.type = Expose;
+    if (priv->running)
-  event.xexpose.send_event = TRUE;
+    {
-  event.xexpose.display = priv->device;
+      XEvent event;
-  event.xexpose.window = priv->internal_win_id;
+      XWindowAttributes attr;
-  event.xexpose.x = attr.x;
+
-  event.xexpose.y = attr.y;
+      g_mutex_lock (priv->x_lock);
-  event.xexpose.width = attr.width;
+
-  event.xexpose.height = attr.height;
+      XGetWindowAttributes (priv->device, priv->internal_win_id, &attr);
-  event.xexpose.count = 0;
+
-  XSendEvent (priv->device, priv->internal_win_id, FALSE, ExposureMask, &event);
+      event.xexpose.type = Expose;
+      event.xexpose.send_event = TRUE;
+      event.xexpose.display = priv->device;
+      event.xexpose.window = priv->internal_win_id;
+      event.xexpose.x = attr.x;
+      event.xexpose.y = attr.y;
+      event.xexpose.width = attr.width;
+      event.xexpose.height = attr.height;
+      event.xexpose.count = 0;
+
+      XSendEvent (priv->device, priv->internal_win_id, FALSE, ExposureMask, &event);
+
+      g_mutex_unlock (priv->x_lock);
+    }
+
+    g_mutex_unlock (priv->glwin_lock);
+  }
+  g_debug ("DRAW OUT\n");
 }
 
 void
 gst_gl_window_run_loop (GstGLWindow *window)
 {
   GstGLWindowPrivate *priv = window->priv;
-  gboolean running = TRUE;
-  XEvent event;
 
   g_debug ("begin loop\n");
 
-  while (running)
+  g_mutex_lock (priv->glwin_lock);
+
+  while (priv->running)
   {
-    XNextEvent(priv->device, &event);
 
-    switch (event.type)
+    g_mutex_unlock (priv->glwin_lock);
+
+    g_mutex_lock (priv->x_lock);
+
+    while (XPending (priv->device))
     {
-      case ClientMessage:
+      XEvent event;
+
+      XNextEvent(priv->device, &event);
+
+      switch (event.type)
       {
-
+        case ClientMessage:
-        if (event.xclient.message_type == priv->atom_custom)
         {
 
-          if (!priv->is_closed)
+          if (event.xclient.message_type == priv->atom_custom)
           {
-            GstGLWindowCB custom_cb = (GstGLWindowCB) event.xclient.data.l[0];
-            gpointer custom_data = (gpointer) event.xclient.data.l[1];
-            custom_cb (custom_data);
-          }
 
-          g_cond_signal (priv->cond_send_message);
+            if (priv->running)
-        }
+            {
+              GstGLWindowCB custom_cb = (GstGLWindowCB) event.xclient.data.l[0];
+              gpointer custom_data = (gpointer) event.xclient.data.l[1];
+              custom_cb (custom_data);
+            }
 
-        else if ( (Atom) event.xclient.data.l[0] == priv->atom_delete_window)
-        {
-          XEvent event;
-
-          g_debug ("Close\n");
-
-          g_mutex_lock (priv->mutex);
-
-          priv->is_closed = TRUE;
-          if (priv->close_cb)
-            priv->close_cb (priv->close_data);
-          running = FALSE;
-
-          XFlush (priv->device);
-          while (XCheckTypedEvent (priv->device, ClientMessage, &event))
-          {
-            g_debug ("discared custom x event\n");
             g_cond_signal (priv->cond_send_message);
           }
 
-          g_mutex_unlock (priv->mutex);
+          else if ( (Atom) event.xclient.data.l[0] == priv->atom_delete_window)
+          {
+            XEvent event;
+
+            g_debug ("Close\n");
+
+            priv->running = FALSE;
+            if (priv->close_cb)
+              priv->close_cb (priv->close_data);
+
+            XFlush (priv->device);
+            while (XCheckTypedEvent (priv->device, ClientMessage, &event))
+            {
+              g_debug ("discared custom x event\n");
+              g_cond_signal (priv->cond_send_message);
+            }
+          }
+          break;
         }
-        break;
-      }
 
-      case CreateNotify:
+        case CreateNotify:
-      case ConfigureNotify:
+        case ConfigureNotify:
-      {
-        gint width = event.xconfigure.width;
-        gint height = event.xconfigure.height;
-        if (priv->resize_cb)
-          priv->resize_cb (priv->resize_data, width, height);
-        break;
-      }
-
-      case DestroyNotify:
-        g_debug ("DestroyNotify\n");
-        break;
-
-      case Expose:
-        if (priv->draw_cb)
         {
-          priv->draw_cb (priv->draw_data);
+          gint width = event.xconfigure.width;
-          //glFlush();
+          gint height = event.xconfigure.height;
-          glXSwapBuffers (priv->device, priv->internal_win_id);
+          if (priv->resize_cb)
+            priv->resize_cb (priv->resize_data, width, height);
+          break;
         }
-        break;
 
-      case VisibilityNotify:
+        case DestroyNotify:
-      {
+          g_debug ("DestroyNotify\n");
-        g_debug ("VisibilityNotify\n");
+          break;
 
-        switch (event.xvisibility.state)
+        case Expose:
+          if (priv->draw_cb)
+          {
+            priv->draw_cb (priv->draw_data);
+            //glFlush();
+            glXSwapBuffers (priv->device, priv->internal_win_id);
+          }
+          break;
+
+        case VisibilityNotify:
         {
-          case VisibilityUnobscured:
+          switch (event.xvisibility.state)
-            if (priv->draw_cb)
+          {
-              priv->draw_cb (priv->draw_data);
+            case VisibilityUnobscured:
-            break;
+              if (priv->draw_cb)
+                priv->draw_cb (priv->draw_data);
+              break;
 
-          case VisibilityPartiallyObscured:
+            case VisibilityPartiallyObscured:
-            if (priv->draw_cb)
+              if (priv->draw_cb)
-              priv->draw_cb (priv->draw_data);
+                priv->draw_cb (priv->draw_data);
-            break;
+              break;
 
-          case VisibilityFullyObscured:
+            case VisibilityFullyObscured:
-            break;
+              break;
 
-          default:
+            default:
-            g_debug("unknown xvisibility event: %d\n", event.xvisibility.state);
+              g_debug("unknown xvisibility event: %d\n", event.xvisibility.state);
-            break;
+              break;
+          }
+          break;
         }
-        break;
-      }
 
-      default:
+        default:
-        break;
+          break;
 
-    }
+      }// switch
-  }
+
+    }// while XPending
+
+    g_mutex_unlock (priv->x_lock);
+
+    g_usleep (10000);
+
+    g_mutex_lock (priv->glwin_lock);
+
+  }// while running
+
+  g_mutex_unlock (priv->glwin_lock);
 
   g_debug ("end loop\n");
 }
@@ -523,49 +599,38 @@ gst_gl_window_run_loop (GstGLWindow *window)
 void
 gst_gl_window_quit_loop (GstGLWindow *window)
 {
+  g_debug ("QUIT LOOP IN\n");
   if (window)
   {
     GstGLWindowPrivate *priv = window->priv;
 
-    g_mutex_lock (priv->mutex);
+    g_mutex_lock (priv->glwin_lock);
 
-    if (!priv->is_closed)
+    priv->running = FALSE;
-    {
 
-      XEvent event;
+    g_mutex_unlock (priv->glwin_lock);
-
-      event.xclient.type = ClientMessage;
-      event.xclient.send_event = TRUE;
-      event.xclient.display = priv->device;
-      event.xclient.window = priv->internal_win_id;
-      event.xclient.message_type = 0;
-      event.xclient.format = 32;
-      event.xclient.data.l[0] = priv->atom_delete_window;
-
-      XSendEvent (priv->device, priv->internal_win_id, FALSE, NoEventMask, &event);
-      XFlush (priv->device);
-
-    }
-
-    g_mutex_unlock (priv->mutex);
   }
+  g_debug ("QUIT LOOP OUT\n");
 }
 
 /* Thread safe */
 void
 gst_gl_window_send_message (GstGLWindow *window, GstGLWindowCB callback, gpointer data)
 {
+  g_debug ("CUSTOM IN\n");
   if (window)
   {
 
     GstGLWindowPrivate *priv = window->priv;
 
-    g_mutex_lock (priv->mutex);
+    g_mutex_lock (priv->glwin_lock);
 
-    if (!priv->is_closed)
+    if (priv->running)
     {
       XEvent event;
 
+      g_mutex_lock (priv->x_lock);
+
       event.xclient.type = ClientMessage;
       event.xclient.send_event = TRUE;
       event.xclient.display = priv->device;
@@ -578,9 +643,12 @@ gst_gl_window_send_message (GstGLWindow *window, GstGLWindowCB callback, gpointe
       XSendEvent (priv->device, priv->internal_win_id, FALSE, NoEventMask, &event);
       XFlush (priv->device);
 
-      g_cond_wait (priv->cond_send_message, priv->mutex);
+      g_mutex_unlock (priv->x_lock);
+
+      g_cond_wait (priv->cond_send_message, priv->glwin_lock);
     }
 
-    g_mutex_unlock (priv->mutex);
+    g_mutex_unlock (priv->glwin_lock);
   }
+  g_debug ("CUSTOM OUT\n");
 }

gst/gl/gstglimagesink.c

@@ -24,10 +24,10 @@
  * SECTION:element-glimagesink
  *
  * glimagesink renders video frames to a drawable on a local or remote
- * display using OpenGL. This element can receive a Window ID from the 
+ * display using OpenGL. This element can receive a Window ID from the
- * application through the XOverlay interface and will then render video 
+ * application through the XOverlay interface and will then render video
  * frames in this drawable.
- * If no Window ID was provided by the application, the element will 
+ * If no Window ID was provided by the application, the element will
  * create its own internal window and render into it.
  *
  * <refsect2>
@@ -46,7 +46,7 @@
  * <para>
  * Through the gl thread, glimagesink handle some events coming from the drawable
  * to manage its appearance even when the data is not flowing (GST_STATE_PAUSED).
- * That means that even when the element is paused, it will receive expose events 
+ * That means that even when the element is paused, it will receive expose events
  * from the drawable and draw the latest frame with correct borders/aspect-ratio.
  * </para>
  * </refsect2>
@@ -60,7 +60,7 @@
  * |[
  * gst-launch -v videotestsrc ! "video/x-raw-yuv, format=(fourcc)I420" ! glimagesink
  * ]| A pipeline to test hardware scaling and hardware colorspace conversion.
- * When your driver supports GLSL (OpenGL Shading Language needs OpenGL >= 2.1), 
+ * When your driver supports GLSL (OpenGL Shading Language needs OpenGL >= 2.1),
  * the 4 following format YUY2, UYVY, I420, YV12 and AYUV are converted to RGB32
  * through some fragment shaders and using one framebuffer (FBO extension OpenGL >= 1.4).
  * If your driver does not support GLSL but supports MESA_YCbCr extension then
@@ -74,7 +74,7 @@
  * |[
  * gst-plugins-gl/tests/examples/generic/cube
  * ]| The graphic FPS scene can be greater than the input video FPS.
- * The graphic scene can be written from a client code through the 
+ * The graphic scene can be written from a client code through the
  * two glfilterapp properties.
  * </refsect2>
  */
@@ -108,7 +108,6 @@ static GstFlowReturn gst_glimage_sink_render (GstBaseSink * bsink,
     GstBuffer * buf);
 static gboolean gst_glimage_sink_start (GstBaseSink * bsink);
 static gboolean gst_glimage_sink_stop (GstBaseSink * bsink);
-static gboolean gst_glimage_sink_unlock (GstBaseSink * bsink);
 
 static void gst_glimage_sink_xoverlay_init (GstXOverlayClass * iface);
 static void gst_glimage_sink_set_xwindow_id (GstXOverlay * overlay,
@@ -223,7 +222,6 @@ gst_glimage_sink_class_init (GstGLImageSinkClass* klass)
     gstbasesink_class->render = gst_glimage_sink_render;
     gstbasesink_class->start = gst_glimage_sink_start;
     gstbasesink_class->stop = gst_glimage_sink_stop;
-    gstbasesink_class->unlock = gst_glimage_sink_unlock;
 }
 
 static void
@@ -285,6 +283,8 @@ gst_glimage_sink_finalize (GObject* object)
         gst_caps_unref (glimage_sink->caps);
 
     g_free (glimage_sink->display_name);
+
+    GST_DEBUG ("finalized");
 }
 
 static void
@@ -395,16 +395,6 @@ gst_glimage_sink_stop (GstBaseSink* bsink)
     return TRUE;
 }
 
-static gboolean
-gst_glimage_sink_unlock (GstBaseSink* bsink)
-{
-    //GstGLImageSink* glimage_sink = GST_GLIMAGE_SINK (bsink);
-
-    GST_DEBUG ("unlock");
-
-    return TRUE;
-}
-
 static void
 gst_glimage_sink_get_times (GstBaseSink* bsink, GstBuffer* buf,
     GstClockTime* start, GstClockTime* end)
@@ -429,7 +419,6 @@ gst_glimage_sink_get_times (GstBaseSink* bsink, GstBuffer* buf,
     }
 }
 
-
 static gboolean
 gst_glimage_sink_set_caps (GstBaseSink* bsink, GstCaps* caps)
 {
@@ -525,7 +514,7 @@ gst_glimage_sink_render (GstBaseSink* bsink, GstBuffer* buf)
                 glimage_sink->width, glimage_sink->height);
 
             if (glimage_sink->window_id)
-                gst_gl_display_set_window_id (glimage_sink->display, glimage_sink->window_id); 
+                gst_gl_display_set_window_id (glimage_sink->display, glimage_sink->window_id);
 
             //init colorspace conversion if needed
             gst_gl_display_init_upload (glimage_sink->display, glimage_sink->format,
@@ -538,7 +527,7 @@ gst_glimage_sink_render (GstBaseSink* bsink, GstBuffer* buf)
             gst_gl_display_set_client_draw_callback (glimage_sink->display,
                 glimage_sink->clientDrawCallback);
 
-            gst_gl_display_set_visible_context (glimage_sink->display, TRUE);          
+            gst_gl_display_set_visible_context (glimage_sink->display, TRUE);
         }
 
         //blocking call
@@ -568,7 +557,7 @@ gst_glimage_sink_render (GstBaseSink* bsink, GstBuffer* buf)
             gl_buffer->texture, gl_buffer->width, gl_buffer->height))
         return GST_FLOW_OK;
     else
-        return GST_FLOW_UNEXPECTED;;
+        return GST_FLOW_UNEXPECTED;
 }