From e658379534eb4a90b654d90f1d0bdf86f37c6e31 Mon Sep 17 00:00:00 2001 From: Alex Ashley Date: Tue, 24 Sep 2013 10:29:06 +0100 Subject: [PATCH] Potential GstContext regression Since the refactoring of GstContext (commits qc9fa2771b508e9aaeecc700e66e958190476f, a7f5dc8b8af837f01782d1572379948ff62daab7, 690326f906dc82e41ea58b81cdb2e3e88b754, d367dc1b0d4ecb37f4d27267e03d7bf0c6c06a6, and 82d158aed3f2e8545e1e7d35085085ff58f18) I am no longer able to get a shared context for an element that is used twice in a pipeline. I used the documentation and eglglessink as my reference for implementing the GstContext logic. As the code was tied to a hardware decoder, I have ported the GstContext code to fakesink to show the problem. Using the old API a single ExampleMgr instance is created, but using the new API each element is creating its own instance. --- plugins/elements/gstfakesink.c | 232 +++++++++++++++++++++++++++++++++ plugins/elements/gstfakesink.h | 48 ++++--- 2 files changed, 264 insertions(+), 16 deletions(-) diff --git a/plugins/elements/gstfakesink.c b/plugins/elements/gstfakesink.c index 1b8c7c3447..3a8548ba6f 100644 --- a/plugins/elements/gstfakesink.c +++ b/plugins/elements/gstfakesink.c @@ -136,6 +136,10 @@ static guint gst_fake_sink_signals[LAST_SIGNAL] = { 0 }; static GParamSpec *pspec_last_message = NULL; +static GstExampleMgr *gst_example_mgr_singleton = NULL; +static void gst_fake_sink_set_context (GstElement * element, + GstContext * context); + static void gst_fake_sink_class_init (GstFakeSinkClass * klass) { @@ -226,6 +230,7 @@ gst_fake_sink_class_init (GstFakeSinkClass * klass) gstelement_class->change_state = GST_DEBUG_FUNCPTR (gst_fake_sink_change_state); + gstelement_class->set_context = GST_DEBUG_FUNCPTR (gst_fake_sink_set_context); gstbase_sink_class->event = GST_DEBUG_FUNCPTR (gst_fake_sink_event); gstbase_sink_class->preroll = GST_DEBUG_FUNCPTR (gst_fake_sink_preroll); @@ -242,6 +247,7 @@ gst_fake_sink_init (GstFakeSink * fakesink) fakesink->state_error = DEFAULT_STATE_ERROR; fakesink->signal_handoffs = DEFAULT_SIGNAL_HANDOFFS; fakesink->num_buffers = DEFAULT_NUM_BUFFERS; + fakesink->example_mgr = NULL; gst_base_sink_set_sync (GST_BASE_SINK (fakesink), DEFAULT_SYNC); } @@ -513,6 +519,7 @@ static gboolean gst_fake_sink_query (GstBaseSink * bsink, GstQuery * query) { gboolean ret; + GstFakeSink *fakesink = GST_FAKE_SINK_CAST (bsink); switch (GST_QUERY_TYPE (query)) { case GST_QUERY_SEEKING:{ @@ -524,6 +531,31 @@ gst_fake_sink_query (GstBaseSink * bsink, GstQuery * query) ret = TRUE; break; } + case GST_QUERY_CONTEXT:{ + const gchar *context_type = NULL; + gst_query_parse_context_type (query, &context_type); + GST_DEBUG_OBJECT (fakesink, "query context %s %u", context_type, + (guint) fakesink->example_mgr); + if (g_strcmp0 (context_type, GST_EXAMPLE_MGR_CONTEXT_TYPE) == 0 && + fakesink->example_mgr) { + GstContext *context, *old_context = NULL; + GST_DEBUG_OBJECT (fakesink, "GST_EXAMPLE_MGR query reply %u", + (guint) fakesink->example_mgr); + gst_query_parse_context (query, &old_context); + if (old_context) { + context = gst_context_copy (old_context); + g_assert (context != NULL); + gst_context_set_example_mgr (context, fakesink->example_mgr); + } else { + context = gst_example_mgr_new_context (fakesink->example_mgr); + } + gst_query_set_context (query, context); + gst_context_unref (context); + return TRUE; + } + ret = GST_BASE_SINK_CLASS (parent_class)->query (bsink, query); + break; + } default: ret = GST_BASE_SINK_CLASS (parent_class)->query (bsink, query); break; @@ -540,8 +572,79 @@ gst_fake_sink_change_state (GstElement * element, GstStateChange transition) switch (transition) { case GST_STATE_CHANGE_NULL_TO_READY: + GST_DEBUG_OBJECT (fakesink, "GST_STATE_CHANGE_NULL_TO_READY"); if (fakesink->state_error == FAKE_SINK_STATE_ERROR_NULL_READY) goto error; + GST_OBJECT_LOCK (fakesink); + if (!fakesink->example_mgr) { /* Query upstream for the context */ + GstPad *sinkPad = GST_BASE_SINK_PAD (fakesink); + GST_DEBUG_OBJECT (fakesink, "Need to find or create an ExampleMgr %u", + (guint) gst_example_mgr_singleton); + if (sinkPad) { + GstQuery *query = gst_example_mgr_query_context (); + GstContext *context = NULL; + GST_DEBUG_OBJECT (fakesink, "Query upstream for ExampleMgr"); + GST_OBJECT_UNLOCK (fakesink); + gst_pad_query (sinkPad, query); + GST_OBJECT_LOCK (fakesink); + gst_query_parse_context (query, &context); + if (context) { + GstExampleMgr *mgr = NULL; + GST_DEBUG_OBJECT (fakesink, "Query upstream got context"); + if (gst_context_get_example_mgr (context, &mgr)) { + GST_DEBUG_OBJECT (fakesink, + "Query upstream found ExampleMgr %u", (guint) mgr); + if (fakesink->example_mgr) { + gst_example_mgr_unref (fakesink->example_mgr); + } + fakesink->example_mgr = mgr; + } + } + gst_query_unref (query); + } + } + if (!fakesink->example_mgr) { + GstMessage *msg; + /* Query downstream for the context */ + /*Post a message in the bus to see if the application has one to share. */ + GST_DEBUG_OBJECT (fakesink, "Post message to look for ExampleMgr"); + msg = gst_message_new_need_context (GST_OBJECT_CAST (fakesink), + GST_EXAMPLE_MGR_CONTEXT_TYPE); + GST_OBJECT_UNLOCK (fakesink); + gst_element_post_message (GST_ELEMENT_CAST (fakesink), msg); + GST_OBJECT_LOCK (fakesink); + } + if (fakesink->example_mgr) { + GST_OBJECT_UNLOCK (fakesink); + } else { + fakesink->example_mgr = gst_example_mgr_new (NULL); + GST_OBJECT_UNLOCK (fakesink); + if (gst_example_mgr_singleton != fakesink->example_mgr) { + GST_ERROR_OBJECT (fakesink, + "More than one ExampleMgr has been allocated!"); + goto error; + } + if (!fakesink->example_mgr) { + GST_ERROR_OBJECT (fakesink, "Could not create GstExampleManager"); + goto error; + } else { + GstMessage *msg; + GstContext *context; + /* Create the context if there is none, post a message and + send an event letting everyone know that the element + has the context. */ + context = gst_example_mgr_new_context (fakesink->example_mgr); + GST_DEBUG_OBJECT (fakesink, + "telling the world about the new ExampleMgr %u", + (guint) fakesink->example_mgr); + gst_element_set_context (GST_ELEMENT_CAST (fakesink), context); + msg = gst_message_new_have_context (GST_OBJECT (fakesink), context); + gst_element_post_message (GST_ELEMENT_CAST (fakesink), msg); + context = gst_example_mgr_new_context (fakesink->example_mgr); + GST_DEBUG_OBJECT (fakesink, + "telling the world about the new ExampleMgr done"); + } + } break; case GST_STATE_CHANGE_READY_TO_PAUSED: if (fakesink->state_error == FAKE_SINK_STATE_ERROR_READY_PAUSED) @@ -573,6 +676,10 @@ gst_fake_sink_change_state (GstElement * element, GstStateChange transition) GST_OBJECT_LOCK (fakesink); g_free (fakesink->last_message); fakesink->last_message = NULL; + if (fakesink->example_mgr) { + gst_example_mgr_unref (fakesink->example_mgr); + fakesink->example_mgr = NULL; + } GST_OBJECT_UNLOCK (fakesink); break; default: @@ -587,3 +694,128 @@ error: ("Erroring out on state change as requested")); return GST_STATE_CHANGE_FAILURE; } + +static void +gst_fake_sink_set_context (GstElement * element, GstContext * context) +{ + GstExampleMgr *mgr = NULL; + GstFakeSink *fakesink = GST_FAKE_SINK (element); + + GST_DEBUG_OBJECT (fakesink, "set context"); + if (gst_context_get_example_mgr (context, &mgr)) { + GST_OBJECT_LOCK (fakesink); + GST_DEBUG_OBJECT (fakesink, "ExampleMgr %u", (guint) mgr); + if (fakesink->example_mgr) { + gst_example_mgr_unref (fakesink->example_mgr); + } + fakesink->example_mgr = mgr; + GST_OBJECT_UNLOCK (fakesink); + } + /* GstElement no longer keeps context + GST_OBJECT_LOCK (fakesink); + context = gst_context_copy (context); + gst_context_set_example_mgr (context, fakesink->example_mgr); + GST_OBJECT_UNLOCK (fakesink); + GST_ELEMENT_CLASS (gst_example_sink_parent_class)->set_context (element, context); + gst_context_unref (context); + */ +} + +struct _GstExampleMgr +{ + volatile gint refcount; + GDestroyNotify destroy_notify; +}; + +GstExampleMgr * +gst_example_mgr_new (GDestroyNotify destroy_notify) +{ + GstExampleMgr *mgr; + + mgr = g_slice_new (GstExampleMgr); + GST_DEBUG ("new ExampleMgr %u", (guint) mgr); + mgr->refcount = 1; + mgr->destroy_notify = destroy_notify; + if (gst_example_mgr_singleton == NULL) { + gst_example_mgr_singleton = mgr; + } + return mgr; +} + +GstExampleMgr * +gst_example_mgr_ref (GstExampleMgr * mgr) +{ + g_return_val_if_fail (mgr != NULL, NULL); + + g_atomic_int_inc (&mgr->refcount); + GST_DEBUG ("ref %u %d", (guint) mgr, mgr->refcount); + + return mgr; +} + +void +gst_example_mgr_unref (GstExampleMgr * mgr) +{ + g_return_if_fail (mgr != NULL); + + GST_DEBUG ("unref %u %d", (guint) mgr, mgr->refcount); + if (g_atomic_int_dec_and_test (&mgr->refcount)) { + GST_DEBUG ("refcount==0"); + if (mgr->destroy_notify) { + mgr->destroy_notify (mgr); + } + if (gst_example_mgr_singleton == mgr) { + gst_example_mgr_singleton = NULL; + } + g_slice_free (GstExampleMgr, mgr); + } +} + +#define GST_EXAMPLE_MGR_CONTEXT_NAME "ExampleMgr" + +gboolean +gst_context_get_example_mgr (GstContext * context, GstExampleMgr ** mgr) +{ + const GstStructure *s; + g_return_val_if_fail (GST_IS_CONTEXT (context), FALSE); + g_return_val_if_fail (strcmp (gst_context_get_context_type (context), + GST_EXAMPLE_MGR_CONTEXT_TYPE) == 0, FALSE); + + s = gst_context_get_structure (context); + return gst_structure_get (s, GST_EXAMPLE_MGR_CONTEXT_NAME, + GST_TYPE_EXAMPLE_MGR, mgr, NULL); +} + +GstContext * +gst_example_mgr_new_context (GstExampleMgr * mgr) +{ + GstContext *context; + context = gst_context_new (GST_EXAMPLE_MGR_CONTEXT_TYPE, FALSE); + if (context) { + gst_context_set_example_mgr (context, mgr); + } + return context; +} + +GstQuery * +gst_example_mgr_query_context (void) +{ + GstQuery *context = gst_query_new_context (GST_EXAMPLE_MGR_CONTEXT_TYPE); + return context; +} + +void +gst_context_set_example_mgr (GstContext * context, GstExampleMgr * mgr) +{ + GstStructure *s; + + if (context && mgr) { + s = gst_context_writable_structure (context); + gst_structure_set (s, GST_EXAMPLE_MGR_CONTEXT_NAME, + GST_TYPE_EXAMPLE_MGR, mgr, NULL); + } +} + +G_DEFINE_BOXED_TYPE (GstExampleMgr, gst_example_mgr, + (GBoxedCopyFunc) gst_example_mgr_ref, + (GBoxedFreeFunc) gst_example_mgr_unref); diff --git a/plugins/elements/gstfakesink.h b/plugins/elements/gstfakesink.h index 72b3671b6d..0b6e1bd0d6 100644 --- a/plugins/elements/gstfakesink.h +++ b/plugins/elements/gstfakesink.h @@ -28,8 +28,6 @@ #include G_BEGIN_DECLS - - #define GST_TYPE_FAKE_SINK \ (gst_fake_sink_get_type()) #define GST_FAKE_SINK(obj) \ @@ -41,7 +39,6 @@ G_BEGIN_DECLS #define GST_IS_FAKE_SINK_CLASS(klass) \ (G_TYPE_CHECK_CLASS_TYPE((klass),GST_TYPE_FAKE_SINK)) #define GST_FAKE_SINK_CAST(obj) ((GstFakeSink *)obj) - /** * GstFakeSinkStateError: * @FAKE_SINK_STATE_ERROR_NONE: no error @@ -54,7 +51,8 @@ G_BEGIN_DECLS * * Possible state change errors for the state-error property. */ -typedef enum { + typedef enum +{ FAKE_SINK_STATE_ERROR_NONE = 0, FAKE_SINK_STATE_ERROR_NULL_READY, FAKE_SINK_STATE_ERROR_READY_PAUSED, @@ -66,34 +64,52 @@ typedef enum { typedef struct _GstFakeSink GstFakeSink; typedef struct _GstFakeSinkClass GstFakeSinkClass; +typedef struct _GstExampleMgr GstExampleMgr; /** * GstFakeSink: * * The opaque #GstFakeSink data structure. */ -struct _GstFakeSink { - GstBaseSink element; +struct _GstFakeSink +{ + GstBaseSink element; - gboolean silent; - gboolean dump; - gboolean signal_handoffs; + gboolean silent; + gboolean dump; + gboolean signal_handoffs; GstFakeSinkStateError state_error; - gchar *last_message; - gint num_buffers; - gint num_buffers_left; + gchar *last_message; + gint num_buffers; + gint num_buffers_left; + GstExampleMgr *example_mgr; }; -struct _GstFakeSinkClass { +struct _GstFakeSinkClass +{ GstBaseSinkClass parent_class; /* signals */ - void (*handoff) (GstElement *element, GstBuffer *buf, GstPad *pad); - void (*preroll_handoff) (GstElement *element, GstBuffer *buf, GstPad *pad); + void (*handoff) (GstElement * element, GstBuffer * buf, GstPad * pad); + void (*preroll_handoff) (GstElement * element, GstBuffer * buf, GstPad * pad); }; G_GNUC_INTERNAL GType gst_fake_sink_get_type (void); -G_END_DECLS +#define GST_EXAMPLE_MGR_CONTEXT_TYPE "gst.example.ExampleMgr" +GstContext *gst_example_mgr_new_context (GstExampleMgr * mgr); +GstQuery *gst_example_mgr_query_context (void); +void gst_context_set_example_mgr (GstContext * context, GstExampleMgr * mgr); +gboolean gst_context_get_example_mgr (GstContext * context, + GstExampleMgr ** mgr); + +#define GST_TYPE_EXAMPLE_MGR (gst_example_mgr_get_type()) +GType gst_example_mgr_get_type (void); + +GstExampleMgr *gst_example_mgr_new (GDestroyNotify destroy_notify); +GstExampleMgr *gst_example_mgr_ref (GstExampleMgr * mgr); +void gst_example_mgr_unref (GstExampleMgr * mgr); + +G_END_DECLS #endif /* __GST_FAKE_SINK_H__ */