bin: implement context propagation when adding elements
authorMatthew Waters <matthew@centricular.com>
Wed, 23 Sep 2015 14:04:48 +0000 (00:04 +1000)
committerMatthew Waters <matthew@centricular.com>
Mon, 28 Sep 2015 08:21:59 +0000 (18:21 +1000)
When adding an element to a bin we need to propagate the GstContext's
to/from the element.

This moves the GstContext list from GstBin to GstElement and adds
convenience functions to get the currently set list of GstContext's.

This does not deal with the collection of GstContext's propagated
using GST_CONTEXT_QUERY.  Element subclasses are advised to call
gst_element_set_context if they need to propagate GstContext's
received from the context query.

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

docs/gst/gstreamer-sections.txt
gst/gstbin.c
gst/gstelement.c
gst/gstelement.h
tests/check/gst/gstcontext.c
win32/common/libgstreamer.def

index e2aaf9d..5f5e577 100644 (file)
@@ -869,6 +869,9 @@ gst_element_get_start_time
 gst_element_set_bus
 gst_element_get_bus
 gst_element_set_context
+gst_element_get_context
+gst_element_get_context_unlocked
+gst_element_get_contexts
 gst_element_get_factory
 gst_element_set_name
 gst_element_get_name
index 9cea0b3..9190bfe 100644 (file)
@@ -197,8 +197,6 @@ struct _GstBinPrivate
 
   gboolean posted_eos;
   gboolean posted_playing;
-
-  GList *contexts;
 };
 
 typedef struct
@@ -228,6 +226,9 @@ static void bin_do_eos (GstBin * bin);
 
 static gboolean gst_bin_add_func (GstBin * bin, GstElement * element);
 static gboolean gst_bin_remove_func (GstBin * bin, GstElement * element);
+static void gst_bin_update_context (GstBin * bin, GstContext * context);
+static void gst_bin_update_context_unlocked (GstBin * bin,
+    GstContext * context);
 
 #if 0
 static void gst_bin_set_index_func (GstElement * element, GstIndex * index);
@@ -530,8 +531,6 @@ gst_bin_dispose (GObject * object)
         GST_STR_NULL (GST_OBJECT_NAME (object)));
   }
 
-  g_list_free_full (bin->priv->contexts, (GDestroyNotify) gst_context_unref);
-
   G_OBJECT_CLASS (parent_class)->dispose (object);
 }
 
@@ -1096,7 +1095,7 @@ gst_bin_add_func (GstBin * bin, GstElement * element)
   gboolean is_sink, is_source, provides_clock, requires_clock;
   GstMessage *clock_message = NULL, *async_message = NULL;
   GstStateChangeReturn ret;
-  GList *l;
+  GList *l, *elem_contexts, *need_context_messages;
 
   GST_DEBUG_OBJECT (bin, "element :%s", GST_ELEMENT_NAME (element));
 
@@ -1168,9 +1167,36 @@ gst_bin_add_func (GstBin * bin, GstElement * element)
    * a new clock will be selected */
   gst_element_set_clock (element, GST_ELEMENT_CLOCK (bin));
 
-  for (l = bin->priv->contexts; l; l = l->next)
+  /* get the element's list of contexts before propagating our own */
+  elem_contexts = gst_element_get_contexts (element);
+  for (l = GST_ELEMENT_CAST (bin)->contexts; l; l = l->next)
     gst_element_set_context (element, l->data);
 
+  need_context_messages = NULL;
+  for (l = elem_contexts; l; l = l->next) {
+    GstContext *replacement, *context = l->data;
+    const gchar *context_type;
+
+    context_type = gst_context_get_context_type (context);
+
+    /* we already set this context above? */
+    replacement =
+        gst_element_get_context_unlocked (GST_ELEMENT (bin), context_type);
+    if (replacement) {
+      gst_context_unref (replacement);
+    } else {
+      GstMessage *msg;
+      GstStructure *s;
+
+      /* ask our parent for the context */
+      msg = gst_message_new_need_context (GST_OBJECT_CAST (bin), context_type);
+      s = (GstStructure *) gst_message_get_structure (msg);
+      gst_structure_set (s, "bin.old.context", GST_TYPE_CONTEXT, context, NULL);
+
+      need_context_messages = g_list_prepend (need_context_messages, msg);
+    }
+  }
+
 #if 0
   /* set the cached index on the children */
   if (bin->priv->index)
