xvimageink: protect buffer_alloc from shutdown

Use the pool_lock in the buffer_alloc function to detect shutdown. Avoids
crashes when the sink is shutdown.

2 files changed, 54 insertions, 20 deletions

diff --git a/sys/xvimage/xvimagesink.c b/sys/xvimage/xvimagesink.c
index b5566efc..785a97cc 100644
--- a/sys/xvimage/xvimagesink.c
+++ b/sys/xvimage/xvimagesink.c
@@ -2209,9 +2209,17 @@ gst_xvimagesink_change_state (GstElement * element, GstStateChange transition)
       gst_xvimagesink_update_colorbalance (xvimagesink);
       break;
     case GST_STATE_CHANGE_READY_TO_PAUSED:
+      g_mutex_lock (xvimagesink->pool_lock);
+      xvimagesink->pool_invalid = FALSE;
+      g_mutex_unlock (xvimagesink->pool_lock);
       break;
     case GST_STATE_CHANGE_PAUSED_TO_PLAYING:
       break;
+    case GST_STATE_CHANGE_PAUSED_TO_READY:
+      g_mutex_lock (xvimagesink->pool_lock);
+      xvimagesink->pool_invalid = TRUE;
+      g_mutex_unlock (xvimagesink->pool_lock);
+      break;
     default:
       break;
   }
@@ -2336,9 +2344,14 @@ gst_xvimagesink_buffer_alloc (GstBaseSink * bsink, guint64 offset, guint size,
   GstCaps *intersection = NULL;
   GstStructure *structure = NULL;
   gint width, height, image_format;
+  GstCaps *new_caps;
 
   xvimagesink = GST_XVIMAGESINK (bsink);
 
+  g_mutex_lock (xvimagesink->pool_lock);
+  if (G_UNLIKELY (xvimagesink->pool_invalid))
+    goto invalid;
+
   if (G_LIKELY (xvimagesink->xcontext->last_caps &&
           gst_caps_is_equal (caps, xvimagesink->xcontext->last_caps))) {
     GST_DEBUG_OBJECT (xvimagesink,
@@ -2366,7 +2379,7 @@ gst_xvimagesink_buffer_alloc (GstBaseSink * bsink, guint64 offset, guint size,
 
   if (gst_caps_is_empty (intersection)) {
     /* So we don't support this kind of buffer, let's define one we'd like */
-    GstCaps *new_caps = gst_caps_copy (caps);
+    new_caps = gst_caps_copy (caps);
 
     structure = gst_caps_get_structure (new_caps, 0);
 
@@ -2392,15 +2405,8 @@ gst_xvimagesink_buffer_alloc (GstBaseSink * bsink, guint64 offset, guint size,
       gst_caps_unref (intersection);
       intersection = gst_caps_intersect (xvimagesink->xcontext->caps, new_caps);
 
-      if (gst_caps_is_empty (intersection)) {
-        GST_WARNING_OBJECT (xvimagesink, "we were requested a buffer with "
-            "caps %" GST_PTR_FORMAT ", but our xcontext caps %" GST_PTR_FORMAT
-            " are completely incompatible with those caps", new_caps,
-            xvimagesink->xcontext->caps);
-        gst_caps_unref (new_caps);
-        ret = GST_FLOW_UNEXPECTED;
-        goto beach;
-      }
+      if (gst_caps_is_empty (intersection))
+        goto incompatible;
     }
 
     /* Clean this copy */
@@ -2424,12 +2430,8 @@ gst_xvimagesink_buffer_alloc (GstBaseSink * bsink, guint64 offset, guint size,
   structure = gst_caps_get_structure (intersection, 0);
   if (!gst_structure_get_int (structure, "width", &width) ||
       !gst_structure_get_int (structure, "height", &height) ||
-      image_format == -1) {
-    GST_WARNING_OBJECT (xvimagesink, "invalid caps for buffer allocation %"
-        GST_PTR_FORMAT, intersection);
-    ret = GST_FLOW_UNEXPECTED;
-    goto beach;
-  }
+      image_format == -1)
+    goto invalid_caps;
 
   /* Store our caps and format as the last_caps to avoid expensive
    * caps intersection next time */
@@ -2440,8 +2442,6 @@ gst_xvimagesink_buffer_alloc (GstBaseSink * bsink, guint64 offset, guint size,
 
 reuse_last_caps:
 
-  g_mutex_lock (xvimagesink->pool_lock);
-
   /* Walking through the pool cleaning unusable images and searching for a
      suitable one */
   while (xvimagesink->image_pool) {
@@ -2466,8 +2466,6 @@ reuse_last_caps:
     }
   }
 
-  g_mutex_unlock (xvimagesink->pool_lock);
-
   if (!xvimage) {
     /* We found no suitable image in the pool. Creating... */
     GST_DEBUG_OBJECT (xvimagesink, "no usable image in pool, creating xvimage");
@@ -2480,6 +2478,7 @@ reuse_last_caps:
       xvimage = NULL;
     }
   }
+  g_mutex_unlock (xvimagesink->pool_lock);
 
   if (xvimage) {
     /* Make sure the buffer is cleared of any previously used flags */
@@ -2495,6 +2494,34 @@ beach:
   }
 
   return ret;
+
+  /* ERRORS */
+invalid:
+  {
+    GST_DEBUG_OBJECT (xvimagesink, "the pool is flushing");
+    ret = GST_FLOW_WRONG_STATE;
+    g_mutex_unlock (xvimagesink->pool_lock);
+    goto beach;
+  }
+incompatible:
+  {
+    GST_WARNING_OBJECT (xvimagesink, "we were requested a buffer with "
+        "caps %" GST_PTR_FORMAT ", but our xcontext caps %" GST_PTR_FORMAT
+        " are completely incompatible with those caps", new_caps,
+        xvimagesink->xcontext->caps);
+    gst_caps_unref (new_caps);
+    ret = GST_FLOW_NOT_NEGOTIATED;
+    g_mutex_unlock (xvimagesink->pool_lock);
+    goto beach;
+  }
+invalid_caps:
+  {
+    GST_WARNING_OBJECT (xvimagesink, "invalid caps for buffer allocation %"
+        GST_PTR_FORMAT, intersection);
+    ret = GST_FLOW_NOT_NEGOTIATED;
+    g_mutex_unlock (xvimagesink->pool_lock);
+    goto beach;
+  }
 }
 
 /* Interfaces stuff */
@@ -3136,6 +3163,12 @@ gst_xvimagesink_reset (GstXvImageSink * xvimagesink)
   xvimagesink->event_thread = NULL;
   GST_OBJECT_UNLOCK (xvimagesink);
 
+  /* invalidate the pool, current allocations continue, new buffer_alloc fails
+   * with wrong_state */
+  g_mutex_lock (xvimagesink->pool_lock);
+  xvimagesink->pool_invalid = TRUE;
+  g_mutex_unlock (xvimagesink->pool_lock);
+
   /* Wait for our event thread to finish before we clean up our stuff. */
   if (thread)
     g_thread_join (thread);
diff --git a/sys/xvimage/xvimagesink.h b/sys/xvimage/xvimagesink.h
index 03400b65..56cf5945 100644
--- a/sys/xvimage/xvimagesink.h
+++ b/sys/xvimage/xvimagesink.h
@@ -250,6 +250,7 @@ struct _GstXvImageSink {
   GValue *par;
 
   GMutex *pool_lock;
+  gboolean pool_invalid;
   GSList *image_pool;
 
   gboolean synchronous;