From d5ded1588920c4471eefe055d09095d9e5e989b5 Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Thu, 24 Sep 2015 00:04:48 +1000 Subject: [PATCH] bin: implement context propagation when adding elements 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 | 3 + gst/gstbin.c | 135 +++++++++++++++++++++-------- gst/gstelement.c | 148 +++++++++++++++++++++++++++++++- gst/gstelement.h | 8 +- tests/check/gst/gstcontext.c | 128 +++++++++++++++++++++++++-- win32/common/libgstreamer.def | 3 + 6 files changed, 377 insertions(+), 48 deletions(-) diff --git a/docs/gst/gstreamer-sections.txt b/docs/gst/gstreamer-sections.txt index e2aaf9d468..5f5e5777f5 100644 --- a/docs/gst/gstreamer-sections.txt +++ b/docs/gst/gstreamer-sections.txt @@ -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 diff --git a/gst/gstbin.c b/gst/gstbin.c index 9cea0b30e3..9190bfe3dd 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -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; - const gchar *context_type; - 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; + + 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, diff --git a/gst/gstelement.c b/gst/gstelement.c index ccb3734088..2c44a39b45 100644 --- a/gst/gstelement.c +++ b/gst/gstelement.c @@ -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; +} diff --git a/gst/gstelement.h b/gst/gstelement.h index 5f45988f63..cb4f8b8163 100644 --- a/gst/gstelement.h +++ b/gst/gstelement.h @@ -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); diff --git a/tests/check/gst/gstcontext.c b/tests/check/gst/gstcontext.c index 5ba0cd2a3f..63d8b79988 100644 --- a/tests/check/gst/gstcontext.c +++ b/tests/check/gst/gstcontext.c @@ -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; } diff --git a/win32/common/libgstreamer.def b/win32/common/libgstreamer.def index e3a93475ab..fc29344e24 100644 --- a/win32/common/libgstreamer.def +++ b/win32/common/libgstreamer.def @@ -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