@@ -1209,6 +1235,49 @@ gst_bin_add_func (GstBin * bin, GstElement * element)
 no_state_recalc:
   GST_OBJECT_UNLOCK (bin);
 
+  for (l = need_context_messages; l; l = l->next) {
+    GstMessage *msg = l->data;
+    GstStructure *s;
+    const gchar *context_type;
+    GstContext *replacement, *context;
+
+    gst_message_parse_context_type (msg, &context_type);
+
+    GST_LOG_OBJECT (bin, "asking parent for context type: %s "
+        "from %" GST_PTR_FORMAT, context_type, element);
+
+    s = (GstStructure *) gst_message_get_structure (msg);
+    gst_structure_get (s, "bin.old.context", GST_TYPE_CONTEXT, &context, NULL);
+    gst_structure_remove_field (s, "bin.old.context");
+    gst_element_post_message (GST_ELEMENT_CAST (bin), msg);
+
+    /* lock to avoid losing a potential write */
+    GST_OBJECT_LOCK (bin);
+    replacement =
+        gst_element_get_context_unlocked (GST_ELEMENT_CAST (bin), context_type);
+
+    if (replacement) {
+      /* we got the context set from GstElement::set_context */
+      gst_context_unref (replacement);
+      GST_OBJECT_UNLOCK (bin);
+    } else {
+      /* Propagate the element's context upwards */
+      GST_LOG_OBJECT (bin, "propagating existing context type: %s %p "
+          "from %" GST_PTR_FORMAT, context_type, context, element);
+
+      gst_bin_update_context_unlocked (bin, context);
+
+      msg =
+          gst_message_new_have_context (GST_OBJECT_CAST (bin),
+          gst_context_ref (context));
+      GST_OBJECT_UNLOCK (bin);
+      gst_element_post_message (GST_ELEMENT_CAST (bin), msg);
+    }
+    gst_context_unref (context);
+  }
+  g_list_free_full (elem_contexts, (GDestroyNotify) gst_context_unref);
+  g_list_free (need_context_messages);
+
   /* post the messages on the bus of the element so that the bin can handle
    * them */
   if (clock_message)
@@ -2631,28 +2700,8 @@ gst_bin_change_state_func (GstElement * element, GstStateChange transition)
       break;
     case GST_STATE_NULL:
       if (current == GST_STATE_READY) {
-        GList *l;
-
         if (!(gst_bin_src_pads_activate (bin, FALSE)))
           goto activate_failure;
-
-        /* Remove all non-persistent contexts */
-        GST_OBJECT_LOCK (bin);
-        for (l = bin->priv->contexts; l;) {
-          GstContext *context = l->data;
-
-          if (!gst_context_is_persistent (context)) {
-            GList *next;
-
-            gst_context_unref (context);
-            next = l->next;
-            bin->priv->contexts = g_list_delete_link (bin->priv->contexts, l);
-            l = next;
-          } else {
-            l = l->next;
-          }
-        }
-        GST_OBJECT_UNLOCK (bin);
       }
       break;
     default:
