Commit 4f9d5adb authored by Edward Hervey's avatar Edward Hervey 🤘
Browse files

gst/gstbin.c: (de)activate src pads before calling state_change on the childs.

Original commit message from CVS:
* gst/gstbin.c: (activate_pads),
(iterator_activate_fold_with_resync), (gst_bin_src_pads_activate),
(gst_bin_change_state_func):
(de)activate src pads before calling state_change on the childs.
This is to avoid the case where a src ghostpad is blocked (holding the
stream lock), which would block the deactivation of the ghostpad's
target pad.
* gst/gstghostpad.c: (gst_proxy_pad_do_query_type),
(gst_proxy_pad_do_event), (gst_proxy_pad_do_query),
(gst_proxy_pad_do_internal_link), (gst_proxy_pad_do_bufferalloc),
(gst_proxy_pad_do_chain), (gst_proxy_pad_do_getrange),
(gst_proxy_pad_do_checkgetrange), (gst_proxy_pad_do_getcaps),
(gst_proxy_pad_do_acceptcaps), (gst_proxy_pad_do_fixatecaps),
(gst_proxy_pad_do_setcaps), (gst_proxy_pad_set_target_unlocked),
(gst_proxy_pad_set_target), (gst_proxy_pad_get_internal),
(gst_proxy_pad_dispose), (gst_proxy_pad_init),
(gst_ghost_pad_parent_set), (gst_ghost_pad_parent_unset),
(gst_ghost_pad_class_init),
(gst_ghost_pad_internal_do_activate_push),
(gst_ghost_pad_internal_do_activate_pull),
(gst_ghost_pad_do_activate_push), (gst_ghost_pad_do_activate_pull),
(gst_ghost_pad_do_link), (gst_ghost_pad_do_unlink),
(gst_ghost_pad_dispose), (gst_ghost_pad_new_no_target),
(gst_ghost_pad_new), (gst_ghost_pad_set_target):
GhostPads now create their internal GstProxyPad at creation (and not
when they're linked, as it was being done previously).
The internal and target pads are linked straight away.
The data will also travel through the other pad in order to make
pad blocking and probes non-hackish (the probe/block now really happens
on the GhostPad and not on the target).
* gst/gstpad.c: (gst_pad_set_blocked_async),
(gst_pad_link_prepare), (gst_pad_push_event):
Remove previous ghostpad cruft.
* gst/gstutils.c: (gst_pad_add_data_probe),
(gst_pad_add_event_probe), (gst_pad_add_buffer_probe),
(gst_pad_remove_data_probe), (gst_pad_remove_event_probe),
(gst_pad_remove_buffer_probe):
Remove previous ghost pad cruft.
Added more detailed debug statements.
* tests/check/gst/gstghostpad.c: (GST_START_TEST):
Fix the testsuite for refcounting changes.
The comments about who has references were correct, but the refcount
being checked wasn't the same (!?!).
parent c687975a
2006-07-11 Edward Hervey <edward@fluendo.com>
* gst/gstbin.c: (activate_pads),
(iterator_activate_fold_with_resync), (gst_bin_src_pads_activate),
(gst_bin_change_state_func):
(de)activate src pads before calling state_change on the childs.
This is to avoid the case where a src ghostpad is blocked (holding the
stream lock), which would block the deactivation of the ghostpad's
target pad.
* gst/gstghostpad.c: (gst_proxy_pad_do_query_type),
(gst_proxy_pad_do_event), (gst_proxy_pad_do_query),
(gst_proxy_pad_do_internal_link), (gst_proxy_pad_do_bufferalloc),
(gst_proxy_pad_do_chain), (gst_proxy_pad_do_getrange),
(gst_proxy_pad_do_checkgetrange), (gst_proxy_pad_do_getcaps),
(gst_proxy_pad_do_acceptcaps), (gst_proxy_pad_do_fixatecaps),
(gst_proxy_pad_do_setcaps), (gst_proxy_pad_set_target_unlocked),
(gst_proxy_pad_set_target), (gst_proxy_pad_get_internal),
(gst_proxy_pad_dispose), (gst_proxy_pad_init),
(gst_ghost_pad_parent_set), (gst_ghost_pad_parent_unset),
(gst_ghost_pad_class_init),
(gst_ghost_pad_internal_do_activate_push),
(gst_ghost_pad_internal_do_activate_pull),
(gst_ghost_pad_do_activate_push), (gst_ghost_pad_do_activate_pull),
(gst_ghost_pad_do_link), (gst_ghost_pad_do_unlink),
(gst_ghost_pad_dispose), (gst_ghost_pad_new_no_target),
(gst_ghost_pad_new), (gst_ghost_pad_set_target):
GhostPads now create their internal GstProxyPad at creation (and not
when they're linked, as it was being done previously).
The internal and target pads are linked straight away.
The data will also travel through the other pad in order to make
pad blocking and probes non-hackish (the probe/block now really happens
on the GhostPad and not on the target).
* gst/gstpad.c: (gst_pad_set_blocked_async),
(gst_pad_link_prepare), (gst_pad_push_event):
Remove previous ghostpad cruft.
* gst/gstutils.c: (gst_pad_add_data_probe),
(gst_pad_add_event_probe), (gst_pad_add_buffer_probe),
(gst_pad_remove_data_probe), (gst_pad_remove_event_probe),
(gst_pad_remove_buffer_probe):
Remove previous ghost pad cruft.
Added more detailed debug statements.
* tests/check/gst/gstghostpad.c: (GST_START_TEST):
Fix the testsuite for refcounting changes.
The comments about who has references were correct, but the refcount
being checked wasn't the same (!?!).
2006-07-10 Stefan Kost <ensonic@users.sf.net>
* docs/gst/gstreamer-sections.txt:
......
......@@ -1762,6 +1762,87 @@ done:
return ret;
}
/* gst_iterator_fold functions for pads_activate
* Note how we don't stop the iterator when we fail an activation. This is
* probably a FIXME since when one pad activation fails, we don't want to
* continue our state change. */
static gboolean
activate_pads (GstPad * pad, GValue * ret, gboolean * active)
{
if (!gst_pad_set_active (pad, *active))
g_value_set_boolean (ret, FALSE);
else if (!*active)
gst_pad_set_caps (pad, NULL);
/* unref the object that was reffed for us by _fold */
gst_object_unref (pad);
return TRUE;
}
/* returns false on error or early cutout (will never happen because the fold
* function always returns TRUE, see FIXME above) of the fold, true if all
* pads in @iter were (de)activated successfully. */
static gboolean
iterator_activate_fold_with_resync (GstIterator * iter, gpointer user_data)
{
GstIteratorResult ires;
GValue ret = { 0 };
/* no need to unset this later, it's just a boolean */
g_value_init (&ret, G_TYPE_BOOLEAN);
g_value_set_boolean (&ret, TRUE);
while (1) {
ires = gst_iterator_fold (iter, (GstIteratorFoldFunction) activate_pads,
&ret, user_data);
switch (ires) {
case GST_ITERATOR_RESYNC:
/* need to reset the result again */
g_value_set_boolean (&ret, TRUE);
gst_iterator_resync (iter);
break;
case GST_ITERATOR_DONE:
/* all pads iterated, return collected value */
goto done;
default:
/* iterator returned _ERROR or premature end with _OK,
* mark an error and exit */
g_value_set_boolean (&ret, FALSE);
goto done;
}
}
done:
/* return collected value */
return g_value_get_boolean (&ret);
}
/* is called with STATE_LOCK
*/
static gboolean
gst_bin_src_pads_activate (GstBin * bin, gboolean active)
{
GstIterator *iter;
gboolean fold_ok;
GST_DEBUG_OBJECT (bin, "src_pads_activate with active %d", active);
iter = gst_element_iterate_src_pads ((GstElement *) bin);
fold_ok = iterator_activate_fold_with_resync (iter, &active);
gst_iterator_free (iter);
if (G_UNLIKELY (!fold_ok))
goto failed;
GST_DEBUG_OBJECT (bin, "pads_activate successful");
return TRUE;
/* ERRORS */
failed:
{
GST_DEBUG_OBJECT (bin, "source pads_activate failed");
return FALSE;
}
}
static GstStateChangeReturn
gst_bin_change_state_func (GstElement * element, GstStateChange transition)
{
......@@ -1791,6 +1872,9 @@ gst_bin_change_state_func (GstElement * element, GstStateChange transition)
GST_DEBUG_OBJECT (element, "clearing EOS elements");
bin_remove_messages (bin, NULL, GST_MESSAGE_EOS);
GST_OBJECT_UNLOCK (bin);
if (current == GST_STATE_READY)
if (!(gst_bin_src_pads_activate (bin, TRUE)))
goto activate_failure;
break;
case GST_STATE_READY:
/* Clear message list on next READY */
......@@ -1798,6 +1882,14 @@ gst_bin_change_state_func (GstElement * element, GstStateChange transition)
GST_DEBUG_OBJECT (element, "clearing all cached messages");
bin_remove_messages (bin, NULL, GST_MESSAGE_ANY);
GST_OBJECT_UNLOCK (bin);
if (current == GST_STATE_PAUSED)
if (!(gst_bin_src_pads_activate (bin, FALSE)))
goto activate_failure;
break;
case GST_STATE_NULL:
if (current == GST_STATE_READY)
if (!(gst_bin_src_pads_activate (bin, FALSE)))
goto activate_failure;
break;
default:
break;
......@@ -1897,6 +1989,11 @@ done:
gst_element_state_get_name (GST_STATE (element)), ret);
return ret;
activate_failure:
GST_CAT_WARNING_OBJECT (GST_CAT_STATES, element,
"failure (de)activating src pads");
return GST_STATE_CHANGE_FAILURE;
}
/*
......
This diff is collapsed.
......@@ -63,7 +63,6 @@
#include "gst_private.h"
#include "gstpad.h"
#include "gstghostpad.h"
#include "gstpadtemplate.h"
#include "gstenumtypes.h"
#include "gstmarshal.h"
......@@ -939,18 +938,10 @@ gboolean
gst_pad_set_blocked_async (GstPad * pad, gboolean blocked,
GstPadBlockCallback callback, gpointer user_data)
{
gboolean was_blocked, was_ghost = FALSE;
gboolean was_blocked = FALSE;
g_return_val_if_fail (GST_IS_PAD (pad), FALSE);
if (GST_IS_GHOST_PAD (pad)) {
pad = gst_ghost_pad_get_target (GST_GHOST_PAD (pad));
if (!pad) {
return FALSE;
}
was_ghost = TRUE;
}
GST_OBJECT_LOCK (pad);
was_blocked = GST_PAD_IS_BLOCKED (pad);
......@@ -990,10 +981,6 @@ gst_pad_set_blocked_async (GstPad * pad, gboolean blocked,
}
GST_OBJECT_UNLOCK (pad);
if (was_ghost) {
gst_object_unref (pad);
}
return TRUE;
had_right_state:
......@@ -1003,9 +990,6 @@ had_right_state:
was_blocked);
GST_OBJECT_UNLOCK (pad);
if (was_ghost) {
gst_object_unref (pad);
}
return FALSE;
}
}
......@@ -1727,8 +1711,9 @@ not_srcpad:
}
src_was_linked:
{
GST_CAT_INFO (GST_CAT_PADS, "src %s:%s was already linked",
GST_DEBUG_PAD_NAME (srcpad));
GST_CAT_INFO (GST_CAT_PADS, "src %s:%s was already linked to %s:%s",
GST_DEBUG_PAD_NAME (srcpad),
GST_DEBUG_PAD_NAME (GST_PAD_PEER (srcpad)));
/* we do not emit a warning in this case because unlinking cannot
* be made MT safe.*/
GST_OBJECT_UNLOCK (srcpad);
......@@ -1743,8 +1728,9 @@ not_sinkpad:
}
sink_was_linked:
{
GST_CAT_INFO (GST_CAT_PADS, "sink %s:%s was already linked",
GST_DEBUG_PAD_NAME (sinkpad));
GST_CAT_INFO (GST_CAT_PADS, "sink %s:%s was already linked to %s:%s",
GST_DEBUG_PAD_NAME (sinkpad),
GST_DEBUG_PAD_NAME (GST_PAD_PEER (sinkpad)));
/* we do not emit a warning in this case because unlinking cannot
* be made MT safe.*/
GST_OBJECT_UNLOCK (sinkpad);
......@@ -3870,6 +3856,7 @@ dropping:
}
not_linked:
{
GST_DEBUG_OBJECT (pad, "Dropping event because pad is not linked");
gst_event_unref (event);
GST_OBJECT_UNLOCK (pad);
return FALSE;
......
......@@ -2783,19 +2783,10 @@ gulong
gst_pad_add_data_probe (GstPad * pad, GCallback handler, gpointer data)
{
gulong sigid;
gboolean was_ghost = FALSE;
g_return_val_if_fail (GST_IS_PAD (pad), 0);
g_return_val_if_fail (handler != NULL, 0);
if (GST_IS_GHOST_PAD (pad)) {
pad = gst_ghost_pad_get_target (GST_GHOST_PAD (pad));
if (!pad) {
return 0;
}
was_ghost = TRUE;
}
GST_OBJECT_LOCK (pad);
sigid = g_signal_connect (pad, "have-data", handler, data);
GST_PAD_DO_EVENT_SIGNALS (pad)++;
......@@ -2805,10 +2796,6 @@ gst_pad_add_data_probe (GstPad * pad, GCallback handler, gpointer data)
GST_PAD_DO_BUFFER_SIGNALS (pad), GST_PAD_DO_EVENT_SIGNALS (pad));
GST_OBJECT_UNLOCK (pad);
if (was_ghost) {
gst_object_unref (pad);
}
return sigid;
}
......@@ -2827,19 +2814,10 @@ gulong
gst_pad_add_event_probe (GstPad * pad, GCallback handler, gpointer data)
{
gulong sigid;
gboolean was_ghost = FALSE;
g_return_val_if_fail (GST_IS_PAD (pad), 0);
g_return_val_if_fail (handler != NULL, 0);
if (GST_IS_GHOST_PAD (pad)) {
pad = gst_ghost_pad_get_target (GST_GHOST_PAD (pad));
if (!pad) {
return 0;
}
was_ghost = TRUE;
}
GST_OBJECT_LOCK (pad);
sigid = g_signal_connect (pad, "have-data::event", handler, data);
GST_PAD_DO_EVENT_SIGNALS (pad)++;
......@@ -2847,10 +2825,6 @@ gst_pad_add_event_probe (GstPad * pad, GCallback handler, gpointer data)
GST_DEBUG_PAD_NAME (pad), GST_PAD_DO_EVENT_SIGNALS (pad));
GST_OBJECT_UNLOCK (pad);
if (was_ghost) {
gst_object_unref (pad);
}
return sigid;
}
......@@ -2869,19 +2843,10 @@ gulong
gst_pad_add_buffer_probe (GstPad * pad, GCallback handler, gpointer data)
{
gulong sigid;
gboolean was_ghost = FALSE;
g_return_val_if_fail (GST_IS_PAD (pad), 0);
g_return_val_if_fail (handler != NULL, 0);
if (GST_IS_GHOST_PAD (pad)) {
pad = gst_ghost_pad_get_target (GST_GHOST_PAD (pad));
if (!pad) {
return 0;
}
was_ghost = TRUE;
}
GST_OBJECT_LOCK (pad);
sigid = g_signal_connect (pad, "have-data::buffer", handler, data);
GST_PAD_DO_BUFFER_SIGNALS (pad)++;
......@@ -2889,10 +2854,6 @@ gst_pad_add_buffer_probe (GstPad * pad, GCallback handler, gpointer data)
GST_DEBUG_PAD_NAME (pad), GST_PAD_DO_BUFFER_SIGNALS (pad));
GST_OBJECT_UNLOCK (pad);
if (was_ghost) {
gst_object_unref (pad);
}
return sigid;
}
......@@ -2906,19 +2867,9 @@ gst_pad_add_buffer_probe (GstPad * pad, GCallback handler, gpointer data)
void
gst_pad_remove_data_probe (GstPad * pad, guint handler_id)
{
gboolean was_ghost = FALSE;
g_return_if_fail (GST_IS_PAD (pad));
g_return_if_fail (handler_id > 0);
if (GST_IS_GHOST_PAD (pad)) {
pad = gst_ghost_pad_get_target (GST_GHOST_PAD (pad));
if (!pad) {
return;
}
was_ghost = TRUE;
}
GST_OBJECT_LOCK (pad);
g_signal_handler_disconnect (pad, handler_id);
GST_PAD_DO_BUFFER_SIGNALS (pad)--;
......@@ -2929,9 +2880,6 @@ gst_pad_remove_data_probe (GstPad * pad, guint handler_id)
GST_PAD_DO_BUFFER_SIGNALS (pad));
GST_OBJECT_UNLOCK (pad);
if (was_ghost) {
gst_object_unref (pad);
}
}
/**
......@@ -2944,29 +2892,15 @@ gst_pad_remove_data_probe (GstPad * pad, guint handler_id)
void
gst_pad_remove_event_probe (GstPad * pad, guint handler_id)
{
gboolean was_ghost = FALSE;
g_return_if_fail (GST_IS_PAD (pad));
g_return_if_fail (handler_id > 0);
if (GST_IS_GHOST_PAD (pad)) {
pad = gst_ghost_pad_get_target (GST_GHOST_PAD (pad));
if (!pad) {
return;
}
was_ghost = TRUE;
}
GST_OBJECT_LOCK (pad);
g_signal_handler_disconnect (pad, handler_id);
GST_PAD_DO_EVENT_SIGNALS (pad)--;
GST_DEBUG ("removed event probe from pad %s:%s, now %d event probes",
GST_DEBUG_PAD_NAME (pad), GST_PAD_DO_EVENT_SIGNALS (pad));
GST_OBJECT_UNLOCK (pad);
if (was_ghost) {
gst_object_unref (pad);
}
}
/**
......@@ -2979,19 +2913,9 @@ gst_pad_remove_event_probe (GstPad * pad, guint handler_id)
void
gst_pad_remove_buffer_probe (GstPad * pad, guint handler_id)
{
gboolean was_ghost = FALSE;
g_return_if_fail (GST_IS_PAD (pad));
g_return_if_fail (handler_id > 0);
if (GST_IS_GHOST_PAD (pad)) {
pad = gst_ghost_pad_get_target (GST_GHOST_PAD (pad));
if (!pad) {
return;
}
was_ghost = TRUE;
}
GST_OBJECT_LOCK (pad);
g_signal_handler_disconnect (pad, handler_id);
GST_PAD_DO_BUFFER_SIGNALS (pad)--;
......@@ -2999,9 +2923,6 @@ gst_pad_remove_buffer_probe (GstPad * pad, guint handler_id)
GST_DEBUG_PAD_NAME (pad), GST_PAD_DO_BUFFER_SIGNALS (pad));
GST_OBJECT_UNLOCK (pad);
if (was_ghost) {
gst_object_unref (pad);
}
}
/**
......
......@@ -306,27 +306,27 @@ GST_START_TEST (test_ghost_pads)
while (GST_OBJECT_REFCOUNT_VALUE (fsrc) > 2)
THREAD_SWITCH ();
ASSERT_OBJECT_REFCOUNT (fsrc, "fsrc", 2); /* gisrc */
ASSERT_OBJECT_REFCOUNT (fsrc, "fsrc", 1);
ASSERT_OBJECT_REFCOUNT (gsink, "gsink", 1);
ASSERT_OBJECT_REFCOUNT (gsrc, "gsink", 1);
ASSERT_OBJECT_REFCOUNT (fsink, "fsink", 2); /* gisink */
ASSERT_OBJECT_REFCOUNT (fsink, "fsink", 1);
ASSERT_OBJECT_REFCOUNT (gisrc, "gisrc", 1); /* gsink */
ASSERT_OBJECT_REFCOUNT (gisrc, "gisrc", 2); /* gsink */
ASSERT_OBJECT_REFCOUNT (isink, "isink", 2); /* gsink */
ASSERT_OBJECT_REFCOUNT (gisink, "gisink", 1); /* gsrc */
ASSERT_OBJECT_REFCOUNT (gisink, "gisink", 2); /* gsrc */
ASSERT_OBJECT_REFCOUNT (isrc, "isrc", 2); /* gsrc */
gst_object_unref (gsink);
ASSERT_OBJECT_REFCOUNT (isink, "isink", 1);
ASSERT_OBJECT_REFCOUNT (gisrc, "gisrc", 1);
ASSERT_OBJECT_REFCOUNT (fsrc, "fsrc", 2); /* gisrc */
ASSERT_OBJECT_REFCOUNT (fsrc, "fsrc", 1);
gst_object_unref (gisrc);
ASSERT_OBJECT_REFCOUNT (fsrc, "fsrc", 1);
gst_object_unref (gsrc);
ASSERT_OBJECT_REFCOUNT (isrc, "isrc", 1);
ASSERT_OBJECT_REFCOUNT (gisink, "gisink", 1);
ASSERT_OBJECT_REFCOUNT (fsink, "fsink", 2); /* gisrc */
ASSERT_OBJECT_REFCOUNT (fsink, "fsink", 1);
gst_object_unref (gisink);
ASSERT_OBJECT_REFCOUNT (fsink, "fsink", 1);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment