funnel: Fix for racy EOS event handling

When eos events are forwarded simultaneouly from two sinkpads on
funnel, it doesnot forward the eos to sourcepad. The reason is
sticky events are stored after the event callbacks are returned.
Therefore while one is about to store the sticky events on the its
sinkpad, other sinkpad starts checking for the eos events on all other
sinkpads and assumes eos is not present yet.

https://bugzilla.gnome.org/show_bug.cgi?id=732851

diff --git a/plugins/elements/gstfunnel.c b/plugins/elements/gstfunnel.c
index a58ae72032278d2b0f406cd2447251241a62cf88..19de206d261a0666c9f61d19bb65215b026ae40c 100644 (file)
--- a/plugins/elements/gstfunnel.c
+++ b/plugins/elements/gstfunnel.c
@@ -45,6 +45,48 @@
 GST_DEBUG_CATEGORY_STATIC (gst_funnel_debug);
 #define GST_CAT_DEFAULT gst_funnel_debug
 
+GType gst_funnel_pad_get_type (void);
+#define GST_TYPE_FUNNEL_PAD \
+  (gst_funnel_pad_get_type())
+#define GST_FUNNEL_PAD(obj) \
+  (G_TYPE_CHECK_INSTANCE_CAST ((obj), GST_TYPE_FUNNEL_PAD, GstFunnelPad))
+#define GST_FUNNEL_PAD_CLASS(klass) \
+  (G_TYPE_CHECK_CLASS_CAST ((klass), GST_TYPE_FUNNEL_PAD, GstFunnelPadClass))
+#define GST_IS_FUNNEL_PAD(obj) \
+  (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GST_TYPE_FUNNEL_PAD))
+#define GST_IS_FUNNEL_PAD_CLASS(klass) \
+  (G_TYPE_CHECK_CLASS_TYPE ((klass), GST_TYPE_FUNNEL_PAD))
+#define GST_FUNNEL_PAD_CAST(obj) \
+  ((GstFunnelPad *)(obj))
+
+typedef struct _GstFunnelPad GstFunnelPad;
+typedef struct _GstFunnelPadClass GstFunnelPadClass;
+
+struct _GstFunnelPad
+{
+  GstPad parent;
+
+  gboolean got_eos;
+};
+
+struct _GstFunnelPadClass
+{
+  GstPadClass parent;
+};
+
+G_DEFINE_TYPE (GstFunnelPad, gst_funnel_pad, GST_TYPE_PAD);
+
+static void
+gst_funnel_pad_class_init (GstFunnelPadClass * klass)
+{
+}
+
+static void
+gst_funnel_pad_init (GstFunnelPad * pad)
+{
+  pad->got_eos = FALSE;
+}
+
 static GstStaticPadTemplate funnel_sink_template =
 GST_STATIC_PAD_TEMPLATE ("sink_%u",
     GST_PAD_SINK,
@@ -62,6 +104,8 @@ GST_STATIC_PAD_TEMPLATE ("src",
 #define gst_funnel_parent_class parent_class
 G_DEFINE_TYPE_WITH_CODE (GstFunnel, gst_funnel, GST_TYPE_ELEMENT, _do_init);
 
+static GstStateChangeReturn gst_funnel_change_state (GstElement * element,
+    GstStateChange transition);
 static GstPad *gst_funnel_request_new_pad (GstElement * element,
     GstPadTemplate * templ, const gchar * name, const GstCaps * caps);
 static void gst_funnel_release_pad (GstElement * element, GstPad * pad);
@@ -112,6 +156,7 @@ gst_funnel_class_init (GstFunnelClass * klass)
   gstelement_class->request_new_pad =
       GST_DEBUG_FUNCPTR (gst_funnel_request_new_pad);
   gstelement_class->release_pad = GST_DEBUG_FUNCPTR (gst_funnel_release_pad);
+  gstelement_class->change_state = GST_DEBUG_FUNCPTR (gst_funnel_change_state);
 }
 
 static void
@@ -132,7 +177,9 @@ gst_funnel_request_new_pad (GstElement * element, GstPadTemplate * templ,
 
   GST_DEBUG_OBJECT (element, "requesting pad");
 
-  sinkpad = gst_pad_new_from_static_template (&funnel_sink_template, name);
+  sinkpad = GST_PAD_CAST (g_object_new (GST_TYPE_FUNNEL_PAD,
+          "name", name, "direction", templ->direction, "template", templ,
+          NULL));
 
   gst_pad_set_chain_function (sinkpad,
       GST_DEBUG_FUNCPTR (gst_funnel_sink_chain));
@@ -156,31 +203,21 @@ gst_funnel_all_sinkpads_eos_unlocked (GstFunnel * funnel, GstPad * pad)
   GList *item;
   gboolean all_eos = FALSE;
 
-  GST_OBJECT_LOCK (funnel);
 
   if (element->numsinkpads == 0)
     goto done;
 
   for (item = element->sinkpads; item != NULL; item = g_list_next (item)) {
-    GstPad *sinkpad = item->data;
-    GstEvent *eos;
+    GstFunnelPad *sinkpad = GST_FUNNEL_PAD_CAST (item->data);
 
-    /* eos event has not enrolled for current pad, we don't check for current pad */
-    if (pad == sinkpad)
-      continue;
-
-    eos = gst_pad_get_sticky_event (sinkpad, GST_EVENT_EOS, 0);
-    if (eos)
-      gst_event_unref (eos);
-
-    if (eos == NULL)
-      goto done;
+    if (sinkpad->got_eos == FALSE) {
+      return FALSE;
+    }
   }
 
   all_eos = TRUE;
 
 done:
-  GST_OBJECT_UNLOCK (funnel);
   return all_eos;
 }
 
@@ -188,23 +225,24 @@ static void
 gst_funnel_release_pad (GstElement * element, GstPad * pad)
 {
   GstFunnel *funnel = GST_FUNNEL (element);
-  GstEvent *eos;
+  GstFunnelPad *fpad = GST_FUNNEL_PAD_CAST (pad);
+  gboolean got_eos;
   gboolean send_eos = FALSE;
 
   GST_DEBUG_OBJECT (funnel, "releasing pad");
 
   gst_pad_set_active (pad, FALSE);
 
-  eos = gst_pad_get_sticky_event (pad, GST_EVENT_EOS, 0);
-  if (eos)
-    gst_event_unref (eos);
+  got_eos = fpad->got_eos;
 
   gst_element_remove_pad (GST_ELEMENT_CAST (funnel), pad);
 
-  if (eos == NULL && gst_funnel_all_sinkpads_eos_unlocked (funnel, NULL)) {
+  GST_OBJECT_LOCK (funnel);
+  if (!got_eos && gst_funnel_all_sinkpads_eos_unlocked (funnel, NULL)) {
     GST_DEBUG_OBJECT (funnel, "Pad removed. All others are EOS. Sending EOS");
     send_eos = TRUE;
   }
+  GST_OBJECT_UNLOCK (funnel);
 
   if (send_eos)
     if (!gst_pad_push_event (funnel->srcpad, gst_event_new_eos ()))
@@ -252,6 +290,7 @@ static gboolean
 gst_funnel_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)
 {
   GstFunnel *funnel = GST_FUNNEL (parent);
+  GstFunnelPad *fpad = GST_FUNNEL_PAD_CAST (pad);
   gboolean forward = TRUE;
   gboolean res = TRUE;
   gboolean unlock = FALSE;
@@ -261,14 +300,23 @@ gst_funnel_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)
     GST_PAD_STREAM_LOCK (funnel->srcpad);
 
     if (GST_EVENT_TYPE (event) == GST_EVENT_EOS) {
+      GST_OBJECT_LOCK (funnel);
+      fpad->got_eos = TRUE;
       if (!gst_funnel_all_sinkpads_eos_unlocked (funnel, pad)) {
         forward = FALSE;
       } else {
         forward = TRUE;
       }
+      GST_OBJECT_UNLOCK (funnel);
     } else if (pad != funnel->last_sinkpad) {
       forward = FALSE;
     }
+  } else if (GST_EVENT_TYPE (event) == GST_EVENT_FLUSH_STOP) {
+    unlock = TRUE;
+    GST_PAD_STREAM_LOCK (funnel->srcpad);
+    GST_OBJECT_LOCK (funnel);
+    fpad->got_eos = FALSE;
+    GST_OBJECT_UNLOCK (funnel);
   }
 
   if (forward)
@@ -281,3 +329,47 @@ gst_funnel_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)
 
   return res;
 }
+
+static void
+reset_pad (const GValue * data, gpointer user_data)
+{
+  GstPad *pad = g_value_get_object (data);
+  GstFunnelPad *fpad = GST_FUNNEL_PAD_CAST (pad);
+
+  GST_OBJECT_LOCK (pad);
+  fpad->got_eos = FALSE;
+  GST_OBJECT_UNLOCK (pad);
+}
+
+static GstStateChangeReturn
+gst_funnel_change_state (GstElement * element, GstStateChange transition)
+{
+  GstStateChangeReturn ret;
+
+  ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);
+  if (ret == GST_STATE_CHANGE_FAILURE)
+    return ret;
+
+  switch (transition) {
+    case GST_STATE_CHANGE_PAUSED_TO_READY:
+    {
+      GstIterator *iter = gst_element_iterate_sink_pads (element);
+      GstIteratorResult res;
+
+      do {
+        res = gst_iterator_foreach (iter, reset_pad, NULL);
+      } while (res == GST_ITERATOR_RESYNC);
+
+      gst_iterator_free (iter);
+
+      if (res == GST_ITERATOR_ERROR)
+        return GST_STATE_CHANGE_FAILURE;
+
+    }
+      break;
+    default:
+      break;
+  }
+
+  return ret;
+}