@@ -3361,12 +3410,23 @@ bin_do_message_forward (GstBin * bin, GstMessage * message)
 static void
 gst_bin_update_context (GstBin * bin, GstContext * context)
 {
-  GList *l;
+  GST_OBJECT_LOCK (bin);
+  gst_bin_update_context_unlocked (bin, context);
+  GST_OBJECT_UNLOCK (bin);
+}
+
+static void
+gst_bin_update_context_unlocked (GstBin * bin, GstContext * context)
+{
   const gchar *context_type;
+  GList *l, **contexts;
 
-  GST_OBJECT_LOCK (bin);
+  contexts = &GST_ELEMENT_CAST (bin)->contexts;
   context_type = gst_context_get_context_type (context);
-  for (l = bin->priv->contexts; l; l = l->next) {
+
+  GST_DEBUG_OBJECT (bin, "set context %p %" GST_PTR_FORMAT, context,
+      gst_context_get_structure (context));
+  for (l = *contexts; l; l = l->next) {
     GstContext *tmp = l->data;
     const gchar *tmp_type = gst_context_get_context_type (tmp);
 
@@ -3380,10 +3440,9 @@ gst_bin_update_context (GstBin * bin, GstContext * context)
     }
   }
   /* Not found? Add */
-  if (l == NULL)
-    bin->priv->contexts =
-        g_list_prepend (bin->priv->contexts, gst_context_ref (context));
-  GST_OBJECT_UNLOCK (bin);
+  if (l == NULL) {
+    *contexts = g_list_prepend (*contexts, gst_context_ref (context));
+  }
 }
 
 /* handle child messages:
@@ -3746,11 +3805,13 @@ gst_bin_handle_message_func (GstBin * bin, GstMessage * message)
     }
     case GST_MESSAGE_NEED_CONTEXT:{
       const gchar *context_type;
-      GList *l;
+      GList *l, *contexts;
 
       gst_message_parse_context_type (message, &context_type);
       GST_OBJECT_LOCK (bin);
-      for (l = bin->priv->contexts; l; l = l->next) {
+      contexts = GST_ELEMENT_CAST (bin)->contexts;
+      GST_LOG_OBJECT (bin, "got need-context message type: %s", context_type);
+      for (l = contexts; l; l = l->next) {
         GstContext *tmp = l->data;
         const gchar *tmp_type = gst_context_get_context_type (tmp);
 
@@ -4144,7 +4205,7 @@ gst_bin_set_context (GstElement * element, GstContext * context)
 
   bin = GST_BIN (element);
 
-  gst_bin_update_context (bin, context);
+  GST_ELEMENT_CLASS (parent_class)->set_context (element, context);
 
   children = gst_bin_iterate_elements (bin);
   while (gst_iterator_foreach (children, set_context,
index ccb3734..2c44a39 100644 (file)
@@ -133,6 +133,8 @@ static gboolean gst_element_set_clock_func (GstElement * element,
 static void gst_element_set_bus_func (GstElement * element, GstBus * bus);
 static gboolean gst_element_post_message_default (GstElement * element,
     GstMessage * message);
+static void gst_element_set_context_default (GstElement * element,
+    GstContext * context);
 
 static gboolean gst_element_default_send_event (GstElement * element,
     GstEvent * event);
@@ -239,6 +241,7 @@ gst_element_class_init (GstElementClass * klass)
   klass->send_event = GST_DEBUG_FUNCPTR (gst_element_default_send_event);
   klass->numpadtemplates = 0;
   klass->post_message = GST_DEBUG_FUNCPTR (gst_element_post_message_default);
+  klass->set_context = GST_DEBUG_FUNCPTR (gst_element_set_context_default);
 
   klass->elementfactory = NULL;
 }
@@ -2816,13 +2819,34 @@ gst_element_change_state_func (GstElement * element, GstStateChange transition)
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
       break;
     case GST_STATE_CHANGE_PAUSED_TO_READY:
-    case GST_STATE_CHANGE_READY_TO_NULL:
+    case GST_STATE_CHANGE_READY_TO_NULL:{
+      GList *l;
+
       /* deactivate pads in both cases, since they are activated on
          ready->paused but the element might not have made it to paused */
       if (!gst_element_pads_activate (element, FALSE)) {
         result = GST_STATE_CHANGE_FAILURE;
       }
+
+      /* Remove all non-persistent contexts */
+      GST_OBJECT_LOCK (element);
+      for (l = element->contexts; l;) {
+        GstContext *context = l->data;
+
+        if (!gst_context_is_persistent (context)) {
+          GList *next;
+
+          gst_context_unref (context);
+          next = l->next;
+          element->contexts = g_list_delete_link (element->contexts, l);
+          l = next;
+        } else {
+          l = l->next;
+        }
+      }
+      GST_OBJECT_UNLOCK (element);
       break;
