glwindow: Use g_thread_join() instead of a custom condition variable for waiting for the navigation thread to finish

Also hide some internal functions and fields while we're at it and fix
a race condition with the startup condition variable.

2 files changed, 50 insertions, 88 deletions

diff --git a/gst-libs/gst/gl/gstglwindow.c b/gst-libs/gst/gl/gstglwindow.c
index 90ea219be..3c12d1c1d 100644
--- a/gst-libs/gst/gl/gstglwindow.c
+++ b/gst-libs/gst/gl/gstglwindow.c
@@ -85,10 +85,6 @@ static void gst_gl_window_default_send_message (GstGLWindow * window,
 static void gst_gl_window_default_send_message_async (GstGLWindow * window,
     GstGLWindowCB callback, gpointer data, GDestroyNotify destroy);
 static gpointer gst_gl_window_navigation_thread (GstGLWindow * window);
-void gst_gl_window_run_navigation (GstGLWindow * window);
-void gst_gl_window_open_navigation (GstGLWindow * window);
-void gst_gl_window_close_navigation (GstGLWindow * window);
-void gst_gl_window_quit_navigation (GstGLWindow * window);
 
 struct _GstGLWindowPrivate
 {
@@ -100,6 +96,10 @@ struct _GstGLWindowPrivate
   guint surface_height;
 
   gboolean alive;
+
+  GMutex nav_lock;
+  GCond nav_create_cond;
+  gboolean nav_alive;
 };
 
 static void gst_gl_window_finalize (GObject * object);
@@ -187,13 +187,12 @@ gst_gl_window_init (GstGLWindow * window)
   window->priv = priv;
 
   g_mutex_init (&window->lock);
-  g_mutex_init (&window->nav_lock);
-  g_cond_init (&window->nav_create_cond);
-  g_cond_init (&window->nav_destroy_cond);
-  window->nav_created = FALSE;
-  window->nav_alive = FALSE;
   window->is_drawing = FALSE;
 
+  g_mutex_init (&window->priv->nav_lock);
+  g_cond_init (&window->priv->nav_create_cond);
+  window->priv->nav_alive = FALSE;
+
   g_weak_ref_init (&window->context_ref, NULL);
 
   priv->main_context = g_main_context_new ();
@@ -310,16 +309,13 @@ gst_gl_window_new (GstGLDisplay * display)
 
   window->display = gst_object_ref (display);
 
-  g_mutex_lock (&window->nav_lock);
+  g_mutex_lock (&window->priv->nav_lock);
+  window->priv->navigation_thread = g_thread_new ("gstglnavigation",
+      (GThreadFunc) gst_gl_window_navigation_thread, window);
 
-  if (!window->nav_created) {
-    window->priv->navigation_thread = g_thread_new ("gstglnavigation",
-        (GThreadFunc) gst_gl_window_navigation_thread, window);
-
-    g_cond_wait (&window->nav_create_cond, &window->nav_lock);
-    window->nav_created = TRUE;
-  }
-  g_mutex_unlock (&window->nav_lock);
+  while (!window->priv->nav_alive)
+    g_cond_wait (&window->priv->nav_create_cond, &window->priv->nav_lock);
+  g_mutex_unlock (&window->priv->nav_lock);
 
   return window;
 }
@@ -330,18 +326,11 @@ gst_gl_window_finalize (GObject * object)
   GstGLWindow *window = GST_GL_WINDOW (object);
   GstGLWindowPrivate *priv = window->priv;
 
-  if (window->nav_alive) {
-    g_mutex_lock (&window->nav_lock);
-    GST_INFO ("send quit navigation loop");
-    gst_gl_window_quit_navigation (window);
-    while (window->nav_alive) {
-      g_cond_wait (&window->nav_destroy_cond, &window->nav_lock);
-    }
-    /* release resource hold by navigation thread */
-    g_thread_join(window->priv->navigation_thread);
-    window->priv->navigation_thread = NULL;
-    g_mutex_unlock (&window->nav_lock);
-  }
+  GST_INFO ("quit navigation loop");
+  g_main_loop_quit (window->navigation_loop);
+  /* wait until navigation thread finished */
+  g_thread_join (window->priv->navigation_thread);
+  window->priv->navigation_thread = NULL;
 
   if (priv->loop)
     g_main_loop_unref (priv->loop);
@@ -352,9 +341,8 @@ gst_gl_window_finalize (GObject * object)
   g_weak_ref_clear (&window->context_ref);
 
   g_mutex_clear (&window->lock);
