cairoxsink: Refactor setup codeHEADmaster

--- and take the stream lock on set_xwindow_id() to avoid races leading to
crashes.

1 files changed, 85 insertions, 51 deletions

diff --git a/ext/xlib/gstcairoxsink.c b/ext/xlib/gstcairoxsink.c
index 67c6aac..dfdd28c 100644
--- a/ext/xlib/gstcairoxsink.c
+++ b/ext/xlib/gstcairoxsink.c
@@ -130,15 +130,84 @@ gst_cairo_x_sink_create_window (GstCairoXSink * xsink)
   cairo_device_release (xsink->device);
 }
 
+static gboolean
+gst_cairo_x_sink_ensure_surface (GstCairoXSink * xsink,
+    cairo_surface_type_t type)
+{
+  const GstCairoXTarget *target;
+
+  if (xsink->window_surface == NULL) {
+    GST_ERROR_OBJECT (xsink, "Tried to ensure a target on inactive sink");
+    return FALSE;
+  }
+
+  target = gst_cairo_x_target_get (type);
+  if (target == NULL) {
+    GST_ERROR_OBJECT (xsink, "Failed to get a target for surface type %d",
+        type);
+    return FALSE;
+  }
+
+  if (xsink->target != target) {
+    cairo_surface_t *new_surface;
+
+    new_surface = target->create_window (xsink->window_surface);
+
+    if (new_surface == NULL) {
+      GST_ERROR_OBJECT (xsink, "Failed to get a target for surface type %d",
+          type);
+      return FALSE;
+    }
+
+    if (xsink->surface)
+      cairo_surface_destroy (xsink->surface);
+    xsink->target = target;
+    xsink->surface = new_surface;
+    gst_cairo_x_sink_update_events (xsink);
+  }
+
+  return TRUE;
+}
+
+/* NB: We accept width/height == 0 for no resize */
+static void
+gst_cairo_x_sink_show (GstCairoXSink * xsink, guint width, guint height)
+{
+  Display *display;
+  Window window;
+
+  if (cairo_device_acquire (xsink->device) != CAIRO_STATUS_SUCCESS)
+    return;
+
+  display = cairo_xlib_device_get_display (xsink->device);
+  window = xsink->target->get_window_id (xsink->surface);
+
+  if (xsink->window_id == 0) {
+    if (width != 0) {
+      cairo_xlib_surface_set_size (xsink->window_surface, width, height);
+      XResizeWindow (display, window, width, height);
+      XMapWindow (display, window);
+    }
+    gst_x_overlay_got_xwindow_id (GST_X_OVERLAY (xsink), window);
+  } else {
+    XMapWindow (display, window);
+  }
+
+  cairo_device_release (xsink->device);
+}
+
 static void
 gst_cairo_x_sink_xoverlay_set_xwindow_id (GstXOverlay * overlay,
     gulong xwindow_id)
 {
   GstCairoXSink *xsink = GST_CAIRO_X_SINK (overlay);
+  GstBaseSink *bsink = GST_BASE_SINK (overlay);
 
   if (xsink->window_id == xwindow_id)
     return;
 
+  GST_PAD_STREAM_LOCK (bsink->sinkpad);
+
   if (xsink->window_surface && xsink->window_id &&
       cairo_device_acquire (xsink->device) == CAIRO_STATUS_SUCCESS) {
     XSelectInput (cairo_xlib_device_get_display (xsink->device),
@@ -159,7 +228,15 @@ gst_cairo_x_sink_xoverlay_set_xwindow_id (GstXOverlay * overlay,
     cairo_surface_destroy (xsink->window_surface);
     xsink->window_surface = NULL;
     gst_cairo_x_sink_create_window (xsink);
+
+    if (xsink->format) {
+      if (gst_cairo_x_sink_ensure_surface (xsink,
+              gst_cairo_format_get_surface_type (xsink->format)))
+        gst_cairo_x_sink_show (xsink, 0, 0);
+    }
   }
+
+  GST_PAD_STREAM_UNLOCK (bsink->sinkpad);
 }
 
 static void
@@ -403,7 +480,6 @@ static gboolean
 gst_cairo_x_sink_set_caps (GstBaseSink * bsink, GstCaps * caps)
 {
   GstCairoXSink *xsink = GST_CAIRO_X_SINK (bsink);
-  const GstCairoXTarget *target;
   GstCairoFormat *format;
   int width, height;
 
@@ -411,47 +487,17 @@ gst_cairo_x_sink_set_caps (GstBaseSink * bsink, GstCaps * caps)
   width = gst_cairo_format_get_width (format),
       height = gst_cairo_format_get_height (format);
 
-  target = gst_cairo_x_target_get (gst_cairo_format_get_surface_type (format));
-  if (target == NULL) {
-    GST_ERROR_OBJECT (xsink, "Failed to get a target for surface type %d",
-        gst_cairo_format_get_surface_type (format));
+  if (!gst_cairo_x_sink_ensure_surface (xsink,
+          gst_cairo_format_get_surface_type (format))) {
     gst_cairo_format_free (format);
     return FALSE;
   }
-  if (xsink->target != target) {
-    cairo_surface_t *new_surface =
-        target->create_window (xsink->window_surface);
-    if (new_surface == NULL) {
-      GST_ERROR_OBJECT (xsink, "Failed to get a target for surface type %d",
-          gst_cairo_format_get_surface_type (format));
-      gst_cairo_format_free (format);
-      return FALSE;
-    }
-    if (xsink->surface)
-      cairo_surface_destroy (xsink->surface);
-    xsink->target = target;
-    xsink->surface = new_surface;
-    gst_cairo_x_sink_update_events (xsink);
-  }
-
-  if (cairo_device_acquire (xsink->device) == CAIRO_STATUS_SUCCESS) {
-    Display *display = cairo_xlib_device_get_display (xsink->device);
-    Window window = xsink->target->get_window_id (xsink->surface);
-    if (xsink->window_id == 0) {
-      cairo_xlib_surface_set_size (xsink->window_surface, width, height);
-      XResizeWindow (display, window, width, height);
-      XMapWindow (display, window);
-      gst_x_overlay_got_xwindow_id (GST_X_OVERLAY (xsink), window);
-    } else {
-      XMapWindow (display, window);
-    }
-
-    cairo_device_release (xsink->device);
-  }
 
   gst_cairo_format_free (xsink->format);
   xsink->format = format;
 
+  gst_cairo_x_sink_show (xsink, width, height);
+
   return TRUE;
 }
 
@@ -562,22 +608,10 @@ gst_cairo_x_sink_buffer_alloc (GstBaseSink * bsink, guint64 offset,
 
   format = gst_cairo_format_new (caps);
   if (gst_cairo_format_is_native (format) && xsink->window_surface) {
-    if (xsink->target == NULL) {
-      /* happens on first buffer alloc */
-      xsink->target =
-          gst_cairo_x_target_get (gst_cairo_format_get_surface_type (format));
-      if (xsink->target) {
-        xsink->surface = xsink->target->create_window (xsink->window_surface);
-        gst_cairo_x_sink_update_events (xsink);
-      }
-    }
-    if (xsink->surface &&
-        (gst_cairo_format_get_surface_type (format) == CAIRO_SURFACE_TYPE_IMAGE
-            || gst_cairo_format_get_surface_type (format) ==
-            cairo_surface_get_type (xsink->surface)))
-      *buf = gst_cairo_buffer_new_similar (xsink->surface, format);
-    else
-      *buf = NULL;
+    if (!gst_cairo_x_sink_ensure_surface (xsink,
+            gst_cairo_format_get_surface_type (format)))
+      return GST_FLOW_NOT_NEGOTIATED;
+    *buf = gst_cairo_buffer_new_similar (xsink->surface, format);
   } else {
     *buf = NULL;
   }