+    }
     default:
       /* this will catch real but unhandled state changes;
        * can only be caused by:
@@ -2919,6 +2943,7 @@ gst_element_dispose (GObject * object)
   bus_p = &element->bus;
   gst_object_replace ((GstObject **) clock_p, NULL);
   gst_object_replace ((GstObject **) bus_p, NULL);
+  g_list_free_full (element->contexts, (GDestroyNotify) gst_context_unref);
   GST_OBJECT_UNLOCK (element);
 
   GST_CAT_INFO_OBJECT (GST_CAT_REFCOUNTING, element, "parent class dispose");
@@ -3029,6 +3054,35 @@ gst_element_get_bus (GstElement * element)
   return result;
 }
 
+static void
+gst_element_set_context_default (GstElement * element, GstContext * context)
+{
+  const gchar *context_type;
+  GList *l;
+
+  GST_OBJECT_LOCK (element);
+  context_type = gst_context_get_context_type (context);
+  for (l = element->contexts; l; l = l->next) {
+    GstContext *tmp = l->data;
+    const gchar *tmp_type = gst_context_get_context_type (tmp);
+
+    /* Always store newest context but never replace
+     * a persistent one by a non-persistent one */
+    if (strcmp (context_type, tmp_type) == 0 &&
+        (gst_context_is_persistent (context) ||
+            !gst_context_is_persistent (tmp))) {
+      gst_context_replace ((GstContext **) & l->data, context);
+      break;
+    }
+  }
+  /* Not found? Add */
+  if (l == NULL) {
+    element->contexts =
+        g_list_prepend (element->contexts, gst_context_ref (context));
+  }
+  GST_OBJECT_UNLOCK (element);
+}
+
 /**
  * gst_element_set_context:
  * @element: a #GstElement to set the context of.
@@ -3054,3 +3108,95 @@ gst_element_set_context (GstElement * element, GstContext * context)
   if (oclass->set_context)
     oclass->set_context (element, context);
 }
+
+/**
+ * gst_element_get_contexts:
+ * @element: a #GstElement to set the context of.
+ *
+ * Gets the contexts set on the element.
+ *
+ * MT safe.
+ *
+ * Returns: (element-type Gst.Context) (transfer full): List of #GstContext
+ *
+ * Since: 1.8
+ */
+GList *
+gst_element_get_contexts (GstElement * element)
+{
+  GList *ret;
+
+  g_return_val_if_fail (GST_IS_ELEMENT (element), NULL);
+
+  GST_OBJECT_LOCK (element);
+  ret = g_list_copy_deep (element->contexts, (GCopyFunc) gst_context_ref, NULL);
+  GST_OBJECT_UNLOCK (element);
+
+  return ret;
+}
+
+static gint
+_match_context_type (GstContext * c1, const gchar * context_type)
+{
+  const gchar *c1_type;
+
+  c1_type = gst_context_get_context_type (c1);
+
+  return g_strcmp0 (c1_type, context_type);
+}
+
+/**
+ * gst_element_get_context_unlocked:
+ * @element: a #GstElement to get the context of.
+ * @context_type: a name of a context to retrieve
+ *
+ * Gets the context with @context_type set on the element or NULL.
+ *
+ * Returns: (transfer full): A #GstContext or NULL
+ *
+ * Since: 1.8
+ */
+GstContext *
+gst_element_get_context_unlocked (GstElement * element,
+    const gchar * context_type)
+{
+  GstContext *ret = NULL;
+  GList *node;
+
+  g_return_val_if_fail (GST_IS_ELEMENT (element), NULL);
+
+  node =
+      g_list_find_custom (element->contexts, context_type,
+      (GCompareFunc) _match_context_type);
+  if (node && node->data)
+    ret = gst_context_ref (node->data);
+
+  return ret;
+}
+
+/**
+ * gst_element_get_context:
+ * @element: a #GstElement to get the context of.
+ * @context_type: a name of a context to retrieve
+ *
+ * Gets the context with @context_type set on the element or NULL.
+ *
+ * MT safe.
+ *
+ * Returns: (transfer full): A #GstContext or NULL
+ *
+ * Since: 1.8
+ */
+GstContext *
+gst_element_get_context (GstElement * element, const gchar * context_type)
+{
+  GstContext *ret = NULL;
+
+  g_return_val_if_fail (GST_IS_ELEMENT (element), NULL);
+
+  GST_OBJECT_LOCK (element);
+  ret = gst_element_get_context_unlocked (element, context_type);
+  GST_OBJECT_UNLOCK (element);
+
+  return ret;
+}
index 5f45988..cb4f8b8 100644 (file)
@@ -568,8 +568,11 @@ struct _GstElement
   GList                *sinkpads;
   guint32               pads_cookie;
 