-  g_mutex_clear (&window->nav_lock);
-  g_cond_clear (&window->nav_create_cond);
-  g_cond_clear (&window->nav_destroy_cond);
+  g_mutex_clear (&window->priv->nav_lock);
+  g_cond_clear (&window->priv->nav_create_cond);
   gst_object_unref (window->display);
 
   G_OBJECT_CLASS (gst_gl_window_parent_class)->finalize (object);
@@ -561,23 +549,6 @@ gst_gl_window_run (GstGLWindow * window)
   window_class->run (window);
 }
 
-/**
- * gst_gl_window_run_navigation:
- * @window: a #GstGLWindow
- *
- * Start the execution of the navigation runloop.
- *
- * Since: 1.6
- */
-void
-gst_gl_window_run_navigation (GstGLWindow * window)
-{
-  g_return_if_fail (GST_IS_GL_WINDOW (window));
-  g_return_if_fail (window->navigation_context != NULL);
-  g_return_if_fail (window->navigation_loop != NULL);
-  g_main_loop_run (window->navigation_loop);
-}
-
 static void
 gst_gl_window_default_quit (GstGLWindow * window)
 {
@@ -929,50 +900,46 @@ GType gst_gl_dummy_window_get_type (void);
 
 G_DEFINE_TYPE (GstGLDummyWindow, gst_gl_dummy_window, GST_GL_TYPE_WINDOW);
 
-void
-gst_gl_window_open_navigation (GstGLWindow * window)
+static gboolean
+gst_gl_window_navigation_started (gpointer data)
 {
-  g_return_if_fail (GST_IS_GL_WINDOW (window));
-  g_mutex_lock (&window->nav_lock);
+  GstGLWindow *window = data;
+
+  g_mutex_lock (&window->priv->nav_lock);
+  window->priv->nav_alive = TRUE;
+  g_cond_signal (&window->priv->nav_create_cond);
+  g_mutex_unlock (&window->priv->nav_lock);
+
+  return G_SOURCE_REMOVE;
+}
+
+static gpointer
+gst_gl_window_navigation_thread (GstGLWindow * window)
+{
+  GSource *source;
+
+  g_return_val_if_fail (GST_IS_GL_WINDOW (window), NULL);
+
   window->navigation_context = g_main_context_new ();
   window->navigation_loop = g_main_loop_new (window->navigation_context, FALSE);
   g_main_context_push_thread_default (window->navigation_context);
-  window->nav_alive = TRUE;
-  g_cond_signal (&window->nav_create_cond);
-  g_mutex_unlock (&window->nav_lock);
-}
 
-void
-gst_gl_window_close_navigation (GstGLWindow * window)
-{
-  g_return_if_fail (GST_IS_GL_WINDOW (window));
-  g_return_if_fail (window->navigation_context != NULL);
-  g_return_if_fail (window->navigation_loop != NULL);
+  source = g_idle_source_new ();
+  g_source_set_callback (source, (GSourceFunc) gst_gl_window_navigation_started,
+      window, NULL);
+  g_source_attach (source, window->navigation_context);
+  g_source_unref (source);
+
+  g_main_loop_run (window->navigation_loop);
 
-  g_mutex_lock (&window->nav_lock);
-  window->nav_alive = FALSE;
   g_main_context_pop_thread_default (window->navigation_context);
+
   g_main_loop_unref (window->navigation_loop);
   g_main_context_unref (window->navigation_context);
-  g_cond_signal (&window->nav_destroy_cond);
-  g_mutex_unlock (&window->nav_lock);
-}
-
-void
-gst_gl_window_quit_navigation (GstGLWindow * window)
-{
-  g_return_if_fail (GST_IS_GL_WINDOW (window));
+  window->navigation_loop = NULL;
+  window->navigation_context = NULL;
 
-  g_main_loop_quit (window->navigation_loop);
-}
-
-static gpointer
-gst_gl_window_navigation_thread (GstGLWindow * window)
-{
-  gst_gl_window_open_navigation (window);
-  gst_gl_window_run_navigation (window);
   GST_INFO ("navigation loop exited\n");
-  gst_gl_window_close_navigation (window);
 
   return NULL;
 }
diff --git a/gst-libs/gst/gl/gstglwindow.h b/gst-libs/gst/gl/gstglwindow.h
index 7ea26f16e..3e3782ab9 100644
--- a/gst-libs/gst/gl/gstglwindow.h
+++ b/gst-libs/gst/gl/gstglwindow.h
@@ -67,11 +67,6 @@ struct _GstGLWindow {
   GstObject parent;
 
   GMutex        lock;
-  GMutex        nav_lock;
-  GCond         nav_create_cond;
-  GCond         nav_destroy_cond;
-  gboolean      nav_created;
-  gboolean      nav_alive;
 
   GstGLDisplay *display;
   GWeakRef      context_ref;