+  /* with object LOCK */
+  GList                *contexts;
+
   /*< private >*/
-  gpointer _gst_reserved[GST_PADDING];
+  gpointer _gst_reserved[GST_PADDING-1];
 };
 
 /**
@@ -745,6 +748,9 @@ GstBus *                gst_element_get_bus             (GstElement * element);
 
 /* context */
 void                    gst_element_set_context         (GstElement * element, GstContext * context);
+GList *                 gst_element_get_contexts        (GstElement * element);
+GstContext *            gst_element_get_context         (GstElement * element, const gchar * context_type);
+GstContext *            gst_element_get_context_unlocked (GstElement * element, const gchar * context_type);
 
 /* pad management */
 gboolean                gst_element_add_pad             (GstElement *element, GstPad *pad);
index 5ba0cd2..63d8b79 100644 (file)
@@ -75,6 +75,9 @@ gst_context_element_set_context (GstElement * element, GstContext * context)
 {
   if (strcmp (gst_context_get_context_type (context), "foobar") == 0)
     ((GstContextElement *) element)->have_foobar = TRUE;
+
+  GST_ELEMENT_CLASS (gst_context_element_parent_class)->set_context (element,
+      context);
 }
 
 static GstStateChangeReturn
@@ -91,9 +94,7 @@ gst_context_element_change_state (GstElement * element,
     if (celement->set_before_ready && !have_foobar)
       return GST_STATE_CHANGE_FAILURE;
     else if (celement->set_before_ready)
-      return
-          GST_ELEMENT_CLASS (gst_context_element_parent_class)->change_state
-          (element, transition);
+      goto chain_up;
 
     if (celement->set_from_need_context && have_foobar)
       return GST_STATE_CHANGE_FAILURE;
@@ -109,9 +110,7 @@ gst_context_element_change_state (GstElement * element,
     if (celement->set_from_need_context && !have_foobar)
       return GST_STATE_CHANGE_FAILURE;
     else if (celement->set_from_need_context)
-      return
-          GST_ELEMENT_CLASS (gst_context_element_parent_class)->change_state
-          (element, transition);
+      goto chain_up;
 
     if (celement->create_self && have_foobar)
       return GST_STATE_CHANGE_FAILURE;
@@ -125,11 +124,9 @@ gst_context_element_change_state (GstElement * element,
       gst_element_post_message (element, msg);
       gst_context_unref (context);
     }
-    return
-        GST_ELEMENT_CLASS (gst_context_element_parent_class)->change_state
-        (element, transition);
   }
 
+chain_up:
   return
       GST_ELEMENT_CLASS (gst_context_element_parent_class)->change_state
       (element, transition);
@@ -321,6 +318,117 @@ GST_START_TEST (test_element_bin_caching)
 
 GST_END_TEST;
 
+GST_START_TEST (test_add_element_to_bin)
+{
+  GstBus *bus;
+  GstElement *bin;
+  GstElement *element;
+  GList *contexts, *contexts2, *l;
+
+  /* Start with an element not inside a bin requesting a context. Add the
+   * element to a bin and check the context propagation. */
+  element = g_object_new (gst_context_element_get_type (), NULL);
+
+  ((GstContextElement *) element)->create_self = TRUE;
+
+  fail_unless (gst_element_set_state (element,
+          GST_STATE_READY) == GST_STATE_CHANGE_SUCCESS);
+
+  fail_unless (((GstContextElement *) element)->have_foobar);
+
+  bin = gst_bin_new (NULL);
+
+  bus = gst_bus_new ();
+  gst_element_set_bus (bin, bus);
+
+  fail_unless (gst_element_set_state (bin,
+          GST_STATE_READY) == GST_STATE_CHANGE_SUCCESS);
+
+  gst_bin_add (GST_BIN (bin), element);
+
+  /* check the contexts are the same */
+  contexts = gst_element_get_contexts (element);
+  contexts2 = gst_element_get_contexts (bin);
+  for (l = contexts; l; l = l->next)
+    fail_unless (g_list_find (contexts2, l->data));
+  g_list_free_full (contexts, (GDestroyNotify) gst_context_unref);
+  g_list_free_full (contexts2, (GDestroyNotify) gst_context_unref);
+
+  gst_element_set_bus (bin, NULL);
+  fail_unless (gst_element_set_state (bin,
+          GST_STATE_NULL) == GST_STATE_CHANGE_SUCCESS);
+
+  gst_object_unref (bus);
+  gst_object_unref (bin);
+}
+
+GST_END_TEST;
+
+GST_START_TEST (test_add_element_to_bin_collision)
+{
+  GstBus *bus;
+  GstElement *bin;
+  GstElement *element, *element2;
+  GList *contexts, *contexts2, *l;
+
+  /* Start with a bin containing an element that requests a context and then add
+   * another element to the bin that has already requested the same context. */
+
+  bin = gst_bin_new (NULL);
+  element = g_object_new (gst_context_element_get_type (), NULL);
+  gst_bin_add (GST_BIN (bin), element);
+
+  ((GstContextElement *) element)->create_self = TRUE;
+
+  bus = gst_bus_new ();
+  gst_element_set_bus (bin, bus);
+
+  fail_unless (gst_element_set_state (bin,
+          GST_STATE_READY) == GST_STATE_CHANGE_SUCCESS);
+
+  fail_unless (((GstContextElement *) element)->have_foobar);
+
+  /* propagate a context without a parent bin */
+  element2 = g_object_new (gst_context_element_get_type (), NULL);
+  ((GstContextElement *) element2)->create_self = TRUE;
+
+  fail_unless (gst_element_set_state (element2,
+          GST_STATE_READY) == GST_STATE_CHANGE_SUCCESS);
+
+  fail_unless (((GstContextElement *) element2)->have_foobar);
+
+  ((GstContextElement *) element)->have_foobar = FALSE;
+  ((GstContextElement *) element2)->have_foobar = FALSE;
+
+  /* add element to bin should result in the propagation of contexts to the
+   * added element */
+  gst_bin_add (GST_BIN (bin), element2);
+
+  fail_unless (((GstContextElement *) element)->have_foobar == FALSE);
+  fail_unless (((GstContextElement *) element2)->have_foobar);
+
+  /* check the contexts are the same */
+  contexts = gst_element_get_contexts (element);
+  contexts2 = gst_element_get_contexts (element2);
+  for (l = contexts; l; l = l->next)
+    fail_unless (g_list_find (contexts2, l->data));
+  g_list_free_full (contexts, (GDestroyNotify) gst_context_unref);
+  contexts = gst_element_get_contexts (bin);
+  for (l = contexts; l; l = l->next)
+    fail_unless (g_list_find (contexts2, l->data));
+  g_list_free_full (contexts, (GDestroyNotify) gst_context_unref);
+  g_list_free_full (contexts2, (GDestroyNotify) gst_context_unref);
+
+  gst_element_set_bus (bin, NULL);
+  fail_unless (gst_element_set_state (bin,
+          GST_STATE_NULL) == GST_STATE_CHANGE_SUCCESS);
+
+  gst_object_unref (bus);
+  gst_object_unref (bin);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_context_suite (void)
 {
@@ -335,6 +443,8 @@ gst_context_suite (void)
   tcase_add_test (tc_chain, test_element_set_from_need_context);
   tcase_add_test (tc_chain, test_element_create_self);
   tcase_add_test (tc_chain, test_element_bin_caching);
+  tcase_add_test (tc_chain, test_add_element_to_bin);
+  tcase_add_test (tc_chain, test_add_element_to_bin_collision);
 
   return s;
 }
index e3a9347..fc29344 100644 (file)
@@ -511,6 +511,9 @@ EXPORTS
        gst_element_get_clock
        gst_element_get_compatible_pad
        gst_element_get_compatible_pad_template
+       gst_element_get_context
+       gst_element_get_context_unlocked
+       gst_element_get_contexts
        gst_element_get_factory
        gst_element_get_request_pad
        gst_element_get_start_time