diff --git a/ChangeLog b/ChangeLog index 6744a7f577..3b2eb2c5be 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,63 @@ +2004-12-13 Wim Taymans + + * docs/design/part-MT-refcounting.txt: + * docs/design/part-conventions.txt: + * gst/gstbin.c: (gst_bin_set_index), (gst_bin_set_clock), + (gst_bin_add_func), (gst_bin_remove_func), + (gst_bin_iterate_elements), (gst_bin_change_state), + (gst_bin_dispose), (gst_bin_get_by_name_recurse_up): + * gst/gstcaps.c: + * gst/gstelement.c: (gst_element_add_pad), + (gst_element_remove_pad), (pad_compare_name), + (gst_element_get_static_pad), (gst_element_get_request_pad), + (gst_element_get_pad), (gst_element_iterate_pads), + (gst_element_class_get_pad_template_list), + (gst_element_class_get_pad_template), (gst_element_get_random_pad), + (gst_element_get_event_masks), (gst_element_send_event), + (gst_element_seek), (gst_element_get_query_types), + (gst_element_query), (gst_element_get_formats), + (gst_element_convert), (gst_element_post_message), + (gst_element_set_locked_state), (gst_element_get_state), + (gst_element_set_state), (gst_element_pads_activate), + (gst_element_dispose), (gst_element_set_manager_func), + (gst_element_get_manager): + * gst/gstelement.h: + * gst/gstiterator.c: (gst_iterator_new), (gst_list_iterator_next), + (gst_list_iterator_resync), (gst_list_iterator_free), + (gst_iterator_new_list): + * gst/gstiterator.h: + * gst/gstmessage.c: (_gst_message_copy): + * gst/gstobject.c: (gst_object_class_init), (gst_object_init), + (gst_object_ref), (gst_object_unref), (gst_object_sink), + (gst_object_replace), (gst_object_dispose), + (gst_object_dispatch_properties_changed), (gst_object_set_name), + (gst_object_set_parent), (gst_object_get_parent), + (gst_object_unparent), (gst_object_check_uniqueness), + (gst_object_get_path_string): + * gst/gstobject.h: + * gst/gstpad.c: (gst_pad_dispose), (gst_pad_set_active), + (gst_pad_is_active), (gst_pad_set_blocked_async), + (gst_pad_is_blocked), (gst_pad_unlink), (gst_pad_is_linked), + (gst_pad_link_prepare_filtered), (gst_pad_link_filtered), + (gst_pad_get_real_parent), (gst_pad_relink_filtered), + (gst_pad_get_peer), (gst_pad_realize), (gst_pad_get_allowed_caps), + (gst_pad_alloc_buffer), (gst_pad_push), (gst_pad_pull), + (gst_pad_pull_range), (gst_pad_push_event): + * gst/gstpad.h: + * gst/gstpipeline.c: (gst_pipeline_init), (gst_pipeline_dispose), + (is_eos), (pipeline_bus_handler): + * gst/gstutils.c: (gst_element_get_compatible_pad_filtered), + (gst_element_link_pads_filtered), (gst_element_unlink): + * gst/parse/grammar.y: + * tools/gst-compprep.c: (main): + * tools/gst-inspect.c: (print_pad_info): + * tools/gst-launch.c: (main): + * tools/gst-xmlinspect.c: (print_element_info): + More MT fixes, added design document describing refcounting + policies used in GStreamer and locking involved. + Fixed unsafe ghostpad dereffing. + Removed old unsafe methods. + 2004-12-10 Wim Taymans * docs/design/part-MT-refcounting.txt: diff --git a/docs/design/part-MT-refcounting.txt b/docs/design/part-MT-refcounting.txt index c188af062d..812b41963c 100644 --- a/docs/design/part-MT-refcounting.txt +++ b/docs/design/part-MT-refcounting.txt @@ -208,6 +208,9 @@ Objects GST_UNLOCK (pad); ... use peer ... + if (peer) + gst_object_unref (GST_OBJECT (peer)); + Note that after releasing the lock the peer might not actually be the peer anymore of the pad. If you need to be sure it is, you need to extend the critical section to include the operations on the peer. @@ -219,12 +222,12 @@ Objects public fields manually. All accessor methods that return an object should increase the refcount of the returned - object. The called should _unref the object after usage. Each method should state this + object. The called should _unref() the object after usage. Each method should state this refcounting policy in the documentation. * Accessing lists - If the object property is a list concurrent list iteration is needed to get the + If the object property is a list, concurrent list iteration is needed to get the contents of the list. GStreamer uses the cookie mechanism to mark the last update of a list. The list and the cookie are protected by the same lock. Each update to a list requires the following actions: @@ -286,7 +289,33 @@ Objects * GstIterator - GstIterator provides an easier way of retrieving elements in a concurrent list. See - the documentation of the GstIterator class. + GstIterator provides an easier way of retrieving elements in a concurrent list. + The followgin code example is equivalent to the previous example. + Example: + + it = _get_iterator(object); + while (!done) { + switch (gst_iterator_next (it, &item)) { + case GST_ITERATOR_OK: + ... use/change item here... + + /* release item here */ + gst_object_unref (item); + break; + case GST_ITERATOR_RESYNC: + /* handle rollback caused by concurrent modification + * of the list here */ + + ...rollback changes to items... + + /* resync iterator to start again */ + gst_iterator_resync (it); + break; + case GST_ITERATOR_DONE: + done = TRUE; + break; + } + } + gst_iterator_free (it); diff --git a/docs/design/part-conventions.txt b/docs/design/part-conventions.txt index 60bb478a6c..60a6ce5354 100644 --- a/docs/design/part-conventions.txt +++ b/docs/design/part-conventions.txt @@ -31,3 +31,53 @@ element flags should be cross-checked with the header, as there are currently tw _FLAGS_ in the middle. FIXME: check flags for consistency. + +Drawing conventions +=================== + +When drawing pictures the folowing conventions apply: + +objects +------- + +Objects are drawn with a box like + +------+ + | | + +------+ + + +pointers +-------- + +a pointer to an object. + +-----+ + *--->| | + +-----+ + +an invalid pointer, this is a pointer that should not be used. + + *-//-> + + +elements +-------- + + +----------+ + | name | + sink src + +----------+ + +pad links +--------- + + -----+ +--- + | | + src--sink + -----+ +--- + + + + + + + diff --git a/docs/design/part-relations.txt b/docs/design/part-relations.txt new file mode 100644 index 0000000000..f2deabbff4 --- /dev/null +++ b/docs/design/part-relations.txt @@ -0,0 +1,487 @@ +Object relation types +--------------------- + +1) parent-child relation + + +---------+ +-------+ + | parent | | child | + *--->| *----->| | + | F1|<-----* 1| + +---------+ +-------+ + + - properties + + - parent has references to multiple children + - child has reference to parent + - reference fields protected with LOCK + - the reference held by each child to the parent is + NOT reflected in the refcount of the parent. + - the parent removes the floating flag of the child when taking + ownership. + - the application has valid reference to parent + - creation/destruction requires two unnested locks and 1 refcount. + + - usage in GStreamer + + GstBin -> GstElement + GstElement -> GstRealPad + + - lifecycle + + a) object creation + + The application creates two object and holds a pointer + to them. The objects are initially FLOATING with a refcount + of 1. + + +---------+ +-------+ + *--->| parent | *--->| child | + | * | | | + | F1| | * F1| + +---------+ +-------+ + + b) establishing the parent-child relationship + + The application then calls a method on the parent object to take + ownership of the child object. The parent performs the following + actions: + + result = _set_parent (child, parent); + if (result) { + LOCK (parent); + ref_pointer = child; + + .. update other data structures .. + UNLOCK (parent); + } + else { + .. child had parent .. + } + + The _set_parent() method performs the following actions: + + LOCK (child); + if (child->parent != NULL) { + UNLOCK (child); + return FALSE; + } + if (IS_FLOATING (child)) { + UNSET (child, FLOATING); + } + else { + _ref (child); + } + child->parent = parent; + UNLOCK (child); + _signal (PARENT_SET, child, parent); + return TRUE; + + The function atomically checks if the child has no parent yet + and will set the parent if not. It will also sink the child, meaning + all floating references to the child are invalid now as it takes + over the refcount of the object. + + Visually: + + after _set_parent() returns TRUE: + + +---------+ +-------+ + *---->| parent | *-//->| child | + | * | | | + | F1|<-------------* 1| + +---------+ +-------+ + + after parent updates ref_pointer to child. + + +---------+ +-------+ + *---->| parent | *-//->| child | + | *--------->| | + | F1|<---------* 1| + +---------+ +-------+ + + - only one parent is able to _sink the same object because the + _set_parent() method is atomic. + - since only one parent is able to _set_parent() the object, only + one will add a reference to the object. + - since the parent can hold multiple references to children, we don't + need to lock the parent when locking the child. Many threads can + call _set_parent() on the children with the same parent, the parent + can then add all those to its lists. + + Note: that the signal is emited before the parent has added the + element to its internal data structures. This is not a problem + since the parent usually has his own signal to inform the app that + the child was reffed. One possible solution would be to update the + internal structure first and then perform a rollback if the _set_parent() + failed. This is not a good solution as iterators might grab the + 'half-added' child too soon. + + c) using the parent-child relationship + + - since the initial floating reference to the child object became + invalid after giving it to the parent, any reference to a child + has at least a refcount > 1. + + - this means that unreffing a child object cannot decrease the refcount + to 0. In fact, only the parent can destroy and dispose the child + object. + + - given a reference to the child object, the parent pointer is only + valid when holding the child LOCK. Indeed, after unlocking the child + LOCK, the parent can unparent the child or the parent could even become + disposed. To avoid the parent dispose problem, when obtaining the + parent pointer, if should be reffed before releasing the child LOCK. + + I) getting a reference to the parent. + + - a referece is held to the child, so it cannot be disposed. + + LOCK (child); + parent = _ref (child->parent); + UNLOCK (child); + + .. use parent .. + + _unref (parent); + + II) getting a reference to a child + + - a reference to a child can be obtained by reffing it before + adding it to the parent or by querying the parent. + + - when requesting a child from the parent, a reference is held to + the parent so it cannot be disposed. The parent will use its + internal data structures to locate the child element and will + return a reference to it with an incremented refcount. The + requester should _unref() the child after usage. + + + d) destroying the parent-child relationship + + - only the parent can actively destroy the parent-child relationship + this typically happens when a method is called on the parent to release + ownership of the child. + + - a child shall never remove itself from the parent. + + - since calling a method on the parent with the child as an argument + requires the caller to obtain a valid reference to the child, the child + refcount is at least > 1. + + - the parent will perform the folowing actions: + + LOCK (parent); + if (ref_pointer == child) { + ref_pointer = NULL; + + .. update other data structures .. + UNLOCK (parent); + + _unparent (child); + } + else { + UNLOCK (parent); + .. not our child .. + } + + The _unparent() method performs the following actions: + + LOCK (child); + if (child->parent != NULL) { + child->parent = NULL; + UNLOCK (child); + _signal (PARENT_UNSET, child, parent); + + _unref (child); + } + else { + UNLOCK (child); + } + + Since the _unparent() method unrefs the child object, it is possible that + the child pointer is invalid after this function. If the parent wants to + perform other actions on the child (such as signal emmision) it should + _ref() the child first. + + +2) single-reffed relation + + +---------+ +---------+ + *--->| object1 | *--->| object2 | + | *--------->| | + | 1| | 2| + +---------+ +---------+ + + - properties + + - one object has a reference to another + - reference field protected with LOCK + - the reference held by the object is reflected in the + refcount of the other object. + - typically the other object can be shared among multiple + other objects where each ref is counted for in the + refcount. + - no object has ownership of the other. + - either shared state or copy-on-write. + - creation/destruction requires one lock and one refcount. + + - usage + + GstRealPad -> GstCaps + GstBuffer -> GstCaps + GstEvent -> GstCaps + GstEvent -> GstObject + GstMessage -> GstCaps + GstMessage -> GstObject + + - lifecycle + + a) Two objects exist unlinked. + + +---------+ +---------+ + *--->| object1 | *--->| object2 | + | * | | | + | 1| | 1| + +---------+ +---------+ + + b) establishing the single-reffed relationship + + The second object is attached to the first one using a method + on the first object. The second object is reffed and a pointer + is updated in the first object using the following algorithm: + + LOCK (object1); + if (object1->pointer) + _unref (object1->pointer); + object1->pointer = _ref (object2); + UNLOCK (object1); + + After releasing the lock on the first object is is not sure that + object2 is still reffed from object1. + + +---------+ +---------+ + *--->| object1 | *--->| object2 | + | *--------->| | + | 1| | 2| + +---------+ +---------+ + + c) using the single-reffed relationship + + The only way to access object2 is by holding a ref to it or by + getting the reference from object1. + Reading the object pointed to by object1 can be done like this: + + LOCK (object1); + object2 = object1->pointer; + _ref (object2); + UNLOCK (object1); + + .. use object2 ... + _unref (object2); + + Depending on the type of the object, modifications can be done either + with copy-on-write or directly into the object. + + Copy on write can practically only be done like this: + + LOCK (object1); + object2 = object1->pointer; + object2 = _copy_on_write (object2); + ... make modifications to object2 ... + UNLOCK (object1); + + Releasing the lock has only a very small window where the copy_on_write + actually does not perform a copy: + + LOCK (object1); + object2 = object1->pointer; + _ref (object2); + UNLOCK (object1); + + .. object2 now has at least 2 refcounts making the next + copy-on-write make a real copy, unless some other thread + writes another object2 to object1 here ... + + object2 = _copy_on_write (object2); + + .. make modifications to object2 ... + + LOCK (object1); + if (object1->pointer != object2) { + if (object1->pointer) + _unref (object1->pointer); + object1->pointer = gst_object_ref (object2); + } + UNLOCK (object1); + + d) destroying the single-reffed relationship + + The folowing algorithm removes the single-reffed link between + object1 and object2. + + LOCK (object1); + _unref (object1->pointer); + object1->pointer = NULL; + UNLOCK (object1); + + Which yields the following initial state again: + + +---------+ +---------+ + *--->| object1 | *--->| object2 | + | * | | | + | 1| | 1| + +---------+ +---------+ + + +3) unreffed relation + + +---------+ +---------+ + *--->| object1 | *--->| object2 | + | *--------->| | + | 1|<---------* 1| + +---------+ +---------+ + + - properties + + - two objects have references to eachother + - both objects can only have 1 reference to another object. + - reference fields protected with LOCK + - the references held by each object are NOT reflected in the + refcount of the other object. + - no object has ownership of the other. + - typically each object is owned by a different parent. + - creation/destruction requires two nested locks and no refcounts. + + - usage + + - This type of link is used when the link is less important than + the existance of the objects, If one of the objects is disposed, so + is the link. + + GstRealPad <-> GstRealPad (srcpad lock taken first) + + - lifecycle + + a) Two objects exist unlinked. + + +---------+ +---------+ + *--->| object1 | *--->| object2 | + | * | | | + | 1| | * 1| + +---------+ +---------+ + + b) establishing the unreffed relationship + + Since we need to take two locks, the order in which these locks are + taken is very important or we might cause deadlocks. This lock order + must be defined for all unreffed relations. In these examples we always + lock object1 first and then object2. + + LOCK (object1); + LOCK (object2); + object2->refpointer = object1; + object1->refpointer = object2; + UNLOCK (object2); + UNLOCK (object1); + + c) using the unreffed relationship + + Reading requires taking one of the locks and reading the corresponing + object. Again we need to ref the object before releasing the lock. + + LOCK (object1); + object2 = _ref (object1->refpointer); + UNLOCK (object1); + + .. use object2 .. + _unref (object2); + + d) destroying the unreffed relationship + + Because of the lock order we need to be careful when destroying this + Relation. + + When only a reference to object1 is held: + + LOCK (object1); + LOCK (object2); + object1->refpointer->refpointer = NULL; + object1->refpointer = NULL; + UNLOCK (object2); + UNLOCK (object1); + + When only a reference to object2 is held we need to get a handle to the + other object fist so that we can lock it first. There is a window where + we need to release all locks and the relation could be invalid. To solve + this we check the relation after grabbing both locks and retry if the + relation changed. + + retry: + LOCK (object2); + object1 = _ref (object2->refpointer); + UNLOCK (object2); + .. things can change here .. + LOCK (object1); + LOCK (object2); + if (object1 == object2->refpointer) { + /* relation unchanged */ + object1->refpointer->refpointer = NULL; + object1->refpointer = NULL; + } + else { + /* relation changed.. retry */ + UNLOCK (object2); + UNLOCK (object1); + _unref (object1); + goto retry; + } + UNLOCK (object2); + UNLOCK (object1); + _unref (object1); + + When references are held to both objects. Note that it is not possible to + get references to both objects with the locks released since when the + references are taken and the locks are released, a concurrent update might + have changed the link, making the references not point to linked objects. + + LOCK (object1); + LOCK (object2); + if (object1->refpointer == object2) { + object2->refpointer = NULL; + object1->refpointer = NULL; + } + else { + .. objects are not linked .. + } + UNLOCK (object2); + UNLOCK (object1); + + +4) double-reffed relation + + +---------+ +---------+ + *--->| object1 | *--->| object2 | + | *--------->| | + | 2|<---------* 2| + +---------+ +---------+ + + - properties + + - two objects have references to eachother + - reference fields protected with LOCK + - the references held by each object are reflected in the + refcount of the other object. + - no object has ownership of the other. + - typically each object is owned by a different parent. + - creation/destruction requires two locks and two refcounts. + + - usage + + Not used in GStreamer. + + - lifecycle + + + + + diff --git a/gst/gstbin.c b/gst/gstbin.c index 43423cb547..b4346ad58f 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -258,36 +258,34 @@ gst_bin_add_func (GstBin * bin, GstElement * element) GstPipeline *manager; gchar *elem_name; - GST_LOCK (element); - /* the element must not already have a parent */ - if (G_UNLIKELY (GST_ELEMENT_PARENT (element) != NULL)) - goto had_parent; - - elem_name = g_strdup (GST_ELEMENT_NAME (element)); - GST_UNLOCK (element); - - GST_LOCK (bin); /* we obviously can't add ourself to ourself */ if (G_UNLIKELY (GST_ELEMENT_CAST (element) == GST_ELEMENT_CAST (bin))) goto adding_itself; + GST_LOCK (element); + elem_name = g_strdup (GST_ELEMENT_NAME (element)); + GST_UNLOCK (element); + + GST_LOCK (bin); /* then check to see if the element's name is already taken in the bin, - * we can safely take the lock here. We can leave the element locked - * as it will not be in the bin. This check is probably bogus because + * we can safely take the lock here. This check is probably bogus because * you can safely change the element name after adding it to the bin. */ if (G_UNLIKELY (gst_object_check_uniqueness (bin->children, elem_name) == FALSE)) goto duplicate_name; - manager = GST_ELEMENT (bin)->manager; - gst_element_set_manager (element, manager); - /* set the element's parent and add the element to the bin's list of children */ - gst_object_set_parent (GST_OBJECT (element), GST_OBJECT (bin)); + if (G_UNLIKELY (!gst_object_set_parent (GST_OBJECT (element), + GST_OBJECT (bin)))) + goto had_parent; bin->children = g_list_prepend (bin->children, element); bin->numchildren++; bin->children_cookie++; + + manager = GST_ELEMENT (bin)->manager; + gst_element_set_manager (element, manager); + GST_UNLOCK (bin); GST_CAT_DEBUG_OBJECT (GST_CAT_PARENTAGE, bin, "added element \"%s\"", @@ -299,16 +297,10 @@ gst_bin_add_func (GstBin * bin, GstElement * element) return TRUE; /* ERROR handling here */ -had_parent: - g_warning ("Element %s already has parent %p", GST_ELEMENT_NAME (element), - bin); - GST_UNLOCK (element); - return FALSE; - adding_itself: + GST_LOCK (bin); g_warning ("Cannot add bin %s to itself", GST_ELEMENT_NAME (bin)); GST_UNLOCK (bin); - g_free (elem_name); return FALSE; duplicate_name: @@ -317,6 +309,12 @@ duplicate_name: GST_UNLOCK (bin); g_free (elem_name); return FALSE; + +had_parent: + g_warning ("Element %s already has parent", elem_name); + GST_UNLOCK (bin); + g_free (elem_name); + return FALSE; } /** @@ -364,18 +362,12 @@ gst_bin_remove_func (GstBin * bin, GstElement * element) { gchar *elem_name; - /* the element must have its parent set to the current bin */ GST_LOCK (element); - /* the element must not already have a parent */ - if (G_UNLIKELY (GST_ELEMENT_PARENT (element) != GST_ELEMENT_CAST (bin))) - goto wrong_parent; - elem_name = g_strdup (GST_ELEMENT_NAME (element)); GST_UNLOCK (element); GST_LOCK (bin); - /* the element must be in the bin's list of children, is this - * check redundant with PARENT checking? */ + /* the element must be in the bin's list of children */ if (G_UNLIKELY (g_list_find (bin->children, element) == NULL)) goto not_in_bin; @@ -391,18 +383,16 @@ gst_bin_remove_func (GstBin * bin, GstElement * element) gst_element_set_manager (element, NULL); - /* we should ref here to avoid bad app behaviour.. */ + /* we ref here because after the _unparent() the element can be disposed + * and we still need it to fire a signal. */ + gst_object_ref (GST_OBJECT (element)); gst_object_unparent (GST_OBJECT (element)); g_signal_emit (G_OBJECT (bin), gst_bin_signals[ELEMENT_REMOVED], 0, element); + gst_object_unref (GST_OBJECT (element)); return TRUE; -wrong_parent: - g_warning ("Element %s is not in bin %p", GST_ELEMENT_NAME (element), bin); - GST_UNLOCK (element); - return FALSE; - not_in_bin: g_warning ("Element %s is not in bin %s", elem_name, GST_ELEMENT_NAME (bin)); GST_UNLOCK (bin); @@ -454,43 +444,6 @@ no_function: return FALSE; } -/* - * bin iterator - */ -typedef struct _GstBinIterator -{ - GstIterator iterator; - GstBin *bin; - GList *list; /* pointer in list */ -} GstBinIterator; - -static GstIteratorResult -gst_bin_iterator_next (GstBinIterator * it, GstElement ** elem) -{ - if (it->list == NULL) - return GST_ITERATOR_DONE; - - *elem = GST_ELEMENT (it->list->data); - gst_object_ref (GST_OBJECT (*elem)); - - it->list = g_list_next (it->list); - - return GST_ITERATOR_OK; -} - -static void -gst_bin_iterator_resync (GstBinIterator * it) -{ - it->list = it->bin->children; -} - -static void -gst_bin_iterator_free (GstBinIterator * it) -{ - gst_object_unref (GST_OBJECT (it->bin)); - g_free (it); -} - /** * gst_bin_iterate_elements: * @bin: #Gstbin to iterate the elements of @@ -507,24 +460,22 @@ gst_bin_iterator_free (GstBinIterator * it) GstIterator * gst_bin_iterate_elements (GstBin * bin) { - GstBinIterator *result; + GstIterator *result; g_return_val_if_fail (GST_IS_BIN (bin), NULL); - /* ne need to lock, nothing can change here */ - result = (GstBinIterator *) gst_iterator_new (sizeof (GstBinIterator), - GST_GET_LOCK (bin), - &bin->children_cookie, - (GstIteratorNextFunction) gst_bin_iterator_next, - (GstIteratorResyncFunction) gst_bin_iterator_resync, - (GstIteratorFreeFunction) gst_bin_iterator_free); - GST_LOCK (bin); - result->bin = GST_BIN (gst_object_ref (GST_OBJECT (bin))); - result->list = bin->children; + gst_object_ref (GST_OBJECT (bin)); + result = gst_iterator_new_list (GST_GET_LOCK (bin), + &bin->children_cookie, + &bin->children, + bin, + (GstIteratorRefFunction) gst_object_ref, + (GstIteratorUnrefFunction) gst_object_unref, + (GstIteratorDisposeFunction) gst_object_unref); GST_UNLOCK (bin); - return GST_ITERATOR (result); + return result; } /* returns 0 if the element is a sink, this is made so that @@ -735,22 +686,22 @@ gst_bin_change_state (GstElement * element) /* FIXME does not work for bins etc */ peer_elem = GST_ELEMENT (gst_object_get_parent (GST_OBJECT (peer))); - GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, - "adding element %s to queue", gst_element_get_name (peer_elem)); + if (peer_elem) { + GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, + "adding element %s to queue", gst_element_get_name (peer_elem)); - /* ref before pushing on the queue */ - gst_object_ref (GST_OBJECT (peer_elem)); - g_queue_push_tail (elem_queue, peer_elem); + /* is reffed before pushing on the queue */ + g_queue_push_tail (elem_queue, peer_elem); + } + gst_object_unref (GST_OBJECT (peer)); } else { GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, "pad %s:%s does not have a peer", GST_DEBUG_PAD_NAME (pad)); } } - if (GST_FLAG_IS_SET (qelement, GST_ELEMENT_LOCKED_STATE)) { - gst_object_unref (GST_OBJECT (qelement)); - continue; - } + if (GST_FLAG_IS_SET (qelement, GST_ELEMENT_LOCKED_STATE)) + goto next_element; /* FIXME handle delayed elements like src and loop based * elements */ @@ -774,8 +725,6 @@ gst_bin_change_state (GstElement * element) GST_ELEMENT_NAME (qelement), pending, gst_element_state_get_name (pending)); ret = GST_STATE_FAILURE; - /* release refcounts in queue */ - g_queue_foreach (elem_queue, (GFunc) gst_object_unref, NULL); /* release refcount of element we popped off the queue */ gst_object_unref (GST_OBJECT (qelement)); goto exit; @@ -783,6 +732,7 @@ gst_bin_change_state (GstElement * element) g_assert_not_reached (); break; } + next_element: gst_object_unref (GST_OBJECT (qelement)); } @@ -807,6 +757,8 @@ gst_bin_change_state (GstElement * element) gst_element_state_get_name (GST_STATE (element))); exit: + /* release refcounts in queue, should normally be empty */ + g_queue_foreach (elem_queue, (GFunc) gst_object_unref, NULL); g_queue_free (elem_queue); return ret; @@ -820,6 +772,8 @@ gst_bin_dispose (GObject * object) GST_CAT_DEBUG_OBJECT (GST_CAT_REFCOUNTING, object, "dispose"); + /* ref to not hit 0 again */ + gst_object_ref (GST_OBJECT (object)); gst_element_set_state (GST_ELEMENT (object), GST_STATE_NULL); while (bin->children) { diff --git a/gst/gstcaps.c b/gst/gstcaps.c index 7cd760363a..df73055bfc 100644 --- a/gst/gstcaps.c +++ b/gst/gstcaps.c @@ -26,7 +26,7 @@ #include "gst_private.h" #include -#define DEBUG_REFCOUNT +//#define DEBUG_REFCOUNT #define CAPS_POISON(caps) G_STMT_START{ \ if (caps) { \ diff --git a/gst/gstelement.c b/gst/gstelement.c index 822889a3fb..7d75a361e1 100644 --- a/gst/gstelement.c +++ b/gst/gstelement.c @@ -1,6 +1,6 @@ /* GStreamer * Copyright (C) 1999,2000 Erik Walthinsen - * 2000 Wim Taymans + * 2004 Wim Taymans * * gstelement.c: The base element, all elements derive from this * @@ -441,6 +441,7 @@ gst_element_add_pad (GstElement * element, GstPad * pad) goto had_parent; GST_LOCK (element); + /* locking pad to look at the name */ GST_LOCK (pad); /* then check to see if there's already a pad by that name here */ if (G_UNLIKELY (!gst_object_check_uniqueness (element->pads, @@ -551,20 +552,25 @@ gst_element_add_ghost_pad (GstElement * element, GstPad * pad, * Removes @pad from @element. @pad will be destroyed if it has not been * referenced elsewhere. * + * Returns: TRUE if the pad could be removed. Can return FALSE if the + * pad is not belonging to the provided element or when wrong parameters + * are passed to this function. + * * MT safe. */ -void +gboolean gst_element_remove_pad (GstElement * element, GstPad * pad) { GstElement *current_parent; - g_return_if_fail (GST_IS_ELEMENT (element)); - g_return_if_fail (GST_IS_PAD (pad)); + g_return_val_if_fail (GST_IS_ELEMENT (element), FALSE); + g_return_val_if_fail (GST_IS_PAD (pad), FALSE); current_parent = gst_pad_get_parent (pad); if (G_UNLIKELY (current_parent != element)) goto not_our_pad; + /* FIXME, is this redundant with pad disposal? */ if (GST_IS_REAL_PAD (pad)) { GstPad *peer = gst_pad_get_peer (pad); @@ -573,7 +579,10 @@ gst_element_remove_pad (GstElement * element, GstPad * pad) /* window for MT unsafeness, someone else could unlink here * and then we call unlink with wrong pads. The unlink * function would catch this and safely return failed. */ - gst_pad_unlink (pad, GST_PAD_CAST (peer)); + if (GST_PAD_IS_SRC (pad)) + gst_pad_unlink (pad, GST_PAD_CAST (peer)); + else + gst_pad_unlink (GST_PAD_CAST (peer), pad); gst_object_unref (GST_OBJECT (peer)); } } else if (GST_IS_GHOST_PAD (pad)) { @@ -604,7 +613,7 @@ gst_element_remove_pad (GstElement * element, GstPad * pad) gst_object_unparent (GST_OBJECT (pad)); - return; + return TRUE; not_our_pad: { @@ -618,7 +627,7 @@ not_our_pad: GST_UNLOCK (element); GST_UNLOCK (pad); g_free (parent_name); - return; + return FALSE; } } @@ -642,36 +651,16 @@ gst_element_no_more_pads (GstElement * element) g_signal_emit (element, gst_element_signals[NO_MORE_PADS], 0); } -/** - * gst_element_get_pad: - * @element: a #GstElement. - * @name: the name of the pad to retrieve. - * - * Retrieves a pad from @element by name. Tries gst_element_get_static_pad() - * first, then gst_element_get_request_pad(). - * - * Returns: the #GstPad if found, otherwise %NULL. - */ -GstPad * -gst_element_get_pad (GstElement * element, const gchar * name) -{ - GstPad *pad; - - g_return_val_if_fail (element != NULL, NULL); - g_return_val_if_fail (GST_IS_ELEMENT (element), NULL); - g_return_val_if_fail (name != NULL, NULL); - - pad = gst_element_get_static_pad (element, name); - if (!pad) - pad = gst_element_get_request_pad (element, name); - - return pad; -} - static gint pad_compare_name (GstPad * pad1, const gchar * name) { - return strcmp (GST_PAD_NAME (pad1), name); + gint result; + + GST_LOCK (pad1); + result = strcmp (GST_PAD_NAME (pad1), name); + GST_UNLOCK (pad1); + + return result; } /** @@ -682,7 +671,10 @@ pad_compare_name (GstPad * pad1, const gchar * name) * Retrieves a pad from @element by name. This version only retrieves * already-existing (i.e. 'static') pads. * - * Returns: the requested #GstPad if found, otherwise NULL. + * Returns: the requested #GstPad if found, otherwise NULL. unref after + * usage. + * + * MT safe. */ GstPad * gst_element_get_static_pad (GstElement * element, const gchar * name) @@ -690,24 +682,26 @@ gst_element_get_static_pad (GstElement * element, const gchar * name) GList *find; GstPad *result = NULL; - g_return_val_if_fail (GST_IS_ELEMENT (element), result); - g_return_val_if_fail (name != NULL, result); + g_return_val_if_fail (GST_IS_ELEMENT (element), NULL); + g_return_val_if_fail (name != NULL, NULL); GST_LOCK (element); find = g_list_find_custom (element->pads, name, (GCompareFunc) pad_compare_name); if (find) { result = GST_PAD (find->data); + gst_object_ref (GST_OBJECT (result)); } - GST_UNLOCK (element); if (result == NULL) { GST_CAT_INFO (GST_CAT_ELEMENT_PADS, "no such pad '%s' in element \"%s\"", - name, GST_OBJECT_NAME (element)); + name, GST_ELEMENT_NAME (element)); } else { GST_CAT_INFO (GST_CAT_ELEMENT_PADS, "found pad %s:%s", - GST_DEBUG_PAD_NAME (result)); + GST_ELEMENT_NAME (element), name); } + GST_UNLOCK (element); + return result; } @@ -749,7 +743,6 @@ gst_element_get_request_pad (GstElement * element, const gchar * name) gchar *str, *endptr = NULL; GstElementClass *class; - g_return_val_if_fail (element != NULL, NULL); g_return_val_if_fail (GST_IS_ELEMENT (element), NULL); g_return_val_if_fail (name != NULL, NULL); @@ -804,30 +797,60 @@ gst_element_get_request_pad (GstElement * element, const gchar * name) } /** - * gst_element_get_pad_list: - * @element: a #GstElement to get pads of. + * gst_element_get_pad: + * @element: a #GstElement. + * @name: the name of the pad to retrieve. * - * Retrieves a list of @element's pads. The list must not be modified by the - * calling code. + * Retrieves a pad from @element by name. Tries gst_element_get_static_pad() + * first, then gst_element_get_request_pad(). * - * Returns: the #GList of pads. + * Returns: the #GstPad if found, otherwise %NULL. Unref after usage. */ -const GList * -gst_element_get_pad_list (GstElement * element) +GstPad * +gst_element_get_pad (GstElement * element, const gchar * name) { - const GList *result = NULL; + GstPad *pad; - g_return_val_if_fail (element != NULL, result); - g_return_val_if_fail (GST_IS_ELEMENT (element), result); + g_return_val_if_fail (GST_IS_ELEMENT (element), NULL); + g_return_val_if_fail (name != NULL, NULL); - g_warning ("calling gst_element_get_pad_list is MT unsafe!!"); + pad = gst_element_get_static_pad (element, name); + if (!pad) + pad = gst_element_get_request_pad (element, name); - /* return the list of pads */ - result = element->pads; + return pad; +} + +/** + * gst_element_iterate_pads: + * @element: a #GstElement to iterate pads of. + * + * Retrieves an iterattor of @element's pads. + * + * Returns: the #GstIterator of pads. + */ +GstIterator * +gst_element_iterate_pads (GstElement * element) +{ + GstIterator *result; + + g_return_val_if_fail (GST_IS_ELEMENT (element), NULL); + + GST_LOCK (element); + gst_object_ref (GST_OBJECT (element)); + result = gst_iterator_new_list (GST_GET_LOCK (element), + &element->pads_cookie, + &element->pads, + element, + (GstIteratorRefFunction) gst_object_ref, + (GstIteratorUnrefFunction) gst_object_unref, + (GstIteratorDisposeFunction) gst_object_unref); + GST_UNLOCK (element); return result; } + /** * gst_element_class_add_pad_template: * @klass: the #GstElementClass to add the pad template to. @@ -885,7 +908,6 @@ gst_element_class_set_details (GstElementClass * klass, GList * gst_element_class_get_pad_template_list (GstElementClass * element_class) { - g_return_val_if_fail (element_class != NULL, NULL); g_return_val_if_fail (GST_IS_ELEMENT_CLASS (element_class), NULL); return element_class->padtemplates; @@ -910,7 +932,6 @@ gst_element_class_get_pad_template (GstElementClass * element_class, { GList *padlist; - g_return_val_if_fail (element_class != NULL, NULL); g_return_val_if_fail (GST_IS_ELEMENT_CLASS (element_class), NULL); g_return_val_if_fail (name != NULL, NULL); @@ -949,17 +970,23 @@ gst_element_get_random_pad (GstElement * element, GstPadDirection dir) for (; pads; pads = g_list_next (pads)) { GstPad *pad = GST_PAD (pads->data); + GST_LOCK (pad); GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "checking pad %s:%s", GST_DEBUG_PAD_NAME (pad)); if (GST_PAD_IS_LINKED (pad)) { + GST_UNLOCK (pad); result = pad; break; } else { GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "pad %s:%s is not linked", GST_DEBUG_PAD_NAME (pad)); } + GST_UNLOCK (pad); } + if (result) + gst_object_ref (GST_OBJECT (result)); + GST_UNLOCK (element); return result; @@ -973,27 +1000,38 @@ gst_element_get_random_pad (GstElement * element, GstPadDirection dir) * If the element doesn't implement an event masks function, * the query will be forwarded to a random linked sink pad. * - * Returns: An array of #GstEventMask elements. + * Returns: An array of #GstEventMask elements. The array + * cannot be modified or freed. + * + * MT safe. */ const GstEventMask * gst_element_get_event_masks (GstElement * element) { GstElementClass *oclass; + const GstEventMask *result = NULL; g_return_val_if_fail (GST_IS_ELEMENT (element), NULL); oclass = GST_ELEMENT_GET_CLASS (element); - if (oclass->get_event_masks) - return oclass->get_event_masks (element); - else { + if (oclass->get_event_masks) { + result = oclass->get_event_masks (element); + } else { GstPad *pad = gst_element_get_random_pad (element, GST_PAD_SINK); - if (pad) - return gst_pad_get_event_masks (GST_PAD_PEER (pad)); + if (pad) { + GstPad *peer = gst_pad_get_peer (pad); + + if (peer) { + result = gst_pad_get_event_masks (peer); + gst_object_unref (GST_OBJECT (peer)); + } + gst_object_unref (GST_OBJECT (pad)); + } } - return NULL; + return result; } /** @@ -1003,34 +1041,47 @@ gst_element_get_event_masks (GstElement * element) * * Sends an event to an element. If the element doesn't * implement an event handler, the event will be forwarded - * to a random sink pad. + * to a random sink pad. This function takes owership of the + * provided event so you should _ref it if you want to reuse + * the event after this call. * * Returns: TRUE if the event was handled. + * + * MT safe. */ gboolean gst_element_send_event (GstElement * element, GstEvent * event) { GstElementClass *oclass; + gboolean result = FALSE; g_return_val_if_fail (GST_IS_ELEMENT (element), FALSE); g_return_val_if_fail (event != NULL, FALSE); oclass = GST_ELEMENT_GET_CLASS (element); - if (oclass->send_event) - return oclass->send_event (element, event); - else { + if (oclass->send_event) { + result = oclass->send_event (element, event); + } else { GstPad *pad = gst_element_get_random_pad (element, GST_PAD_SINK); if (pad) { - GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "sending event to random pad %s:%s", - GST_DEBUG_PAD_NAME (pad)); - return gst_pad_send_event (GST_PAD_PEER (pad), event); + GstPad *peer = gst_pad_get_peer (pad); + + if (peer) { + GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, + "sending event to random pad %s:%s", GST_DEBUG_PAD_NAME (pad)); + + result = gst_pad_send_event (peer, event); + gst_object_unref (GST_OBJECT (peer)); + } + gst_object_unref (GST_OBJECT (pad)); } } GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "can't send event on element %s", GST_ELEMENT_NAME (element)); - return FALSE; + + return result; } /** @@ -1042,13 +1093,18 @@ gst_element_send_event (GstElement * element, GstEvent * event) * Sends a seek event to an element. * * Returns: TRUE if the event was handled. + * + * MT safe. */ gboolean gst_element_seek (GstElement * element, GstSeekType seek_type, guint64 offset) { GstEvent *event = gst_event_new_seek (seek_type, offset); + gboolean result; - return gst_element_send_event (element, event); + result = gst_element_send_event (element, event); + + return result; } /** @@ -1059,27 +1115,38 @@ gst_element_seek (GstElement * element, GstSeekType seek_type, guint64 offset) * If the element doesn't implement a query types function, * the query will be forwarded to a random sink pad. * - * Returns: An array of #GstQueryType elements. + * Returns: An array of #GstQueryType elements that should not + * be freed or modified. + * + * MT safe. */ const GstQueryType * gst_element_get_query_types (GstElement * element) { GstElementClass *oclass; + const GstQueryType *result = NULL; g_return_val_if_fail (GST_IS_ELEMENT (element), NULL); oclass = GST_ELEMENT_GET_CLASS (element); - if (oclass->get_query_types) - return oclass->get_query_types (element); - else { + if (oclass->get_query_types) { + result = oclass->get_query_types (element); + } else { GstPad *pad = gst_element_get_random_pad (element, GST_PAD_SINK); - if (pad) - return gst_pad_get_query_types (GST_PAD_PEER (pad)); - } + if (pad) { + GstPad *peer = gst_pad_get_peer (pad); - return NULL; + if (peer) { + result = gst_pad_get_query_types (peer); + + gst_object_unref (GST_OBJECT (peer)); + } + gst_object_unref (GST_OBJECT (pad)); + } + } + return result; } /** @@ -1102,6 +1169,7 @@ gst_element_query (GstElement * element, GstQueryType type, GstFormat * format, gint64 * value) { GstElementClass *oclass; + gboolean result = FALSE; g_return_val_if_fail (GST_IS_ELEMENT (element), FALSE); g_return_val_if_fail (format != NULL, FALSE); @@ -1109,19 +1177,30 @@ gst_element_query (GstElement * element, GstQueryType type, oclass = GST_ELEMENT_GET_CLASS (element); - if (oclass->query) - return oclass->query (element, type, format, value); - else { + if (oclass->query) { + result = oclass->query (element, type, format, value); + } else { GstPad *pad = gst_element_get_random_pad (element, GST_PAD_SRC); - if (pad) - return gst_pad_query (pad, type, format, value); - pad = gst_element_get_random_pad (element, GST_PAD_SINK); - if (pad) - return gst_pad_query (GST_PAD_PEER (pad), type, format, value); - } + if (pad) { + result = gst_pad_query (pad, type, format, value); - return FALSE; + gst_object_unref (GST_OBJECT (pad)); + } else { + pad = gst_element_get_random_pad (element, GST_PAD_SINK); + if (pad) { + GstPad *peer = gst_pad_get_peer (pad); + + if (peer) { + result = gst_pad_query (peer, type, format, value); + + gst_object_unref (GST_OBJECT (peer)); + } + gst_object_unref (GST_OBJECT (pad)); + } + } + } + return result; } /** @@ -1138,21 +1217,30 @@ const GstFormat * gst_element_get_formats (GstElement * element) { GstElementClass *oclass; + const GstFormat *result = NULL; g_return_val_if_fail (GST_IS_ELEMENT (element), NULL); oclass = GST_ELEMENT_GET_CLASS (element); - if (oclass->get_formats) - return oclass->get_formats (element); - else { + if (oclass->get_formats) { + result = oclass->get_formats (element); + } else { GstPad *pad = gst_element_get_random_pad (element, GST_PAD_SINK); - if (pad) - return gst_pad_get_formats (GST_PAD_PEER (pad)); + if (pad) { + GstPad *peer = gst_pad_get_peer (pad); + + if (peer) { + result = gst_pad_get_formats (peer); + + gst_object_unref (GST_OBJECT (peer)); + } + gst_object_unref (GST_OBJECT (pad)); + } } - return NULL; + return result; } /** @@ -1175,6 +1263,7 @@ gst_element_convert (GstElement * element, GstFormat * dest_format, gint64 * dest_value) { GstElementClass *oclass; + gboolean result = FALSE; g_return_val_if_fail (GST_IS_ELEMENT (element), FALSE); g_return_val_if_fail (dest_format != NULL, FALSE); @@ -1187,36 +1276,53 @@ gst_element_convert (GstElement * element, oclass = GST_ELEMENT_GET_CLASS (element); - if (oclass->convert) - return oclass->convert (element, + if (oclass->convert) { + result = oclass->convert (element, src_format, src_value, dest_format, dest_value); - else { + } else { GstPad *pad = gst_element_get_random_pad (element, GST_PAD_SINK); - if (pad) - return gst_pad_convert (GST_PAD_PEER (pad), - src_format, src_value, dest_format, dest_value); - } + if (pad) { + GstPad *peer = gst_pad_get_peer (pad); - return FALSE; + if (peer) { + result = gst_pad_convert (peer, + src_format, src_value, dest_format, dest_value); + + gst_object_unref (GST_OBJECT (peer)); + } + gst_object_unref (GST_OBJECT (pad)); + } + } + return result; } -/* MT safe */ +/** + * gst_element_post_message: + * @element: a #GstElement posting the message + * @message: a #GstMessage to post + * + * Post a message on the elements #GstBus. + * + * Returns: TRUE if the message was successfuly posted. + * + * MT safe. + */ gboolean gst_element_post_message (GstElement * element, GstMessage * message) { GstPipeline *manager; gboolean result = FALSE; - g_return_val_if_fail (GST_IS_ELEMENT (element), result); - g_return_val_if_fail (message != NULL, result); + g_return_val_if_fail (GST_IS_ELEMENT (element), FALSE); + g_return_val_if_fail (message != NULL, FALSE); GST_LOCK (element); manager = element->manager; if (manager == NULL) { GST_UNLOCK (element); gst_data_unref (GST_DATA (message)); - return result; + return FALSE; } gst_object_ref (GST_OBJECT (manager)); GST_UNLOCK (element); @@ -1234,6 +1340,8 @@ gst_element_post_message (GstElement * element, GstMessage * message) * This function is only used internally by the #gst_element_error macro. * * Returns: a newly allocated string, or NULL if the format was NULL or "" + * + * MT safe. */ gchar * _gst_element_error_printf (const gchar * format, ...) @@ -1268,6 +1376,8 @@ _gst_element_error_printf (const gchar * format, ...) * Signals an error condition on an element. * This function is used internally by elements. * It results in the "error" signal. + * + * MT safe. */ void gst_element_error_full (GstElement * element, GQuark domain, gint code, gchar * message, @@ -1363,6 +1473,8 @@ gst_element_is_locked_state (GstElement * element) * * Returns: TRUE if the state was changed, FALSE if bad params were given or * the element was already in the correct state. + * + * MT safe. */ gboolean gst_element_set_locked_state (GstElement * element, gboolean locked_state) @@ -1386,6 +1498,8 @@ gst_element_set_locked_state (GstElement * element, gboolean locked_state) GST_ELEMENT_NAME (element)); GST_FLAG_UNSET (element, GST_ELEMENT_LOCKED_STATE); } + GST_UNLOCK (element); + return TRUE; was_ok: @@ -1423,6 +1537,7 @@ gst_element_sync_state_with_parent (GstElement * element) return TRUE; } +/* MT safe */ static gboolean gst_element_get_state_func (GstElement * element, GstElementState * state, GstElementState * pending, GTimeVal * timeout) @@ -1461,7 +1576,6 @@ gst_element_get_state_func (GstElement * element, return ret; } - /** * gst_element_get_state: * @element: a #GstElement to get the state of. @@ -1475,21 +1589,24 @@ gst_element_get_state_func (GstElement * element, * * Returns: TRUE if the element has no more pending state, FALSE * if the element is still performing a state change. + * + * MT safe. */ gboolean gst_element_get_state (GstElement * element, GstElementState * state, GstElementState * pending, GTimeVal * timeout) { GstElementClass *oclass; + gboolean result = FALSE; g_return_val_if_fail (GST_IS_ELEMENT (element), FALSE); oclass = GST_ELEMENT_GET_CLASS (element); if (oclass->get_state) - return (oclass->get_state) (element, state, pending, timeout); + result = (oclass->get_state) (element, state, pending, timeout); - return FALSE; + return result; } /** @@ -1499,6 +1616,10 @@ gst_element_get_state (GstElement * element, * Abort the state change of the element. This function is used * by elements that do asynchronous state changes and find out * something is wrong. + * + * This function should be called with the STATE_LOCK held. + * + * MT safe. */ void gst_element_abort_state (GstElement * element) @@ -1528,6 +1649,10 @@ gst_element_abort_state (GstElement * element) * * Commit the state change of the element. This function is used * by elements that do asynchronous state changes. + * + * This function can only be called with the STATE_LOCK held. + * + * MT safe. */ void gst_element_commit_state (GstElement * element) @@ -1563,8 +1688,9 @@ gst_element_commit_state (GstElement * element) * requested state by going through all the intermediary states and calling * the class's state change function for each. * - * Returns: TRUE if the state was successfully set. - * (using #GstElementStateReturn). + * Returns: Result of the state change using #GstElementStateReturn. + * + * MT safe. */ GstElementStateReturn gst_element_set_state (GstElement * element, GstElementState state) @@ -1575,9 +1701,6 @@ gst_element_set_state (GstElement * element, GstElementState state) oclass = GST_ELEMENT_GET_CLASS (element); - /* reentrancy issues with signals in change_state) */ - gst_object_ref (GST_OBJECT (element)); - /* get the element state lock */ GST_STATE_LOCK (element); @@ -1657,17 +1780,18 @@ exit: GST_CAT_INFO_OBJECT (GST_CAT_STATES, element, "exit state change"); - gst_object_unref (GST_OBJECT (element)); - return return_val; } +/* is called with STATE_LOCK */ +/* FIXME make MT safe */ static gboolean gst_element_pads_activate (GstElement * element, gboolean active) { - GList *pads = element->pads; + GList *pads; gboolean result = TRUE; + pads = element->pads; while (pads && result) { GstPad *pad = GST_PAD (pads->data); @@ -1682,6 +1806,8 @@ gst_element_pads_activate (GstElement * element, gboolean active) return result; } +/* is called with STATE_LOCK */ +/* FIXME make MT safe */ static GstElementStateReturn gst_element_change_state (GstElement * element) { @@ -1771,6 +1897,8 @@ gst_element_dispose (GObject * object) GST_CAT_INFO_OBJECT (GST_CAT_REFCOUNTING, element, "dispose"); + /* ref so we don't hit 0 again */ + gst_object_ref (GST_OBJECT (object)); gst_element_set_state (element, GST_STATE_NULL); /* first we break all our links with the ouside */ @@ -1778,15 +1906,18 @@ gst_element_dispose (GObject * object) gst_element_remove_pad (element, GST_PAD (element->pads->data)); } - element->numsrcpads = 0; - element->numsinkpads = 0; - element->numpads = 0; + g_assert (element->pads == 0); + + GST_LOCK (element); + gst_object_replace ((GstObject **) & element->manager, NULL); + gst_object_replace ((GstObject **) & element->clock, NULL); + GST_UNLOCK (element); + + GST_STATE_LOCK (element); if (element->state_cond) g_cond_free (element->state_cond); element->state_cond = NULL; - - gst_object_replace ((GstObject **) & element->manager, NULL); - gst_object_replace ((GstObject **) & element->clock, NULL); + GST_STATE_UNLOCK (element); GST_CAT_INFO_OBJECT (GST_CAT_REFCOUNTING, element, "dispose parent"); @@ -1936,9 +2067,9 @@ gst_element_set_manager_func (GstElement * element, GstPipeline * manager) GST_CAT_DEBUG_OBJECT (GST_CAT_PARENTAGE, element, "setting manager to %p", manager); + /* setting the manager cannot increase the refcount */ GST_LOCK (element); - gst_object_replace ((GstObject **) & GST_ELEMENT_MANAGER (element), - GST_OBJECT (manager)); + GST_ELEMENT_MANAGER (element) = manager; GST_UNLOCK (element); } @@ -1949,6 +2080,8 @@ gst_element_set_manager_func (GstElement * element, GstPipeline * manager) * * Sets the manager of the element. For internal use only, unless you're * writing a new bin subclass. + * + * MT safe. */ void gst_element_set_manager (GstElement * element, GstPipeline * manager) @@ -1967,9 +2100,9 @@ gst_element_set_manager (GstElement * element, GstPipeline * manager) * gst_element_get_manager: * @element: a #GstElement to get the manager of. * - * Returns the manager of the element. + * Returns the manager of the element. * - * Returns: the element's #GstPipeline. + * Returns: the element's #GstPipeline. unref after usage. * * MT safe. */ @@ -1982,6 +2115,7 @@ gst_element_get_manager (GstElement * element) GST_LOCK (element); result = GST_ELEMENT_MANAGER (element); + gst_object_ref (GST_OBJECT (result)); GST_UNLOCK (element); return result; diff --git a/gst/gstelement.h b/gst/gstelement.h index 8e914d36bf..d92eeb2236 100644 --- a/gst/gstelement.h +++ b/gst/gstelement.h @@ -279,7 +279,7 @@ GstTask* gst_element_create_task (GstElement *element, GstTaskFunction func, g /* pad management */ gboolean gst_element_add_pad (GstElement *element, GstPad *pad); -void gst_element_remove_pad (GstElement *element, GstPad *pad); +gboolean gst_element_remove_pad (GstElement *element, GstPad *pad); GstPad * gst_element_add_ghost_pad (GstElement *element, GstPad *pad, const gchar *name); void gst_element_no_more_pads (GstElement *element); @@ -288,8 +288,6 @@ GstPad* gst_element_get_static_pad (GstElement *element, const gchar *name); GstPad* gst_element_get_request_pad (GstElement *element, const gchar *name); void gst_element_release_request_pad (GstElement *element, GstPad *pad); -G_CONST_RETURN GList* - gst_element_get_pad_list (GstElement *element); GstIterator* gst_element_iterate_pads (GstElement *element); /* event/query/format stuff */ diff --git a/gst/gstiterator.c b/gst/gstiterator.c index 4c995c239d..04fb41c4fb 100644 --- a/gst/gstiterator.c +++ b/gst/gstiterator.c @@ -58,6 +58,78 @@ gst_iterator_new (guint size, return result; } +/* + * list iterator + */ +typedef struct _GstListIterator +{ + GstIterator iterator; + gpointer owner; + GList **orig; + GList *list; /* pointer in list */ + GstIteratorRefFunction reffunc; + GstIteratorUnrefFunction unreffunc; + GstIteratorDisposeFunction freefunc; +} GstListIterator; + +static GstIteratorResult +gst_list_iterator_next (GstListIterator * it, gpointer * elem) +{ + if (it->list == NULL) + return GST_ITERATOR_DONE; + + *elem = it->list->data; + if (it->reffunc) { + it->reffunc (*elem); + } + it->list = g_list_next (it->list); + + return GST_ITERATOR_OK; +} + +static void +gst_list_iterator_resync (GstListIterator * it) +{ + it->list = *it->orig; +} + +static void +gst_list_iterator_free (GstListIterator * it) +{ + if (it->freefunc) { + it->freefunc (it->owner); + } + g_free (it); +} + +GstIterator * +gst_iterator_new_list (GMutex * lock, + guint32 * master_cookie, + GList ** list, + gpointer owner, + GstIteratorRefFunction ref, + GstIteratorUnrefFunction unref, GstIteratorDisposeFunction free) +{ + GstListIterator *result; + + /* no need to lock, nothing can change here */ + result = (GstListIterator *) gst_iterator_new (sizeof (GstListIterator), + lock, + master_cookie, + (GstIteratorNextFunction) gst_list_iterator_next, + (GstIteratorResyncFunction) gst_list_iterator_resync, + (GstIteratorFreeFunction) gst_list_iterator_free); + + result->owner = owner; + result->orig = list; + result->list = *list; + result->reffunc = ref; + result->unreffunc = unref; + result->freefunc = free; + + return GST_ITERATOR (result); +} + GstIteratorResult gst_iterator_next (GstIterator * it, gpointer * elem) { diff --git a/gst/gstiterator.h b/gst/gstiterator.h index 0697686dba..0f5f35f5ce 100644 --- a/gst/gstiterator.h +++ b/gst/gstiterator.h @@ -35,6 +35,10 @@ typedef enum { typedef struct _GstIterator GstIterator; +typedef void (*GstIteratorRefFunction) (gpointer item); +typedef void (*GstIteratorUnrefFunction) (gpointer item); +typedef void (*GstIteratorDisposeFunction) (gpointer owner); + typedef GstIteratorResult (*GstIteratorNextFunction) (GstIterator *it, gpointer *result); typedef void (*GstIteratorResyncFunction) (GstIterator *it); typedef void (*GstIteratorFreeFunction) (GstIterator *it); @@ -49,7 +53,7 @@ struct _GstIterator { GstIteratorResyncFunction resync; GstIteratorFreeFunction free; - GMutex *lock; + GMutex *lock; guint32 cookie; /* cookie of the iterator */ guint32 *master_cookie; /* pointer to guint32 holding the cookie when this iterator was created */ @@ -62,9 +66,13 @@ GstIterator* gst_iterator_new (guint size, GstIteratorResyncFunction resync, GstIteratorFreeFunction free); -GstIterator* gst_iterator_new_list (GMutex *lock, +GstIterator* gst_iterator_new_list (GMutex *lock, guint32 *master_cookie, - GList *list); + GList **list, + gpointer owner, + GstIteratorRefFunction ref, + GstIteratorUnrefFunction unref, + GstIteratorDisposeFunction free); GstIteratorResult gst_iterator_next (GstIterator *it, gpointer *result); void gst_iterator_resync (GstIterator *it); diff --git a/gst/gstmessage.c b/gst/gstmessage.c index 6bce2bdfda..8bed94a1f0 100644 --- a/gst/gstmessage.c +++ b/gst/gstmessage.c @@ -61,12 +61,17 @@ _gst_message_copy (GstMessage * message) { GstMessage *copy; + GST_CAT_INFO (GST_CAT_MESSAGE, "copy message %p", message); + copy = gst_mem_chunk_alloc (chunk); #ifndef GST_DISABLE_TRACE gst_alloc_trace_new (_message_trace, copy); #endif memcpy (copy, message, sizeof (GstMessage)); + if (GST_MESSAGE_SRC (copy)) { + gst_object_ref (GST_MESSAGE_SRC (copy)); + } /* FIXME copy/ref additional fields */ switch (GST_MESSAGE_TYPE (message)) { diff --git a/gst/gstobject.c b/gst/gstobject.c index 79f3577663..41baa765fa 100644 --- a/gst/gstobject.c +++ b/gst/gstobject.c @@ -30,6 +30,17 @@ #include "gsttrace.h" #endif +#define DEBUG_REFCOUNT +#define REFCOUNT_HACK + +#ifdef REFCOUNT_HACK +#define PATCH_REFCOUNT(obj) ((GObject*)(obj))->ref_count = 100000; +#define PATCH_REFCOUNT1(obj) ((GObject*)(obj))->ref_count = 1; +#else +#define PATCH_REFCOUNT(obj) +#define PATCH_REFCOUNT1(obj) +#endif + /* Object signals and args */ enum { @@ -162,6 +173,7 @@ gst_object_class_init (GstObjectClass * klass) G_TYPE_PARAM); klass->path_string_separator = "/"; + klass->lock = g_mutex_new (); klass->signal_object = g_object_new (gst_signal_object_get_type (), NULL); @@ -183,6 +195,10 @@ gst_object_init (GstObject * object) object->parent = NULL; object->name = NULL; + gst_atomic_int_init (&(object)->refcount, 1); +#ifdef REFCOUNT_HACK + PATCH_REFCOUNT (object); +#endif object->flags = 0; GST_FLAG_SET (object, GST_OBJECT_FLOATING); @@ -229,11 +245,25 @@ gst_object_ref (GstObject * object) { g_return_val_if_fail (GST_IS_OBJECT (object), NULL); +#ifdef DEBUG_REFCOUNT +#ifdef REFCOUNT_HACK GST_CAT_LOG_OBJECT (GST_CAT_REFCOUNTING, object, "ref %d->%d", - G_OBJECT (object)->ref_count, G_OBJECT (object)->ref_count + 1); + GST_OBJECT_REFCOUNT_VALUE (object), + GST_OBJECT_REFCOUNT_VALUE (object) + 1); +#else + GST_CAT_LOG_OBJECT (GST_CAT_REFCOUNTING, object, "ref %d->%d", + ((GObject *) object)->ref_count, ((GObject *) object)->ref_count + 1); +#endif +#endif +#ifdef REFCOUNT_HACK + gst_atomic_int_inc (&object->refcount); + PATCH_REFCOUNT (object); +#else /* FIXME, not MT safe because glib is not MT safe */ - g_object_ref (G_OBJECT (object)); + g_object_ref (object); +#endif + return object; } @@ -244,18 +274,50 @@ gst_object_ref (GstObject * object) * Decrements the refence count on the object. If reference count hits * zero, destroy the object. This function does not take the lock * on the object as it relies on atomic refcounting. + * + * The unref method should never be called with the LOCK held since + * this might deadlock the dispose function. + * + * Returns: NULL, so that constructs like + * + * object = gst_object_unref (object); + * + * automatically clear the input object pointer. */ -void +GstObject * gst_object_unref (GstObject * object) { - g_return_if_fail (GST_IS_OBJECT (object)); - g_return_if_fail (G_OBJECT (object)->ref_count > 0); + g_return_val_if_fail (GST_IS_OBJECT (object), NULL); +#ifdef REFCOUNT_HACK + g_return_val_if_fail (GST_OBJECT_REFCOUNT_VALUE (object) > 0, NULL); +#else + g_return_val_if_fail (((GObject *) object)->ref_count > 0, NULL); +#endif + +#ifdef DEBUG_REFCOUNT +#ifdef REFCOUNT_HACK GST_CAT_LOG_OBJECT (GST_CAT_REFCOUNTING, object, "unref %d->%d", - G_OBJECT (object)->ref_count, G_OBJECT (object)->ref_count - 1); + GST_OBJECT_REFCOUNT_VALUE (object), + GST_OBJECT_REFCOUNT_VALUE (object) - 1); +#else + GST_CAT_LOG_OBJECT (GST_CAT_REFCOUNTING, object, "unref %d->%d", + ((GObject *) object)->ref_count, ((GObject *) object)->ref_count - 1); +#endif +#endif - /* FIXME, not MT safe because glib is not MT safe */ - g_object_unref (G_OBJECT (object)); +#ifdef REFCOUNT_HACK + if (G_UNLIKELY (gst_atomic_int_dec_and_test (&object->refcount))) { + PATCH_REFCOUNT1 (object); + g_object_unref (object); + } else { + PATCH_REFCOUNT (object); + } +#else + g_object_unref (object); +#endif + + return NULL; } /** @@ -279,11 +341,14 @@ gst_object_sink (GstObject * object) GST_CAT_LOG_OBJECT (GST_CAT_REFCOUNTING, object, "sink"); GST_LOCK (object); - if (GST_OBJECT_IS_FLOATING (object)) { + if (G_LIKELY (GST_OBJECT_IS_FLOATING (object))) { GST_FLAG_UNSET (object, GST_OBJECT_FLOATING); + GST_UNLOCK (object); gst_object_unref (object); + } else { + GST_UNLOCK (object); } - GST_UNLOCK (object); + return; } /** @@ -304,13 +369,23 @@ gst_object_replace (GstObject ** oldobj, GstObject * newobj) g_return_if_fail (*oldobj == NULL || GST_IS_OBJECT (*oldobj)); g_return_if_fail (newobj == NULL || GST_IS_OBJECT (newobj)); +#ifdef DEBUG_REFCOUNT +#ifdef REFCOUNT_HACK + GST_CAT_LOG (GST_CAT_REFCOUNTING, "replace %s (%d) with %s (%d)", + *oldobj ? GST_STR_NULL (GST_OBJECT_NAME (*oldobj)) : "(NONE)", + *oldobj ? GST_OBJECT_REFCOUNT_VALUE (*oldobj) : 0, + newobj ? GST_STR_NULL (GST_OBJECT_NAME (newobj)) : "(NONE)", + newobj ? GST_OBJECT_REFCOUNT_VALUE (newobj) : 0); +#else GST_CAT_LOG (GST_CAT_REFCOUNTING, "replace %s (%d) with %s (%d)", *oldobj ? GST_STR_NULL (GST_OBJECT_NAME (*oldobj)) : "(NONE)", *oldobj ? G_OBJECT (*oldobj)->ref_count : 0, newobj ? GST_STR_NULL (GST_OBJECT_NAME (newobj)) : "(NONE)", newobj ? G_OBJECT (newobj)->ref_count : 0); +#endif +#endif - if (*oldobj != newobj) { + if (G_UNLIKELY (*oldobj != newobj)) { if (newobj) gst_object_ref (newobj); if (*oldobj) @@ -320,15 +395,23 @@ gst_object_replace (GstObject ** oldobj, GstObject * newobj) } } +/* dispose is called when the object has to release all links + * to other objects */ static void gst_object_dispose (GObject * object) { GST_CAT_LOG_OBJECT (GST_CAT_REFCOUNTING, object, "dispose"); + GST_LOCK (object); GST_FLAG_SET (GST_OBJECT (object), GST_OBJECT_DESTROYED); GST_OBJECT_PARENT (object) = NULL; + GST_UNLOCK (object); + + /* need to patch refcount so it is finalized */ + PATCH_REFCOUNT1 (object) + + parent_class->dispose (object); - parent_class->dispose (object); } /* finalize is called when the object has to free its resources */ @@ -359,49 +442,62 @@ gst_object_finalize (GObject * object) parent_class->finalize (object); } -/* FIXME a class wide mutex is enough */ -static GStaticRecMutex dispatch_mutex = G_STATIC_REC_MUTEX_INIT; - /* Changing a GObject property of a GstObject will result in "deep_notify" * signals being emitted by the object itself, as well as in each parent * object. This is so that an application can connect a listener to the * top-level bin to catch property-change notifications for all contained - * elements. */ + * elements. + * + * This function is not MT safe in glib so we need to lock it with a + * classwide mutex. + * + * MT safe. + */ static void gst_object_dispatch_properties_changed (GObject * object, guint n_pspecs, GParamSpec ** pspecs) { GstObject *gst_object, *parent, *old_parent; guint i; - const gchar *name; - - g_static_rec_mutex_lock (&dispatch_mutex); - /* do the standard dispatching */ - G_OBJECT_CLASS (parent_class)->dispatch_properties_changed (object, n_pspecs, - pspecs); + gchar *name, *debug_name; + GstObjectClass *klass; /* we fail when this is not a GstObject */ g_return_if_fail (GST_IS_OBJECT (object)); + klass = GST_OBJECT_GET_CLASS (object); + + GST_CLASS_LOCK (klass); + /* do the standard dispatching */ + PATCH_REFCOUNT (object); + G_OBJECT_CLASS (parent_class)->dispatch_properties_changed (object, n_pspecs, + pspecs); + PATCH_REFCOUNT (object); + gst_object = GST_OBJECT_CAST (object); name = gst_object_get_name (gst_object); + debug_name = GST_STR_NULL (name); /* now let the parent dispatch those, too */ parent = gst_object_get_parent (gst_object); while (parent) { /* for debugging ... */ gchar *parent_name = gst_object_get_name (parent); + gchar *debug_parent_name = GST_STR_NULL (parent_name); /* need own category? */ for (i = 0; i < n_pspecs; i++) { GST_CAT_LOG (GST_CAT_EVENT, "deep notification from %s to %s (%s)", - name ? name : "(null)", - parent_name ? parent_name : "(null)", pspecs[i]->name); + debug_name, debug_parent_name, pspecs[i]->name); - /* FIXME, not MT safe because of glib */ + /* not MT safe because of glib, fixed by taking class lock higher up */ + PATCH_REFCOUNT (parent); + PATCH_REFCOUNT (object); g_signal_emit (parent, gst_object_signals[DEEP_NOTIFY], g_quark_from_string (pspecs[i]->name), GST_OBJECT_CAST (object), pspecs[i]); + PATCH_REFCOUNT (parent); + PATCH_REFCOUNT (object); } g_free (parent_name); @@ -409,7 +505,8 @@ gst_object_dispatch_properties_changed (GObject * object, parent = gst_object_get_parent (old_parent); gst_object_unref (old_parent); } - g_static_rec_mutex_unlock (&dispatch_mutex); + g_free (name); + GST_CLASS_UNLOCK (klass); } /** @@ -425,6 +522,8 @@ gst_object_dispatch_properties_changed (GObject * object, * strings that should be excluded from the notify. * The default handler will print the new value of the property * using g_print. + * + * MT safe. */ void gst_object_default_deep_notify (GObject * object, GstObject * orig, @@ -564,45 +663,48 @@ gst_object_get_name (GstObject * object) * Sets the parent of @object. The object's reference count will be incremented, * and any floating reference will be removed (see gst_object_sink()). * + * This function takes the object lock. + * * Causes the parent-set signal to be emitted. * + * Returns: TRUE if the parent could be set or FALSE when the object + * already had a parent, the object and parent are the same or wrong + * parameters were provided. + * * MT safe. */ -void +gboolean gst_object_set_parent (GstObject * object, GstObject * parent) { - g_return_if_fail (GST_IS_OBJECT (object)); - g_return_if_fail (GST_IS_OBJECT (parent)); - g_return_if_fail (object != parent); + g_return_val_if_fail (GST_IS_OBJECT (object), FALSE); + g_return_val_if_fail (GST_IS_OBJECT (parent), FALSE); + g_return_val_if_fail (object != parent, FALSE); GST_CAT_LOG_OBJECT (GST_CAT_REFCOUNTING, object, "set parent (ref and sink)"); - /* no need to hold locks here as object is either floating and - * owned by this thread only or has a refcount bigger than 1 when - * concurrent access is performed */ - gst_object_ref (object); GST_LOCK (object); - if G_UNLIKELY - (object->parent != NULL) - goto had_parent; + if (G_UNLIKELY (object->parent != NULL)) + goto had_parent; /* sink object, we don't call our own function because we don't - * need to release/acquire the lock needlessly */ - if (GST_OBJECT_IS_FLOATING (object)) { - GST_FLAG_UNSET (object, GST_OBJECT_FLOATING); - gst_object_unref (object); - } + * need to release/acquire the lock needlessly or touch the refcount + * in the floating case. */ object->parent = parent; - GST_UNLOCK (object); + if (G_LIKELY (GST_OBJECT_IS_FLOATING (object))) { + GST_FLAG_UNSET (object, GST_OBJECT_FLOATING); + GST_UNLOCK (object); + } else { + GST_UNLOCK (object); + gst_object_ref (object); + } g_signal_emit (G_OBJECT (object), gst_object_signals[PARENT_SET], 0, parent); - return; + return TRUE; had_parent: GST_UNLOCK (object); - g_critical ("object already had a parent"); - return; + return FALSE; } /** @@ -626,7 +728,7 @@ gst_object_get_parent (GstObject * object) GST_LOCK (object); result = object->parent; - if (result) + if (G_LIKELY (result)) gst_object_ref (result); GST_UNLOCK (object); @@ -653,17 +755,18 @@ gst_object_unparent (GstObject * object) GST_LOCK (object); parent = object->parent; - if G_LIKELY - (parent != NULL) { + if (G_LIKELY (parent != NULL)) { GST_CAT_LOG_OBJECT (GST_CAT_REFCOUNTING, object, "unparent"); object->parent = NULL; - } - GST_UNLOCK (object); + GST_UNLOCK (object); - g_signal_emit (G_OBJECT (object), gst_object_signals[PARENT_UNSET], 0, - parent); + g_signal_emit (G_OBJECT (object), gst_object_signals[PARENT_UNSET], 0, + parent); - gst_object_unref (object); + gst_object_unref (object); + } else { + GST_UNLOCK (object); + } } /** @@ -835,7 +938,8 @@ gst_object_get_path_string (GstObject * object) parent = gst_object_get_parent (object); /* add parents to list, refcount remains increased while * we handle the object */ - parentage = g_slist_prepend (parentage, parent); + if (parent) + parentage = g_slist_prepend (parentage, parent); } else { break; } diff --git a/gst/gstobject.h b/gst/gstobject.h index 616aab53f3..5b04db2901 100644 --- a/gst/gstobject.h +++ b/gst/gstobject.h @@ -29,6 +29,7 @@ #include /* note that this gets wrapped in __GST_OBJECT_H__ */ +#include #include G_BEGIN_DECLS @@ -42,6 +43,7 @@ GST_EXPORT GType _gst_object_type; #define GST_OBJECT(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GST_TYPE_OBJECT, GstObject)) #define GST_OBJECT_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GST_TYPE_OBJECT, GstObjectClass)) #define GST_OBJECT_CAST(obj) ((GstObject*)(obj)) +#define GST_OBJECT_CLASS_CAST(klass) ((GstObjectClass*)(klass)) /* make sure we don't change the object size but stil make it compile * without libxml */ @@ -51,12 +53,16 @@ GST_EXPORT GType _gst_object_type; typedef enum { - GST_OBJECT_DESTROYED = 0, + GST_OBJECT_DISPOSING = 0, + GST_OBJECT_DESTROYED = 1, GST_OBJECT_FLOATING, GST_OBJECT_FLAG_LAST = 4 } GstObjectFlags; +#define GST_OBJECT_REFCOUNT(caps) ((GST_OBJECT_CAST(caps))->refcount) +#define GST_OBJECT_REFCOUNT_VALUE(caps) (gst_atomic_int_read (&(GST_OBJECT_CAST(caps))->refcount)) + /* we do a GST_OBJECT_CAST to avoid type checking, better call these * function with a valid object! */ #define GST_LOCK(obj) (g_mutex_lock(GST_OBJECT_CAST(obj)->lock)) @@ -67,8 +73,9 @@ typedef enum #define GST_OBJECT_NAME(obj) (GST_OBJECT_CAST(obj)->name) #define GST_OBJECT_PARENT(obj) (GST_OBJECT_CAST(obj)->parent) +/* for the flags we double-not to make them comparable to TRUE and FALSE */ #define GST_FLAGS(obj) (GST_OBJECT_CAST (obj)->flags) -#define GST_FLAG_IS_SET(obj,flag) (GST_FLAGS (obj) & (1<<(flag))) +#define GST_FLAG_IS_SET(obj,flag) (!!(GST_FLAGS (obj) & (1<<(flag)))) #define GST_FLAG_SET(obj,flag) G_STMT_START{ (GST_FLAGS (obj) |= (1<<(flag))); }G_STMT_END #define GST_FLAG_UNSET(obj,flag) G_STMT_START{ (GST_FLAGS (obj) &= ~(1<<(flag))); }G_STMT_END @@ -78,16 +85,25 @@ typedef enum struct _GstObject { GObject object; + /*< public >*/ + GstAtomicInt refcount; + /*< public >*/ /* with LOCK */ GMutex *lock; /* locking for all sorts of things */ gchar *name; /* name */ - GstObject *parent; /* this object's parent */ + GstObject *parent; /* this object's parent, no refcount is held for the parent. */ guint32 flags; /*< private >*/ gpointer _gst_reserved[GST_PADDING]; }; + +#define GST_CLASS_LOCK(obj) (g_mutex_lock(GST_OBJECT_CLASS_CAST(obj)->lock)) +#define GST_CLASS_TRYLOCK(obj) (g_mutex_trylock(GST_OBJECT_CLASS_CAST(obj)->lock)) +#define GST_CLASS_UNLOCK(obj) (g_mutex_unlock(GST_OBJECT_CLASS_CAST(obj)->lock)) +#define GST_CLASS_GET_LOCK(obj) (GST_OBJECT_CLASS_CAST(obj)->lock) + /* signal_object is used to signal to the whole class */ struct _GstObjectClass { GObjectClass parent_class; @@ -95,6 +111,8 @@ struct _GstObjectClass { gchar *path_string_separator; GObject *signal_object; + GMutex *lock; + /* signals */ void (*parent_set) (GstObject *object, GstObject *parent); void (*parent_unset) (GstObject *object, GstObject *parent); @@ -111,9 +129,6 @@ struct _GstObjectClass { gpointer _gst_reserved[GST_PADDING]; }; - - - /* normal GObject stuff */ GType gst_object_get_type (void); @@ -122,7 +137,7 @@ void gst_object_set_name (GstObject *object, const gchar *name); gchar* gst_object_get_name (GstObject *object); /* parentage routines */ -void gst_object_set_parent (GstObject *object, GstObject *parent); +gboolean gst_object_set_parent (GstObject *object, GstObject *parent); GstObject* gst_object_get_parent (GstObject *object); void gst_object_unparent (GstObject *object); @@ -131,7 +146,7 @@ void gst_object_default_deep_notify (GObject *object, GstObject *ori /* refcounting + life cycle */ GstObject * gst_object_ref (GstObject *object); -void gst_object_unref (GstObject *object); +GstObject * gst_object_unref (GstObject *object); void gst_object_sink (GstObject *object); /* replace object pointer */ diff --git a/gst/gstpad.c b/gst/gstpad.c index 7d03428a1c..683cdffb4a 100644 --- a/gst/gstpad.c +++ b/gst/gstpad.c @@ -47,6 +47,18 @@ GST_DEBUG_CATEGORY_STATIC (debug_dataflow); }G_STMT_END #define GST_CAT_DEFAULT GST_CAT_PADS +#define GST_PAD_REALIZE_AND_LOCK(pad, realpad, lost_ghostpad) \ + GST_LOCK (pad); \ + realpad = GST_PAD_REALIZE (pad); \ + if (G_UNLIKELY (realpad == NULL)) { \ + GST_UNLOCK (pad); \ + goto lost_ghostpad; \ + } \ + if (G_UNLIKELY (pad != GST_PAD_CAST (realpad))) { \ + GST_LOCK (realpad); \ + GST_UNLOCK (pad); \ + } + enum { TEMPL_PAD_CREATED, @@ -116,6 +128,8 @@ gst_pad_dispose (GObject * object) GstPad *pad = GST_PAD (object); gst_pad_set_pad_template (pad, NULL); + /* FIXME, we have links to many other things like caps + * and the peer pad... */ G_OBJECT_CLASS (pad_parent_class)->dispose (object); } @@ -425,30 +439,29 @@ gst_pad_set_active (GstPad * pad, gboolean active) g_return_val_if_fail (GST_IS_PAD (pad), FALSE); - GST_LOCK (pad); - old = GST_PAD_IS_ACTIVE (pad); + GST_PAD_REALIZE_AND_LOCK (pad, realpad, lost_ghostpad); + + old = GST_PAD_IS_ACTIVE (realpad); if (G_UNLIKELY (old == active)) goto exit; - realpad = GST_PAD_REALIZE (pad); - /* make sure data is disallowed when going inactive */ if (!active) { GST_CAT_DEBUG (GST_CAT_PADS, "de-activating pad %s:%s", GST_DEBUG_PAD_NAME (realpad)); GST_FLAG_SET (realpad, GST_PAD_DISABLED); /* unlock blocked pads so element can resume and stop */ - GST_PAD_BLOCK_SIGNAL (pad); + GST_PAD_BLOCK_SIGNAL (realpad); } activatefunc = realpad->activatefunc; if (activatefunc) { /* unlock so element can sync */ - GST_UNLOCK (pad); - result = activatefunc (pad, active); + GST_UNLOCK (realpad); + result = activatefunc (GST_PAD_CAST (realpad), active); /* and lock again */ - GST_LOCK (pad); + GST_LOCK (realpad); } else { result = TRUE; } @@ -461,9 +474,14 @@ gst_pad_set_active (GstPad * pad, gboolean active) } exit: - GST_UNLOCK (pad); + GST_UNLOCK (realpad); return result; + +lost_ghostpad: + { + return FALSE; + } } /** @@ -480,14 +498,20 @@ gboolean gst_pad_is_active (GstPad * pad) { gboolean result = FALSE; + GstRealPad *realpad; g_return_val_if_fail (GST_IS_PAD (pad), FALSE); - GST_LOCK (pad); - result = !GST_FLAG_IS_SET (pad, GST_PAD_DISABLED); - GST_UNLOCK (pad); + GST_PAD_REALIZE_AND_LOCK (pad, realpad, lost_ghostpad); + result = !GST_FLAG_IS_SET (realpad, GST_PAD_DISABLED); + GST_UNLOCK (realpad); return result; + +lost_ghostpad: + { + return FALSE; + } } /** @@ -520,36 +544,30 @@ gst_pad_set_blocked_async (GstPad * pad, gboolean blocked, gboolean was_blocked; GstRealPad *realpad; - g_return_val_if_fail (GST_IS_REAL_PAD (pad), FALSE); + g_return_val_if_fail (GST_IS_PAD (pad), FALSE); - GST_LOCK (pad); - realpad = GST_PAD_REALIZE (pad); - if (GST_PAD_CAST (realpad) != pad) { - GST_LOCK (realpad); - GST_UNLOCK (pad); - } + GST_PAD_REALIZE_AND_LOCK (pad, realpad, lost_ghostpad); - /* beware for turning flags into booleans */ - was_blocked = !!GST_RPAD_IS_BLOCKED (realpad); + was_blocked = GST_RPAD_IS_BLOCKED (realpad); if (G_UNLIKELY (was_blocked == blocked)) goto had_right_state; if (blocked) { - GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "blocking pad %s:%s", - GST_DEBUG_PAD_NAME (pad)); + GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, realpad, "blocking pad %s:%s", + GST_DEBUG_PAD_NAME (realpad)); GST_FLAG_SET (realpad, GST_PAD_BLOCKED); realpad->block_callback = callback; realpad->block_data = user_data; if (!callback) { - GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "waiting for block"); - GST_PAD_BLOCK_WAIT (pad); - GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "blocked"); + GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, realpad, "waiting for block"); + GST_PAD_BLOCK_WAIT (realpad); + GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, realpad, "blocked"); } } else { - GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "unblocking pad %s:%s", - GST_DEBUG_PAD_NAME (pad)); + GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, realpad, "unblocking pad %s:%s", + GST_DEBUG_PAD_NAME (realpad)); GST_FLAG_UNSET (realpad, GST_PAD_BLOCKED); @@ -557,23 +575,29 @@ gst_pad_set_blocked_async (GstPad * pad, gboolean blocked, realpad->block_data = user_data; if (callback) { - GST_PAD_BLOCK_SIGNAL (pad); + GST_PAD_BLOCK_SIGNAL (realpad); } else { - GST_PAD_BLOCK_SIGNAL (pad); - GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "waiting for unblock"); - GST_PAD_BLOCK_WAIT (pad); - GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "unblocked"); + GST_PAD_BLOCK_SIGNAL (realpad); + GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, realpad, "waiting for unblock"); + GST_PAD_BLOCK_WAIT (realpad); + GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, realpad, "unblocked"); } } - GST_UNLOCK (pad); + GST_UNLOCK (realpad); return TRUE; +lost_ghostpad: + { + return FALSE; + } had_right_state: - GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "pad %s:%s was in right state", - GST_DEBUG_PAD_NAME (pad)); - GST_UNLOCK (pad); - return FALSE; + { + GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, realpad, + "pad %s:%s was in right state", GST_DEBUG_PAD_NAME (realpad)); + GST_UNLOCK (realpad); + return FALSE; + } } /** @@ -613,14 +637,20 @@ gboolean gst_pad_is_blocked (GstPad * pad) { gboolean result = FALSE; + GstRealPad *realpad; g_return_val_if_fail (GST_IS_PAD (pad), result); - GST_LOCK (pad); - result = GST_FLAG_IS_SET (pad, GST_PAD_BLOCKED); - GST_UNLOCK (pad); + GST_PAD_REALIZE_AND_LOCK (pad, realpad, lost_ghostpad); + result = GST_FLAG_IS_SET (realpad, GST_PAD_BLOCKED); + GST_UNLOCK (realpad); return result; + +lost_ghostpad: + { + return FALSE; + } } /** @@ -1042,26 +1072,13 @@ gst_pad_unlink (GstPad * srcpad, GstPad * sinkpad) GST_DEBUG_PAD_NAME (srcpad), srcpad, GST_DEBUG_PAD_NAME (sinkpad), sinkpad); - GST_LOCK (srcpad); - realsrc = GST_PAD_REALIZE (srcpad); - if (srcpad != GST_PAD_CAST (realsrc)) { - GST_LOCK (realsrc); - /* we don't care if the ghostpad goes away now */ - GST_UNLOCK (srcpad); - } + GST_PAD_REALIZE_AND_LOCK (srcpad, realsrc, lost_src_ghostpad); + if (G_UNLIKELY (GST_RPAD_DIRECTION (realsrc) != GST_PAD_SRC)) goto not_srcpad; - if (G_UNLIKELY (GST_RPAD_PEER (realsrc) == NULL)) - goto was_unlinked; + GST_PAD_REALIZE_AND_LOCK (sinkpad, realsink, lost_sink_ghostpad); - GST_LOCK (sinkpad); - realsink = GST_PAD_REALIZE (sinkpad); - if (sinkpad != GST_PAD_CAST (realsink)) { - GST_LOCK (realsink); - /* we don't care if the ghostpad goes away now */ - GST_UNLOCK (sinkpad); - } if (G_UNLIKELY (GST_RPAD_DIRECTION (realsink) != GST_PAD_SINK)) goto not_sinkpad; @@ -1083,8 +1100,8 @@ gst_pad_unlink (GstPad * srcpad, GstPad * sinkpad) gst_caps_replace (&GST_RPAD_APPFILTER (realsrc), NULL); gst_caps_replace (&GST_RPAD_APPFILTER (realsink), NULL); - GST_UNLOCK (realsrc); GST_UNLOCK (realsink); + GST_UNLOCK (realsrc); /* fire off a signal to each of the pads telling them * that they've been unlinked */ @@ -1094,27 +1111,29 @@ gst_pad_unlink (GstPad * srcpad, GstPad * sinkpad) 0, realsrc); GST_CAT_INFO (GST_CAT_ELEMENT_PADS, "unlinked %s:%s and %s:%s", - GST_DEBUG_PAD_NAME (srcpad), GST_DEBUG_PAD_NAME (sinkpad)); + GST_DEBUG_PAD_NAME (realsrc), GST_DEBUG_PAD_NAME (realsink)); return TRUE; +lost_src_ghostpad: + { + return FALSE; + } not_srcpad: { g_critical ("pad %s is not a source pad", GST_PAD_NAME (realsrc)); GST_UNLOCK (realsrc); return FALSE; } +lost_sink_ghostpad: + { + GST_UNLOCK (realsrc); + return FALSE; + } not_sinkpad: { g_critical ("pad %s is not a sink pad", GST_PAD_NAME (realsink)); - GST_UNLOCK (realsrc); GST_UNLOCK (realsink); - return FALSE; - } -was_unlinked: - { - /* we do not emit a warning in this case because unlinking cannot - * be made MT safe.*/ GST_UNLOCK (realsrc); return FALSE; } @@ -1122,8 +1141,8 @@ not_linked_together: { /* we do not emit a warning in this case because unlinking cannot * be made MT safe.*/ - GST_UNLOCK (realsrc); GST_UNLOCK (realsink); + GST_UNLOCK (realsrc); return FALSE; } } @@ -1142,14 +1161,116 @@ gboolean gst_pad_is_linked (GstPad * pad) { gboolean result; + GstRealPad *realpad; g_return_val_if_fail (GST_IS_PAD (pad), FALSE); - GST_LOCK (pad); - result = (GST_PAD_PEER (pad) != NULL); - GST_UNLOCK (pad); - + GST_PAD_REALIZE_AND_LOCK (pad, realpad, lost_ghostpad); + result = (GST_PAD_PEER (realpad) != NULL); + GST_UNLOCK (realpad); return result; + +lost_ghostpad: + { + return FALSE; + } +} + +static GstPadLinkReturn +gst_pad_link_prepare_filtered (GstPad * srcpad, GstPad * sinkpad, + GstRealPad ** outrealsrc, GstRealPad ** outrealsink, + const GstCaps * filtercaps) +{ + GstRealPad *realsrc, *realsink; + + /* generic checks */ + g_return_val_if_fail (GST_IS_PAD (srcpad), GST_PAD_LINK_REFUSED); + g_return_val_if_fail (GST_IS_PAD (sinkpad), GST_PAD_LINK_REFUSED); + + GST_CAT_INFO (GST_CAT_PADS, "trying to link %s:%s and %s:%s", + GST_DEBUG_PAD_NAME (srcpad), GST_DEBUG_PAD_NAME (sinkpad)); + + /* now we need to deal with the real/ghost stuff */ + GST_PAD_REALIZE_AND_LOCK (srcpad, realsrc, lost_src_ghostpad); + + if (G_UNLIKELY (GST_RPAD_DIRECTION (realsrc) != GST_PAD_SRC)) + goto not_srcpad; + + if (G_UNLIKELY (GST_RPAD_PEER (realsrc) != NULL)) + goto src_was_linked; + + GST_PAD_REALIZE_AND_LOCK (sinkpad, realsink, lost_sink_ghostpad); + + if (G_UNLIKELY (GST_RPAD_DIRECTION (realsink) != GST_PAD_SINK)) + goto not_sinkpad; + + if (G_UNLIKELY (GST_RPAD_PEER (realsink) != NULL)) + goto sink_was_linked; + + if ((GST_PAD (realsrc) != srcpad) || (GST_PAD (realsink) != sinkpad)) { + GST_CAT_INFO (GST_CAT_PADS, "*actually* linking %s:%s and %s:%s", + GST_DEBUG_PAD_NAME (realsrc), GST_DEBUG_PAD_NAME (realsink)); + } + *outrealsrc = realsrc; + *outrealsink = realsink; + + /* FIXME check pad caps for non-empty intersection */ + + /* FIXME check pad scheduling for non-empty intersection */ + + /* update filter */ + if (filtercaps) { + GstCaps *filtercopy; + + filtercopy = gst_caps_copy (filtercaps); + filtercopy = gst_caps_ref (filtercopy); + + gst_caps_replace (&GST_PAD_APPFILTER (realsrc), filtercopy); + gst_caps_replace (&GST_PAD_APPFILTER (realsink), filtercopy); + } else { + gst_caps_replace (&GST_PAD_APPFILTER (realsrc), NULL); + gst_caps_replace (&GST_PAD_APPFILTER (realsink), NULL); + } + return GST_PAD_LINK_OK; + +lost_src_ghostpad: + { + return GST_PAD_LINK_REFUSED; + } +not_srcpad: + { + g_critical ("pad %s is not a source pad", GST_PAD_NAME (realsrc)); + GST_UNLOCK (realsrc); + return GST_PAD_LINK_WRONG_DIRECTION; + } +src_was_linked: + { + /* we do not emit a warning in this case because unlinking cannot + * be made MT safe.*/ + GST_UNLOCK (realsrc); + return GST_PAD_LINK_WAS_LINKED; + } +lost_sink_ghostpad: + { + GST_DEBUG ("lost sink ghostpad"); + GST_UNLOCK (realsrc); + return GST_PAD_LINK_REFUSED; + } +not_sinkpad: + { + g_critical ("pad %s is not a sink pad", GST_PAD_NAME (realsink)); + GST_UNLOCK (realsink); + GST_UNLOCK (realsrc); + return GST_PAD_LINK_WRONG_DIRECTION; + } +sink_was_linked: + { + /* we do not emit a warning in this case because unlinking cannot + * be made MT safe.*/ + GST_UNLOCK (realsink); + GST_UNLOCK (realsrc); + return GST_PAD_LINK_WAS_LINKED; + } } /** @@ -1166,6 +1287,8 @@ gst_pad_is_linked (GstPad * pad) * * Returns: A result code indicating if the connection worked or * what went wrong. + * + * MT Safe. */ GstPadLinkReturn gst_pad_link_filtered (GstPad * srcpad, GstPad * sinkpad, @@ -1174,72 +1297,17 @@ gst_pad_link_filtered (GstPad * srcpad, GstPad * sinkpad, GstRealPad *realsrc, *realsink; GstPadLinkReturn result; - /* generic checks */ - g_return_val_if_fail (GST_IS_PAD (srcpad), GST_PAD_LINK_REFUSED); - g_return_val_if_fail (GST_IS_PAD (sinkpad), GST_PAD_LINK_REFUSED); + result = gst_pad_link_prepare_filtered (srcpad, sinkpad, &realsrc, &realsink, + filtercaps); - GST_CAT_INFO (GST_CAT_PADS, "trying to link %s:%s and %s:%s", - GST_DEBUG_PAD_NAME (srcpad), GST_DEBUG_PAD_NAME (sinkpad)); + if (result != GST_PAD_LINK_OK) + goto prepare_failed; - /* now we need to deal with the real/ghost stuff */ - realsrc = GST_PAD_REALIZE (srcpad); - realsink = GST_PAD_REALIZE (sinkpad); - - if ((GST_PAD (realsrc) != srcpad) || (GST_PAD (realsink) != sinkpad)) { - GST_CAT_INFO (GST_CAT_PADS, "*actually* linking %s:%s and %s:%s", - GST_DEBUG_PAD_NAME (realsrc), GST_DEBUG_PAD_NAME (realsink)); - } - - g_return_val_if_fail (realsrc != NULL, GST_PAD_LINK_REFUSED); - g_return_val_if_fail (realsink != NULL, GST_PAD_LINK_REFUSED); - - GST_LOCK (realsrc); - GST_LOCK (realsink); - - /* FIXME: shouldn't we convert this to g_return_val_if_fail? */ - if (GST_RPAD_PEER (realsrc) != NULL) { - GST_CAT_INFO (GST_CAT_PADS, "Real source pad %s:%s has a peer, failed", - GST_DEBUG_PAD_NAME (realsrc)); - result = GST_PAD_LINK_REFUSED; - goto error; - } - if (GST_RPAD_PEER (realsink) != NULL) { - GST_CAT_INFO (GST_CAT_PADS, "Real sink pad %s:%s has a peer, failed", - GST_DEBUG_PAD_NAME (realsink)); - result = GST_PAD_LINK_REFUSED; - goto error; - } - - if (GST_RPAD_DIRECTION (realsrc) != GST_PAD_SRC) { - GST_CAT_INFO (GST_CAT_PADS, - "Real src pad %s:%s is not a source pad, failed", - GST_DEBUG_PAD_NAME (realsrc)); - result = GST_PAD_LINK_REFUSED; - goto error; - } - if (GST_RPAD_DIRECTION (realsink) != GST_PAD_SINK) { - GST_CAT_INFO (GST_CAT_PADS, "Real sink pad %s:%s is not a sink pad, failed", - GST_DEBUG_PAD_NAME (realsink)); - result = GST_PAD_LINK_REFUSED; - goto error; - } - /* FIXME check pad caps for non-empty intersection */ - - /* update filter before calling the link functions */ - if (filtercaps) { - GstCaps *filtercopy = gst_caps_copy (filtercaps); - - filtercopy = gst_caps_ref (filtercopy); - - GST_RPAD_APPFILTER (realsrc) = filtercopy; - GST_RPAD_APPFILTER (realsink) = filtercopy; - } else { - GST_RPAD_APPFILTER (realsrc) = NULL; - GST_RPAD_APPFILTER (realsink) = NULL; - } - GST_UNLOCK (realsrc); GST_UNLOCK (realsink); + GST_UNLOCK (realsrc); + /* FIXME released the locks here, concurrent thread might link + * something else. */ if (GST_RPAD_LINKFUNC (realsrc)) { /* this one will call the peer link function */ result = @@ -1259,8 +1327,8 @@ gst_pad_link_filtered (GstPad * srcpad, GstPad * sinkpad, GST_RPAD_PEER (realsrc) = GST_REAL_PAD (realsink); GST_RPAD_PEER (realsink) = GST_REAL_PAD (realsrc); - GST_UNLOCK (realsrc); GST_UNLOCK (realsink); + GST_UNLOCK (realsrc); /* fire off a signal to each of the pads telling them * that they've been linked */ @@ -1281,17 +1349,15 @@ gst_pad_link_filtered (GstPad * srcpad, GstPad * sinkpad, gst_caps_replace (&GST_RPAD_APPFILTER (realsink), NULL); } - GST_UNLOCK (realsrc); GST_UNLOCK (realsink); + GST_UNLOCK (realsrc); } - return result; -error: - GST_UNLOCK (realsrc); - GST_UNLOCK (realsink); - - return result; +prepare_failed: + { + return result; + } } /** @@ -1363,17 +1429,18 @@ gst_pad_get_real_parent (GstPad * pad) g_return_val_if_fail (GST_IS_PAD (pad), NULL); - GST_LOCK (pad); - realpad = GST_PAD_REALIZE (pad); - if (pad != GST_PAD_CAST (realpad)) { - GST_LOCK (realpad); - GST_UNLOCK (pad); - } + GST_PAD_REALIZE_AND_LOCK (pad, realpad, lost_ghostpad); element = GST_PAD_PARENT (realpad); - gst_object_ref (GST_OBJECT (element)); + if (element) + gst_object_ref (GST_OBJECT (element)); GST_UNLOCK (realpad); return element; + +lost_ghostpad: + { + return NULL; + } } /* FIXME not MT safe */ @@ -1425,24 +1492,25 @@ GstPadLinkReturn gst_pad_relink_filtered (GstPad * srcpad, GstPad * sinkpad, const GstCaps * filtercaps) { - GstCaps *filtercopy; + GstRealPad *realsrc, *realsink; + GstPadLinkReturn result; - /* create copy with two refs */ - filtercopy = gst_caps_copy (filtercaps); - filtercopy = gst_caps_ref (filtercopy); + result = gst_pad_link_prepare_filtered (srcpad, sinkpad, &realsrc, &realsink, + filtercaps); - /* probably check if intersection with new filter is ok */ - GST_LOCK (srcpad); - GST_LOCK (sinkpad); - gst_caps_replace (&GST_PAD_CAPS (srcpad), NULL); - gst_caps_replace (&GST_PAD_APPFILTER (srcpad), filtercopy); + if (result != GST_PAD_LINK_OK) + goto prepare_failed; - gst_caps_replace (&GST_PAD_CAPS (sinkpad), NULL); - gst_caps_replace (&GST_PAD_APPFILTER (sinkpad), filtercopy); - GST_UNLOCK (sinkpad); - GST_UNLOCK (srcpad); + /* clear caps to force renegotiation */ + gst_caps_replace (&GST_PAD_CAPS (realsrc), NULL); + gst_caps_replace (&GST_PAD_CAPS (realsink), NULL); + GST_UNLOCK (realsink); + GST_UNLOCK (realsrc); return GST_PAD_LINK_OK; + +prepare_failed: + return result; } /** @@ -1619,15 +1687,60 @@ gst_pad_get_peer (GstPad * pad) g_return_val_if_fail (GST_IS_PAD (pad), NULL); + GST_PAD_REALIZE_AND_LOCK (pad, realpad, lost_ghostpad); + result = GST_RPAD_PEER (realpad); + if (result) + gst_object_ref (GST_OBJECT (result)); + GST_UNLOCK (realpad); + + return GST_PAD_CAST (result); + +lost_ghostpad: + { + return NULL; + } +} + +/** + * gst_pad_realize: + * @pad: a #GstPad to realize + * + * If the pad is a #GstRealPad, it is simply returned, else + * the #GstGhostPad will be dereffed to the real pad. + * + * After this function you always receive the real pad of + * the provided pad. + * + * This function unrefs the input pad and refs the result so + * that you can write constructs like: + * + * pad = gst_pad_realize(pad) + * + * without having to unref the old pad. + * + * Returns: the real #GstPad or NULL when an old reference to a + * ghostpad is used. + * + * MT safe. + */ +GstPad * +gst_pad_realize (GstPad * pad) +{ + GstRealPad *result; + + g_return_val_if_fail (GST_IS_PAD (pad), NULL); + GST_LOCK (pad); - realpad = GST_PAD_REALIZE (pad); - if (pad != GST_PAD_CAST (realpad)) { - GST_LOCK (realpad); + result = GST_PAD_REALIZE (pad); + if (result && pad != GST_PAD_CAST (result)) { + gst_object_ref (GST_OBJECT (result)); + GST_UNLOCK (pad); + /* no other thread could dispose this since we + * hold at least one ref */ + gst_object_unref (GST_OBJECT (pad)); + } else { GST_UNLOCK (pad); } - result = GST_RPAD_PEER (realpad); - gst_object_ref (GST_OBJECT (result)); - GST_UNLOCK (realpad); return GST_PAD_CAST (result); } @@ -1641,13 +1754,16 @@ gst_pad_get_peer (GstPad * pad) * * Returns: the allowed #GstCaps of the pad link. Free the caps when * you no longer need it. + * + * MT safe. */ GstCaps * gst_pad_get_allowed_caps (GstPad * pad) { - const GstCaps *mycaps; + GstCaps *mycaps; GstCaps *caps; GstCaps *peercaps; + GstRealPad *peer; g_return_val_if_fail (GST_IS_REAL_PAD (pad), NULL); @@ -1655,15 +1771,25 @@ gst_pad_get_allowed_caps (GstPad * pad) GST_DEBUG_PAD_NAME (pad)); mycaps = gst_pad_get_caps (pad); - if (GST_RPAD_PEER (pad) == NULL) { + GST_LOCK (pad); + peer = GST_RPAD_PEER (pad); + if (peer == NULL) { GST_CAT_DEBUG (GST_CAT_PROPERTIES, "%s:%s: no peer, returning own caps", GST_DEBUG_PAD_NAME (pad)); - return gst_caps_copy (mycaps); + GST_UNLOCK (pad); + + return mycaps; + } else { + gst_object_ref (GST_OBJECT (peer)); + GST_UNLOCK (pad); } - peercaps = gst_pad_get_caps (GST_PAD_PEER (pad)); + peercaps = gst_pad_get_caps (GST_PAD_CAST (peer)); + gst_object_unref (GST_OBJECT (peer)); + caps = gst_caps_intersect (mycaps, peercaps); gst_caps_unref (peercaps); + gst_caps_unref (mycaps); GST_CAT_DEBUG (GST_CAT_CAPS, "allowed caps %" GST_PTR_FORMAT, caps); @@ -1705,32 +1831,36 @@ gst_pad_alloc_buffer (GstPad * pad, guint64 offset, gint size) if (G_LIKELY ((bufferallocfunc = peer->bufferallocfunc) == NULL)) goto fallback; + GST_UNLOCK (pad); + GST_CAT_DEBUG (GST_CAT_PADS, "calling bufferallocfunc &%s (@%p) of peer pad %s:%s", GST_DEBUG_FUNCPTR_NAME (bufferallocfunc), &bufferallocfunc, GST_DEBUG_PAD_NAME (peer)); - GST_UNLOCK (pad); - result = bufferallocfunc (GST_PAD (peer), offset, size, GST_PAD_CAPS (pad)); + result = + bufferallocfunc (GST_PAD_CAST (peer), offset, size, GST_PAD_CAPS (pad)); - if (result == NULL) { + if (G_UNLIKELY (result == NULL)) { GST_LOCK (pad); goto fallback; } return result; - /* fallback case */ + /* fallback case, allocate a buffer of our own, add pad caps. */ fallback: - caps = GST_PAD_CAPS (pad); - gst_caps_ref (caps); - GST_UNLOCK (pad); + { + caps = GST_PAD_CAPS (pad); + gst_caps_ref (caps); + GST_UNLOCK (pad); - result = gst_buffer_new_and_alloc (size); - gst_buffer_set_caps (result, caps); - gst_caps_unref (caps); + result = gst_buffer_new_and_alloc (size); + gst_buffer_set_caps (result, caps); + gst_caps_unref (caps); - return result; + return result; + } } static void @@ -1991,7 +2121,8 @@ gst_pad_push (GstPad * pad, GstBuffer * buffer) GstPadChainFunction chainfunc; g_return_val_if_fail (GST_IS_REAL_PAD (pad), GST_FLOW_ERROR); - g_return_val_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SRC, GST_FLOW_ERROR); + g_return_val_if_fail (GST_RPAD_DIRECTION (pad) == GST_PAD_SRC, + GST_FLOW_ERROR); g_return_val_if_fail (buffer != NULL, GST_FLOW_ERROR); g_return_val_if_fail (GST_IS_BUFFER (buffer), GST_FLOW_ERROR); @@ -2006,19 +2137,16 @@ gst_pad_push (GstPad * pad, GstBuffer * buffer) if (G_UNLIKELY (!GST_RPAD_IS_ACTIVE (peer))) goto not_active; - gst_object_ref (GST_OBJECT (peer)); + gst_object_ref (GST_OBJECT_CAST (peer)); GST_UNLOCK (pad); - GST_LOCK (peer); - if (G_UNLIKELY ((chainfunc = peer->chainfunc) == NULL)) - goto no_function; - - /* NOTE: after this unlock the peer could change chainfunction, + /* NOTE: we read the peer chainfunc unlocked. * we cannot hold the lock for the peer so we might send * the data to the wrong function. This is not really a * problem since functions are assigned at creation time * and don't change that often... */ - GST_UNLOCK (peer); + if (G_UNLIKELY ((chainfunc = peer->chainfunc) == NULL)) + goto no_function; GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "calling chainfunction &%s of peer pad %s:%s", @@ -2026,7 +2154,7 @@ gst_pad_push (GstPad * pad, GstBuffer * buffer) ret = chainfunc (GST_PAD_CAST (peer), buffer); - gst_object_unref (GST_OBJECT (peer)); + gst_object_unref (GST_OBJECT_CAST (peer)); return ret; @@ -2044,7 +2172,8 @@ no_function: GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "pushing, but not chainhandler"); g_warning ("push on pad %s:%s but it has no chainhandler, file a bug.", GST_DEBUG_PAD_NAME (peer)); - GST_UNLOCK_RETURN (peer, GST_FLOW_ERROR); + gst_object_unref (GST_OBJECT (peer)); + return GST_FLOW_ERROR; } /** @@ -2067,7 +2196,7 @@ gst_pad_pull (GstPad * pad, GstBuffer ** buffer) GstPadGetFunction getfunc; g_return_val_if_fail (GST_IS_REAL_PAD (pad), GST_FLOW_ERROR); - g_return_val_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SINK, + g_return_val_if_fail (GST_RPAD_DIRECTION (pad) == GST_PAD_SINK, GST_FLOW_ERROR); g_return_val_if_fail (buffer != NULL, GST_FLOW_ERROR); @@ -2079,23 +2208,20 @@ gst_pad_pull (GstPad * pad, GstBuffer ** buffer) if (G_UNLIKELY ((peer = GST_RPAD_PEER (pad)) == NULL)) goto not_connected; - gst_object_ref (GST_OBJECT (peer)); + gst_object_ref (GST_OBJECT_CAST (peer)); GST_UNLOCK (pad); - GST_LOCK (peer); + /* see note in above function */ if (G_UNLIKELY ((getfunc = peer->getfunc) == NULL)) goto no_function; - /* see note in above function */ - GST_UNLOCK (peer); - GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "calling getfunc %s of peer pad %s:%s", GST_DEBUG_FUNCPTR_NAME (getfunc), GST_DEBUG_PAD_NAME (peer)); ret = getfunc (GST_PAD_CAST (peer), buffer); - gst_object_unref (GST_OBJECT (peer)); + gst_object_unref (GST_OBJECT_CAST (peer)); return ret; @@ -2109,7 +2235,8 @@ no_function: GST_ELEMENT_ERROR (GST_PAD_PARENT (pad), CORE, PAD, (NULL), ("pull on pad %s:%s but the peer pad %s:%s has no getfunc", GST_DEBUG_PAD_NAME (pad), GST_DEBUG_PAD_NAME (peer))); - GST_UNLOCK_RETURN (peer, GST_FLOW_ERROR); + gst_object_unref (GST_OBJECT (peer)); + return GST_FLOW_ERROR; } /** @@ -2135,7 +2262,7 @@ gst_pad_pull_range (GstPad * pad, guint64 offset, guint size, GstPadGetRangeFunction getrangefunc; g_return_val_if_fail (GST_IS_REAL_PAD (pad), GST_FLOW_ERROR); - g_return_val_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SINK, + g_return_val_if_fail (GST_RPAD_DIRECTION (pad) == GST_PAD_SINK, GST_FLOW_ERROR); g_return_val_if_fail (buffer != NULL, GST_FLOW_ERROR); @@ -2147,23 +2274,20 @@ gst_pad_pull_range (GstPad * pad, guint64 offset, guint size, if (G_UNLIKELY ((peer = GST_RPAD_PEER (pad)) == NULL)) goto not_connected; - gst_object_ref (GST_OBJECT (peer)); + gst_object_ref (GST_OBJECT_CAST (peer)); GST_UNLOCK (pad); - GST_LOCK (peer); + /* see note in above function */ if (G_UNLIKELY ((getrangefunc = peer->getrangefunc) == NULL)) goto no_function; - /* see note in above function */ - GST_UNLOCK (peer); - GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad, "calling getrangefunc %s of peer pad %s:%s", GST_DEBUG_FUNCPTR_NAME (getrangefunc), GST_DEBUG_PAD_NAME (peer)); ret = getrangefunc (GST_PAD_CAST (peer), offset, size, buffer); - gst_object_unref (GST_OBJECT (peer)); + gst_object_unref (GST_OBJECT_CAST (peer)); return ret; @@ -2177,7 +2301,8 @@ no_function: GST_ELEMENT_ERROR (GST_PAD_PARENT (pad), CORE, PAD, (NULL), ("pullrange on pad %s:%s but the peer pad %s:%s has no getrangefunction", GST_DEBUG_PAD_NAME (pad), GST_DEBUG_PAD_NAME (peer))); - GST_UNLOCK_RETURN (peer, GST_FLOW_ERROR); + gst_object_unref (GST_OBJECT (peer)); + return GST_FLOW_ERROR; } /************************************************************************ diff --git a/gst/gstpad.h b/gst/gstpad.h index f88df90c00..9327a4f22c 100644 --- a/gst/gstpad.h +++ b/gst/gstpad.h @@ -87,10 +87,12 @@ typedef struct _GstStaticPadTemplate GstStaticPadTemplate; typedef struct _GstPadLink GstPadLink; typedef enum { - GST_PAD_LINK_NOSCHED = -3, /* pads cannot cooperate in scheduling */ - GST_PAD_LINK_NOFORMAT = -2, /* pads do not have common format */ - GST_PAD_LINK_REFUSED = -1, /* refused for some reason */ - GST_PAD_LINK_OK = 0, /* link ok */ + GST_PAD_LINK_NOSCHED = -5, /* pads cannot cooperate in scheduling */ + GST_PAD_LINK_NOFORMAT = -4, /* pads do not have common format */ + GST_PAD_LINK_REFUSED = -3, /* refused for some reason */ + GST_PAD_LINK_WRONG_DIRECTION = -2, /* pads have wrong direction */ + GST_PAD_LINK_WAS_LINKED = -1, /* pad was already linked */ + GST_PAD_LINK_OK = 0, /* link ok */ } GstPadLinkReturn; #define GST_PAD_LINK_FAILED(ret) (ret < GST_PAD_LINK_OK) @@ -223,6 +225,7 @@ struct _GstRealPad { GstPadEventMaskFunction eventmaskfunc; GList *ghostpads; + guint32 ghostpads_cookie; /* query/convert/formats functions */ GstPadConvertFunction convertfunc; @@ -458,6 +461,7 @@ gboolean gst_pad_unlink (GstPad *srcpad, GstPad *sinkpad); gboolean gst_pad_is_linked (GstPad *pad); GstPad* gst_pad_get_peer (GstPad *pad); +GstPad* gst_pad_realize (GstPad *pad); /* capsnego functions */ void gst_pad_set_getcaps_function (GstPad *pad, GstPadGetCapsFunction getcaps); diff --git a/gst/gstpipeline.c b/gst/gstpipeline.c index 73abd8ffa3..97652733b7 100644 --- a/gst/gstpipeline.c +++ b/gst/gstpipeline.c @@ -133,6 +133,7 @@ gst_pipeline_init (GTypeInstance * instance, gpointer g_class) gst_bus_set_sync_handler (pipeline->bus, (GstBusSyncHandler) pipeline_bus_handler, pipeline); pipeline->eosed = NULL; + /* we are our own manager */ GST_ELEMENT_MANAGER (pipeline) = pipeline; } @@ -144,6 +145,10 @@ gst_pipeline_dispose (GObject * object) g_assert (GST_IS_SCHEDULER (pipeline->scheduler)); gst_scheduler_reset (pipeline->scheduler); + gst_object_replace ((GstObject **) & pipeline->bus, NULL); + gst_object_replace ((GstObject **) & pipeline->scheduler, NULL); + gst_object_replace ((GstObject **) & pipeline->fixed_clock, NULL); + G_OBJECT_CLASS (parent_class)->dispose (object); } @@ -165,26 +170,27 @@ is_eos (GstPipeline * pipeline) GList *eosed; GstElementState state, pending; gboolean complete; + gchar *name; complete = gst_element_get_state (element, &state, &pending, NULL); + name = gst_element_get_name (element); if (!complete) { - GST_DEBUG ("element %s still performing state change", - gst_element_get_name (element)); + GST_DEBUG ("element %s still performing state change", name); result = FALSE; done = TRUE; - break; + goto done; } else if (state != GST_STATE_PLAYING) { - GST_DEBUG ("element %s not playing %d %d", - gst_element_get_name (element), GST_STATE (element), - GST_STATE_PENDING (element)); - break; + GST_DEBUG ("element %s not playing %d %d", name, state, pending); + goto done; } eosed = g_list_find (pipeline->eosed, element); if (!eosed) { result = FALSE; done = TRUE; } + done: + g_free (name); gst_object_unref (GST_OBJECT (element)); break; } @@ -200,6 +206,7 @@ is_eos (GstPipeline * pipeline) break; } } + gst_iterator_free (sinks); return result; } @@ -225,7 +232,9 @@ pipeline_bus_handler (GstBus * bus, GstMessage * message, } /* we drop all EOS messages */ result = GST_BUS_DROP; + gst_message_unref (message); } + break; case GST_MESSAGE_ERROR: break; default: diff --git a/gst/gstthread.c b/gst/gstthread.c deleted file mode 100644 index a3c5d9193c..0000000000 --- a/gst/gstthread.c +++ /dev/null @@ -1,744 +0,0 @@ -/* GStreamer - * Copyright (C) 1999,2000 Erik Walthinsen - * 2000 Wim Taymans - * 2003 Benjamin Otte - * - * gstthread.c: Threaded container object - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Library General Public - * License as published by the Free Software Foundation; either - * version 2 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Library General Public License for more details. - * - * You should have received a copy of the GNU Library General Public - * License along with this library; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 02111-1307, USA. - */ - -#include "gst_private.h" - -#include "gstthread.h" -#include "gstmarshal.h" -#include "gstscheduler.h" -#include "gstinfo.h" - -#define GST_CAT_DEFAULT GST_CAT_THREAD -#define STACK_SIZE 0x200000 - -static GstElementDetails gst_thread_details = -GST_ELEMENT_DETAILS ("Threaded container", - "Generic/Bin", - "Container that creates/manages a thread", - "Erik Walthinsen , " - "Benjamin Otte dispose = gst_thread_dispose; - -#ifndef GST_DISABLE_LOADSAVE - gstobject_class->save_thyself = GST_DEBUG_FUNCPTR (gst_thread_save_thyself); - gstobject_class->restore_thyself = - GST_DEBUG_FUNCPTR (gst_thread_restore_thyself); -#endif - - gstelement_class->change_state = GST_DEBUG_FUNCPTR (gst_thread_change_state); - - gobject_class->set_property = GST_DEBUG_FUNCPTR (gst_thread_set_property); - gobject_class->get_property = GST_DEBUG_FUNCPTR (gst_thread_get_property); - - gstbin_class->child_state_change = - GST_DEBUG_FUNCPTR (gst_thread_child_state_change); -} - -static void -gst_thread_init (GTypeInstance * instance, gpointer g_class) -{ - GstScheduler *scheduler; - GstThread *thread = GST_THREAD (instance); - - GST_DEBUG ("initializing thread"); - - /* threads are managing bins and iterate themselves */ - /* CR1: the GstBin code checks these flags */ - GST_FLAG_SET (thread, GST_BIN_FLAG_MANAGER); - GST_FLAG_SET (thread, GST_BIN_SELF_SCHEDULABLE); - - scheduler = gst_scheduler_factory_make (NULL, GST_ELEMENT (thread)); - g_assert (scheduler); - - thread->lock = g_mutex_new (); - thread->cond = g_cond_new (); - thread->iterate_lock = g_mutex_new (); - - thread->thread_id = (GThread *) NULL; /* set in NULL -> READY */ - thread->priority = G_THREAD_PRIORITY_NORMAL; -} - -static void -gst_thread_dispose (GObject * object) -{ - GstThread *thread = GST_THREAD (object); - - GST_CAT_DEBUG (GST_CAT_REFCOUNTING, "GstThread: dispose"); - - /* if we get here, the thread is really stopped as it has released - * the last refcount to the thread object, so we can safely free the - * mutex and cond vars */ - G_OBJECT_CLASS (parent_class)->dispose (object); - - g_assert (GST_STATE (thread) == GST_STATE_NULL); - - GST_CAT_DEBUG (GST_CAT_REFCOUNTING, "GstThread: dispose, freeing locks"); - - g_mutex_free (thread->lock); - g_cond_free (thread->cond); - g_mutex_free (thread->iterate_lock); - - gst_object_replace ((GstObject **) & GST_ELEMENT_SCHED (thread), NULL); -} - -/** - * gst_thread_set_priority: - * @thread: the thread to change - * @priority: the new priority for the thread - * - * change the thread's priority - */ -void -gst_thread_set_priority (GstThread * thread, GThreadPriority priority) -{ - g_return_if_fail (GST_IS_THREAD (thread)); - - thread->priority = priority; -} - -static void -gst_thread_set_property (GObject * object, guint prop_id, const GValue * value, - GParamSpec * pspec) -{ - GstThread *thread; - - thread = GST_THREAD (object); - - switch (prop_id) { - case ARG_PRIORITY: - thread->priority = g_value_get_enum (value); - break; - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } -} - -static void -gst_thread_get_property (GObject * object, guint prop_id, GValue * value, - GParamSpec * pspec) -{ - GstThread *thread; - - thread = GST_THREAD (object); - - switch (prop_id) { - case ARG_PRIORITY: - g_value_set_enum (value, thread->priority); - break; - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } -} - - -/** - * gst_thread_new: - * @name: the name of the thread - * - * Create a new thread with the given name. - * - * Returns: The new thread - */ -GstElement * -gst_thread_new (const gchar * name) -{ - return gst_element_factory_make ("thread", name); -} - -/** - * gst_thread_get_current: - * - * Gets the current GstThread. - * - * Returns: The current GstThread or NULL if you are not running inside a - * #GstThread. - */ -GstThread * -gst_thread_get_current (void) -{ - return (GstThread *) g_private_get (gst_thread_current); -} - -static inline void -gst_thread_release_children_locks (GstThread * thread) -{ - GstRealPad *peer = NULL; - GstElement *peerelement; - GList *elements = (GList *) gst_bin_get_list (GST_BIN (thread)); - - while (elements) { - GstElement *element = GST_ELEMENT (elements->data); - GList *pads; - - g_assert (element); - GST_DEBUG_OBJECT (thread, "waking element \"%s\"", - GST_ELEMENT_NAME (element)); - elements = g_list_next (elements); - - if (!gst_element_release_locks (element)) - g_warning ("element %s could not release locks", - GST_ELEMENT_NAME (element)); - - pads = GST_ELEMENT_PADS (element); - - while (pads) { - if (GST_PAD_PEER (pads->data)) { - peer = GST_REAL_PAD (GST_PAD_PEER (pads->data)); - pads = g_list_next (pads); - } else { - pads = g_list_next (pads); - continue; - } - - if (!peer) - continue; - - peerelement = GST_PAD_PARENT (peer); - if (!peerelement) - continue; /* FIXME: deal with case where there's no peer */ - - if (GST_ELEMENT_SCHED (peerelement) != GST_ELEMENT_SCHED (thread)) { - GST_LOG_OBJECT (thread, "element \"%s\" has pad cross sched boundary", - GST_ELEMENT_NAME (element)); - GST_LOG_OBJECT (thread, "waking element \"%s\"", - GST_ELEMENT_NAME (peerelement)); - if (!gst_element_release_locks (peerelement)) - g_warning ("element %s could not release locks", - GST_ELEMENT_NAME (peerelement)); - } - } - } -} - -/* sync the main thread, if there is one and makes sure that the thread - * is not spinning and in the waiting state. We can only do this if - * this function is not called from the thread itself. - * - * This function should be called with the thread lock held. - */ -static void -gst_thread_sync (GstThread * thread, gboolean is_self) -{ - if (thread->thread_id == NULL) - return; - - /* need to stop spinning in any case */ - GST_FLAG_UNSET (thread, GST_THREAD_STATE_SPINNING); - - if (is_self) { - /* we're trying to sync ourself. Not much we can do here but hope - * that the thread will stop spinning and end up in the waiting - * state */ - GST_DEBUG_OBJECT (thread, "syncing itself"); - } else { - GST_DEBUG_OBJECT (thread, "syncing thread, grabbing lock"); - /* another thread is trying to sync us. The strategy is to - * repeadetly unlock the element locks until the thread is - * blocking in the waiting state. We use a timeout because - * unlocking all of the children at the same time is not - * atomic and might fail. */ - while (!GST_FLAG_IS_SET (thread, GST_THREAD_STATE_WAITING)) { - GTimeVal tv; - - GST_LOG_OBJECT (thread, "syncing thread..."); - - /* release child locks */ - gst_thread_release_children_locks (thread); - g_get_current_time (&tv); - g_time_val_add (&tv, 1000); /* wait a millisecond to sync the thread */ - GST_DEBUG_OBJECT (thread, "wait"); - g_cond_timed_wait (thread->cond, thread->lock, &tv); - } - GST_LOG_OBJECT (thread, "caught thread"); - /* at this point we should be waiting */ - g_assert (GST_FLAG_IS_SET (thread, GST_THREAD_STATE_WAITING)); - } - g_assert (!GST_FLAG_IS_SET (thread, GST_THREAD_STATE_SPINNING)); -} - -static GstElementStateReturn -gst_thread_change_state (GstElement * element) -{ - GstThread *thread; - GstElementStateReturn ret; - gint transition; - gboolean is_self; - - g_return_val_if_fail (GST_IS_THREAD (element), GST_STATE_FAILURE); - - GST_DEBUG_OBJECT (element, "changing state from %s to %s", - gst_element_state_get_name (GST_STATE (element)), - gst_element_state_get_name (GST_STATE_PENDING (element))); - - thread = GST_THREAD (element); - - /* boolean to check if we called the state change in the same thread as - * the iterate thread */ - is_self = (thread == gst_thread_get_current ()); - - GST_LOG_OBJECT (thread, "grabbing lock"); - g_mutex_lock (thread->lock); - - gst_thread_sync (thread, is_self); - - /* no iteration is allowed during this state change because an iteration - * can cause another state change conflicting with this one */ - /* do not try to grab the lock if this method is called from the - * same thread as the iterate thread, the lock might be held and we - * might deadlock */ - if (!is_self) - g_mutex_lock (thread->iterate_lock); - - transition = GST_STATE_TRANSITION (element); - - switch (transition) { - case GST_STATE_NULL_TO_READY: - /* create the thread */ - GST_FLAG_UNSET (thread, GST_THREAD_STATE_REAPING); - GST_LOG_OBJECT (element, "grabbing lock"); - thread->thread_id = g_thread_create_full (gst_thread_main_loop, - thread, STACK_SIZE, FALSE, TRUE, thread->priority, NULL); - if (!thread->thread_id) { - GST_ERROR_OBJECT (element, "g_thread_create_full failed"); - goto error_out; - } - GST_LOG_OBJECT (element, "GThread created"); - - /* wait for it to 'spin up' */ - g_cond_wait (thread->cond, thread->lock); - break; - case GST_STATE_READY_TO_PAUSED: - break; - case GST_STATE_PAUSED_TO_PLAYING: - { - /* FIXME: recurse into sub-bins */ - GList *elements = (GList *) gst_bin_get_list (GST_BIN (thread)); - - while (elements) { - gst_element_enable_threadsafe_properties ((GstElement *) elements-> - data); - elements = g_list_next (elements); - } - /* reset self to spinning */ - GST_FLAG_SET (thread, GST_THREAD_STATE_SPINNING); - break; - } - case GST_STATE_PLAYING_TO_PAUSED: - { - GList *elements; - - GST_FLAG_UNSET (thread, GST_THREAD_STATE_SPINNING); - - elements = (GList *) gst_bin_get_list (GST_BIN (thread)); - - while (elements) { - gst_element_disable_threadsafe_properties ((GstElement *) elements-> - data); - elements = g_list_next (elements); - } - break; - } - case GST_STATE_PAUSED_TO_READY: - GST_FLAG_UNSET (thread, GST_THREAD_STATE_SPINNING); - break; - case GST_STATE_READY_TO_NULL: - /* we can't join the threads here, because this could have been triggered - by ourself (ouch) */ - GST_LOG_OBJECT (thread, "destroying GThread %p", thread->thread_id); - GST_FLAG_SET (thread, GST_THREAD_STATE_REAPING); - GST_FLAG_UNSET (thread, GST_THREAD_STATE_SPINNING); - /* thread was already gone */ - if (thread->thread_id != NULL) { - thread->thread_id = NULL; - if (is_self) { - /* or should we continue? */ - GST_LOG_OBJECT (thread, - "Thread %s is destroying itself. Function call will not return!", - GST_ELEMENT_NAME (thread)); - gst_scheduler_reset (GST_ELEMENT_SCHED (thread)); - - /* unlock and signal - we are out */ - GST_DEBUG_OBJECT (thread, "signal"); - g_cond_signal (thread->cond); - GST_DEBUG_OBJECT (thread, "unlock"); - g_mutex_unlock (thread->lock); - - GST_INFO_OBJECT (thread, "GThread %p is exiting", g_thread_self ()); - - g_signal_emit (G_OBJECT (thread), gst_thread_signals[SHUTDOWN], 0); - - g_thread_exit (NULL); - return GST_STATE_SUCCESS; - } else { - /* now wait for the thread to destroy itself */ - GST_DEBUG_OBJECT (thread, "signal"); - g_cond_signal (thread->cond); - GST_DEBUG_OBJECT (thread, "wait"); - g_cond_wait (thread->cond, thread->lock); - GST_DEBUG_OBJECT (thread, "done"); - /* it should be dead now */ - } - } - break; - default: - break; - } - GST_LOG_OBJECT (thread, "unlocking lock"); - g_mutex_unlock (thread->lock); - - if (GST_ELEMENT_CLASS (parent_class)->change_state) { - ret = GST_ELEMENT_CLASS (parent_class)->change_state (GST_ELEMENT (thread)); - } else { - ret = GST_STATE_SUCCESS; - } - if (!is_self) - g_mutex_unlock (thread->iterate_lock); - - return ret; - -error_out: - GST_CAT_DEBUG (GST_CAT_STATES, "changing state from %s to %s failed for %s", - gst_element_state_get_name (GST_STATE (element)), - gst_element_state_get_name (GST_STATE_PENDING (element)), - GST_ELEMENT_NAME (element)); - - g_mutex_unlock (thread->lock); - - if (!is_self) - g_mutex_unlock (thread->iterate_lock); - - return GST_STATE_FAILURE; -} - -/* When a child changes its state the thread might have to start. - * - * When we are in the thread context we set the spinning flag. - * When we are not in the thread context, we grab the lock. The - * thread can then only be in the waiting or spinning state. If it's - * waiting we signal it to start. If it was spinning, we let it spin. - */ -static void -gst_thread_child_state_change (GstBin * bin, GstElementState oldstate, - GstElementState newstate, GstElement * element) -{ - GstThread *thread = GST_THREAD (bin); - gboolean is_self; - GstThread *current; - - current = gst_thread_get_current (); - - is_self = (current == thread); - - GST_LOG_OBJECT (bin, "(from thread %s) child %s changed state from %s to %s", - current ? GST_ELEMENT_NAME (current) : - "(none)", GST_ELEMENT_NAME (element), - gst_element_state_get_name (oldstate), - gst_element_state_get_name (newstate)); - - if (parent_class->child_state_change) - parent_class->child_state_change (bin, oldstate, newstate, element); - - /* see if we have to wake up the thread now. */ - if (is_self) { - GST_LOG_OBJECT (element, "we are in the thread context"); - /* we are the thread, set the spinning flag so that we start spinning - * next time */ - if (newstate == GST_STATE_PLAYING) { - GST_FLAG_SET (thread, GST_THREAD_STATE_SPINNING); - } - } else { - g_mutex_lock (thread->lock); - /* thread is now spinning or waiting after grabbing the lock */ - if (newstate == GST_STATE_PLAYING) { - if (!GST_FLAG_IS_SET (thread, GST_THREAD_STATE_WAITING)) { - /* its spinning, that's not really a problem, it will - * continue to do so */ - GST_LOG_OBJECT (element, "thread is playing"); - } else { - /* waiting, set spinning flag and trigger restart. */ - GST_LOG_OBJECT (element, "signal playing"); - GST_FLAG_SET (thread, GST_THREAD_STATE_SPINNING); - g_cond_signal (GST_THREAD (bin)->cond); - } - } - g_mutex_unlock (thread->lock); - } - GST_LOG_OBJECT (element, "done child state change"); -} - -/** - * gst_thread_main_loop: - * @arg: the thread to start - * - * The main loop of the thread. The thread will iterate - * while the state is GST_THREAD_STATE_SPINNING. - */ -static void * -gst_thread_main_loop (void *arg) -{ - GstThread *thread = NULL; - GstElement *element = NULL; - GstScheduler *sched; - - thread = GST_THREAD (arg); - GST_LOG_OBJECT (element, "grabbing lock"); - g_mutex_lock (thread->lock); - element = GST_ELEMENT (arg); - GST_LOG_OBJECT (thread, "Started main loop"); - - /* initialize gst_thread_current */ - g_private_set (gst_thread_current, thread); - - /* set up the element's scheduler */ - gst_scheduler_setup (GST_ELEMENT_SCHED (thread)); - GST_FLAG_UNSET (thread, GST_THREAD_STATE_REAPING); - GST_FLAG_UNSET (thread, GST_THREAD_STATE_WAITING); - - /* signals the startup of the thread */ - GST_LOG_OBJECT (element, "signal"); - g_cond_signal (thread->cond); - GST_LOG_OBJECT (element, "unlocking lock"); - - gst_object_ref (GST_OBJECT (thread)); - /* as long as we're not dying we can continue */ - while (!(GST_FLAG_IS_SET (thread, GST_THREAD_STATE_REAPING))) { - /* in the playing state we need to do something special */ - if (GST_STATE (thread) == GST_STATE_PLAYING) { - GST_LOG_OBJECT (thread, "starting to iterate"); - /* continue iteration while spinning */ - while (GST_FLAG_IS_SET (thread, GST_THREAD_STATE_SPINNING)) { - gboolean status; - - g_mutex_unlock (thread->lock); - g_mutex_lock (thread->iterate_lock); - status = gst_bin_iterate (GST_BIN (thread)); - g_mutex_unlock (thread->iterate_lock); - g_mutex_lock (thread->lock); - - if (!status) { - GST_DEBUG_OBJECT (thread, "iterate returned false"); - if (GST_STATE (thread) != GST_STATE_PLAYING) { - GST_DEBUG_OBJECT (thread, - "stopping spinning as state is not playing"); - GST_FLAG_UNSET (thread, GST_THREAD_STATE_SPINNING); - } - } - /* if we hold the last refcount, the app unreffed the - * thread and we should stop ASAP */ - if (G_OBJECT (thread)->ref_count == 1) { - GST_DEBUG_OBJECT (thread, "reaping as refcount is only 1"); - GST_FLAG_SET (thread, GST_THREAD_STATE_REAPING); - GST_FLAG_UNSET (thread, GST_THREAD_STATE_SPINNING); - } - } - } - /* do not try to sync when we are REAPING */ - if (!(GST_FLAG_IS_SET (thread, GST_THREAD_STATE_REAPING))) { - /* sync section */ - GST_LOG_OBJECT (thread, "entering sync"); - GST_DEBUG_OBJECT (thread, "signal"); - g_cond_signal (thread->cond); - GST_DEBUG_OBJECT (thread, "wait"); - GST_FLAG_UNSET (thread, GST_THREAD_STATE_SPINNING); - GST_FLAG_SET (thread, GST_THREAD_STATE_WAITING); - g_cond_wait (thread->cond, thread->lock); - GST_FLAG_UNSET (thread, GST_THREAD_STATE_WAITING); - GST_LOG_OBJECT (thread, "wait done"); - } - } - GST_LOG_OBJECT (thread, "unlocking lock"); - thread->thread_id = NULL; - g_mutex_unlock (thread->lock); - - /* we need to destroy the scheduler here because it might have - * mapped it's stack into the threads stack space */ - sched = GST_ELEMENT_SCHED (thread); - if (sched) - gst_scheduler_reset (sched); - - g_signal_emit (G_OBJECT (thread), gst_thread_signals[SHUTDOWN], 0); - - /* signal - we are out */ - GST_LOG_OBJECT (thread, "Thread %p exits main loop", g_thread_self ()); - g_cond_signal (thread->cond); - gst_object_unref (GST_OBJECT (thread)); - /* don't assume the GstThread object exists anymore now */ - - return NULL; -} - -#ifndef GST_DISABLE_LOADSAVE -static xmlNodePtr -gst_thread_save_thyself (GstObject * object, xmlNodePtr self) -{ - if (GST_OBJECT_CLASS (parent_class)->save_thyself) - GST_OBJECT_CLASS (parent_class)->save_thyself (object, self); - return NULL; -} - -static void -gst_thread_restore_thyself (GstObject * object, xmlNodePtr self) -{ - GST_LOG_OBJECT (object, "restoring"); - - if (GST_OBJECT_CLASS (parent_class)->restore_thyself) - GST_OBJECT_CLASS (parent_class)->restore_thyself (object, self); -} -#endif /* GST_DISABLE_LOADSAVE */ diff --git a/gst/gstthread.h b/gst/gstthread.h deleted file mode 100644 index 806923d684..0000000000 --- a/gst/gstthread.h +++ /dev/null @@ -1,88 +0,0 @@ -/* GStreamer - * Copyright (C) 1999,2000 Erik Walthinsen - * 2000 Wim Taymans - * - * gstthread.h: Header for GstThread object - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Library General Public - * License as published by the Free Software Foundation; either - * version 2 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Library General Public License for more details. - * - * You should have received a copy of the GNU Library General Public - * License along with this library; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 02111-1307, USA. - */ - - -#ifndef __GST_THREAD_H__ -#define __GST_THREAD_H__ - -#include - -#include - - -G_BEGIN_DECLS - -extern GPrivate *gst_thread_current; - -typedef enum { - GST_THREAD_STATE_SPINNING = GST_BIN_FLAG_LAST, - GST_THREAD_STATE_REAPING, - GST_THREAD_STATE_WAITING, - - /* padding */ - GST_THREAD_FLAG_LAST = GST_BIN_FLAG_LAST + 4 -} GstThreadState; - -#define GST_TYPE_THREAD (gst_thread_get_type()) -#define GST_THREAD(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GST_TYPE_THREAD,GstThread)) -#define GST_IS_THREAD(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GST_TYPE_THREAD)) -#define GST_THREAD_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GST_TYPE_THREAD,GstThreadClass)) -#define GST_IS_THREAD_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GST_TYPE_THREAD)) -#define GST_THREAD_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GST_TYPE_THREAD, GstThreadClass)) - -typedef struct _GstThread GstThread; -typedef struct _GstThreadClass GstThreadClass; - -struct _GstThread { - GstBin bin; - - GThread *thread_id; /* id of the thread, if any */ - GThreadPriority priority; - - GMutex *lock; /* thread lock/condititon pairs */ - GCond *cond; /* used to control the thread */ - - GMutex *iterate_lock; /* lock iteration in state change */ - - gpointer _gst_reserved[GST_PADDING-1]; -}; - -struct _GstThreadClass { - GstBinClass parent_class; - - /* signals */ - void (*shutdown) (GstThread *thread); - - gpointer _gst_reserved[GST_PADDING]; -}; - -GType gst_thread_get_type (void); - -GstElement* gst_thread_new (const gchar *name); - -void gst_thread_set_priority (GstThread *thread, GThreadPriority priority); -GstThread * gst_thread_get_current (void); - -G_END_DECLS - - -#endif /* __GST_THREAD_H__ */ diff --git a/gst/gstutils.c b/gst/gstutils.c index dd585bb50d..4739e72035 100644 --- a/gst/gstutils.c +++ b/gst/gstutils.c @@ -610,10 +610,11 @@ GstPad * gst_element_get_compatible_pad_filtered (GstElement * element, GstPad * pad, const GstCaps * filtercaps) { - const GList *pads; + GstIterator *pads; GstPadTemplate *templ; GstCaps *templcaps; GstPad *foundpad = NULL; + gboolean done; g_return_val_if_fail (GST_IS_ELEMENT (element), NULL); g_return_val_if_fail (GST_IS_PAD (pad), NULL); @@ -627,21 +628,56 @@ gst_element_get_compatible_pad_filtered (GstElement * element, GstPad * pad, g_return_val_if_fail (pad != NULL, NULL); g_return_val_if_fail (GST_RPAD_PEER (pad) == NULL, NULL); + done = FALSE; /* try to get an existing unlinked pad */ - pads = gst_element_get_pad_list (element); - while (pads) { - GstPad *current = GST_PAD (pads->data); + pads = gst_element_iterate_pads (element); + while (!done) { + gpointer padptr; - GST_CAT_LOG (GST_CAT_ELEMENT_PADS, "examing pad %s:%s", - GST_DEBUG_PAD_NAME (current)); - if (GST_PAD_PEER (current) == NULL && - gst_pad_can_link_filtered (pad, current, filtercaps)) { - GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, - "found existing unlinked pad %s:%s", GST_DEBUG_PAD_NAME (current)); - return current; + switch (gst_iterator_next (pads, &padptr)) { + case GST_ITERATOR_OK: + { + GstPad *peer; + GstPad *current; + + current = GST_PAD (padptr); + + GST_CAT_LOG (GST_CAT_ELEMENT_PADS, "examing pad %s:%s", + GST_DEBUG_PAD_NAME (current)); + + peer = gst_pad_get_peer (current); + + if (peer == NULL && + gst_pad_can_link_filtered (pad, current, filtercaps)) { + + GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, + "found existing unlinked pad %s:%s", + GST_DEBUG_PAD_NAME (current)); + + gst_iterator_free (pads); + + return current; + } else { + GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "unreffing pads"); + + gst_object_unref (GST_OBJECT (current)); + if (peer) + gst_object_unref (GST_OBJECT (peer)); + } + break; + } + case GST_ITERATOR_DONE: + done = TRUE; + break; + case GST_ITERATOR_RESYNC: + gst_iterator_resync (pads); + break; + case GST_ITERATOR_ERROR: + g_assert_not_reached (); + break; } - pads = g_list_next (pads); } + gst_iterator_free (pads); /* try to create a new one */ /* requesting is a little crazy, we need a template. Let's create one */ @@ -792,18 +828,24 @@ gst_element_link_pads_filtered (GstElement * src, const gchar * srcpadname, if (!(GST_PAD_DIRECTION (srcpad) == GST_PAD_SRC)) { GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "pad %s:%s is no src pad", GST_DEBUG_PAD_NAME (srcpad)); + gst_object_unref (GST_OBJECT (srcpad)); return FALSE; } if (GST_PAD_PEER (srcpad) != NULL) { GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "pad %s:%s is already linked", GST_DEBUG_PAD_NAME (srcpad)); + gst_object_unref (GST_OBJECT (srcpad)); return FALSE; } } srcpads = NULL; } else { - srcpads = gst_element_get_pad_list (src); + GST_LOCK (src); + srcpads = GST_ELEMENT_PADS (src); srcpad = srcpads ? (GstPad *) GST_PAD_REALIZE (srcpads->data) : NULL; + if (srcpad) + gst_object_ref (GST_OBJECT (srcpad)); + GST_UNLOCK (src); } if (destpadname) { destpad = gst_element_get_pad (dest, destpadname); @@ -815,23 +857,36 @@ gst_element_link_pads_filtered (GstElement * src, const gchar * srcpadname, if (!(GST_PAD_DIRECTION (destpad) == GST_PAD_SINK)) { GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "pad %s:%s is no sink pad", GST_DEBUG_PAD_NAME (destpad)); + gst_object_unref (GST_OBJECT (destpad)); return FALSE; } if (GST_PAD_PEER (destpad) != NULL) { GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "pad %s:%s is already linked", GST_DEBUG_PAD_NAME (destpad)); + gst_object_unref (GST_OBJECT (destpad)); return FALSE; } } destpads = NULL; } else { - destpads = gst_element_get_pad_list (dest); + GST_LOCK (dest); + destpads = GST_ELEMENT_PADS (dest); destpad = destpads ? (GstPad *) GST_PAD_REALIZE (destpads->data) : NULL; + if (destpad) + gst_object_ref (GST_OBJECT (destpad)); + GST_UNLOCK (dest); } if (srcpadname && destpadname) { + gboolean result; + /* two explicitly specified pads */ - return gst_pad_link_filtered (srcpad, destpad, filtercaps); + result = gst_pad_link_filtered (srcpad, destpad, filtercaps); + + gst_object_unref (GST_OBJECT (srcpad)); + gst_object_unref (GST_OBJECT (destpad)); + + return result; } if (srcpad) { /* loop through the allowed pads in the source, trying to find a @@ -852,21 +907,32 @@ gst_element_link_pads_filtered (GstElement * src, const gchar * srcpadname, filtercaps) == GST_PAD_LINK_OK) { GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "linked pad %s:%s to pad %s:%s", GST_DEBUG_PAD_NAME (srcpad), GST_DEBUG_PAD_NAME (temp)); + if (destpad) + gst_object_unref (GST_OBJECT (destpad)); + gst_object_unref (GST_OBJECT (srcpad)); + gst_object_unref (GST_OBJECT (temp)); return TRUE; } } /* find a better way for this mess */ if (srcpads) { srcpads = g_list_next (srcpads); - if (srcpads) + if (srcpads) { + gst_object_unref (GST_OBJECT (srcpad)); srcpad = (GstPad *) GST_PAD_REALIZE (srcpads->data); + } } } while (srcpads); } if (srcpadname) { GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "no link possible from %s:%s to %s", GST_DEBUG_PAD_NAME (srcpad), GST_ELEMENT_NAME (dest)); + gst_object_unref (GST_OBJECT (srcpad)); + if (destpad) + gst_object_unref (GST_OBJECT (destpad)); return FALSE; + } else { + gst_object_unref (GST_OBJECT (srcpad)); } if (destpad) { /* loop through the existing pads in the destination */ @@ -883,20 +949,33 @@ gst_element_link_pads_filtered (GstElement * src, const gchar * srcpadname, filtercaps) == GST_PAD_LINK_OK) { GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "linked pad %s:%s to pad %s:%s", GST_DEBUG_PAD_NAME (temp), GST_DEBUG_PAD_NAME (destpad)); + gst_object_unref (GST_OBJECT (temp)); + gst_object_unref (GST_OBJECT (destpad)); + if (srcpad) + gst_object_unref (GST_OBJECT (srcpad)); return TRUE; } } if (destpads) { destpads = g_list_next (destpads); - if (destpads) + if (destpads) { + gst_object_unref (GST_OBJECT (destpad)); destpad = (GstPad *) GST_PAD_REALIZE (destpads->data); + } } } while (destpads); } if (destpadname) { GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "no link possible from %s to %s:%s", GST_ELEMENT_NAME (src), GST_DEBUG_PAD_NAME (destpad)); + gst_object_unref (GST_OBJECT (destpad)); + if (srcpad) + gst_object_unref (GST_OBJECT (srcpad)); return FALSE; + } else { + gst_object_unref (GST_OBJECT (destpad)); + if (srcpad) + gst_object_unref (GST_OBJECT (srcpad)); } GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, @@ -923,6 +1002,8 @@ gst_element_link_pads_filtered (GstElement * src, const gchar * srcpadname, GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "linked pad %s:%s to pad %s:%s", GST_DEBUG_PAD_NAME (srcpad), GST_DEBUG_PAD_NAME (destpad)); + gst_object_unref (GST_OBJECT (srcpad)); + gst_object_unref (GST_OBJECT (destpad)); return TRUE; } /* it failed, so we release the request pads */ @@ -1111,8 +1192,8 @@ gst_element_unlink_many (GstElement * element_1, GstElement * element_2, ...) void gst_element_unlink (GstElement * src, GstElement * dest) { - const GList *srcpads; - GstPad *pad; + GstIterator *pads; + gboolean done = FALSE; g_return_if_fail (GST_IS_ELEMENT (src)); g_return_if_fail (GST_IS_ELEMENT (dest)); @@ -1120,23 +1201,46 @@ gst_element_unlink (GstElement * src, GstElement * dest) GST_CAT_DEBUG (GST_CAT_ELEMENT_PADS, "unlinking \"%s\" and \"%s\"", GST_ELEMENT_NAME (src), GST_ELEMENT_NAME (dest)); - srcpads = gst_element_get_pad_list (src); + pads = gst_element_iterate_pads (src); + while (!done) { + gpointer data; - while (srcpads) { - pad = GST_PAD (srcpads->data); + switch (gst_iterator_next (pads, &data)) { + case GST_ITERATOR_OK: + { + GstPad *pad = GST_PAD_CAST (data); - /* we only care about real src pads */ - if (GST_IS_REAL_PAD (pad) && GST_PAD_IS_SRC (pad)) { - GstPad *peerpad = GST_PAD_PEER (pad); + /* we only care about real src pads */ + if (GST_IS_REAL_PAD (pad) && GST_PAD_IS_SRC (pad)) { + GstPad *peerpad = gst_pad_get_peer (pad); - /* see if the pad is connected and is really a pad - * of dest */ - if (peerpad && (GST_OBJECT_PARENT (peerpad) == (GstObject *) dest)) { - gst_pad_unlink (pad, peerpad); + /* see if the pad is connected and is really a pad + * of dest */ + if (peerpad) { + GstElement *peerelem = gst_pad_get_parent (peerpad); + + if (peerelem == dest) { + gst_pad_unlink (pad, peerpad); + } + if (peerelem) + gst_object_unref (GST_OBJECT (peerelem)); + + gst_object_unref (GST_OBJECT (peerpad)); + } + } + gst_object_unref (GST_OBJECT (pad)); + break; } + case GST_ITERATOR_RESYNC: + gst_iterator_resync (pads); + break; + case GST_ITERATOR_DONE: + done = TRUE; + break; + default: + g_assert_not_reached (); + break; } - - srcpads = g_list_next (srcpads); } } diff --git a/gst/parse/grammar.y b/gst/parse/grammar.y index 8f4a7dd101..69b67f988c 100644 --- a/gst/parse/grammar.y +++ b/gst/parse/grammar.y @@ -289,25 +289,51 @@ gst_parse_free_link (link_t *link) if (link->caps) gst_caps_unref (link->caps); gst_parse_link_free (link); } + static void gst_parse_element_lock (GstElement *element, gboolean lock) { GstPad *pad; - GList *walk = (GList *) gst_element_get_pad_list (element); + GstIterator *pads; gboolean unlocked_peer = FALSE; + gboolean done = FALSE; + GList *walk; if (gst_element_is_locked_state (element) == lock) return; + /* check if we have an unlocked peer */ - for (; walk; walk = walk->next) { - pad = (GstPad *) GST_PAD_REALIZE (walk->data); - if (GST_PAD_IS_SINK (pad) && GST_PAD_PEER (pad) && - !gst_element_is_locked_state (GST_PAD_PARENT (GST_PAD_PEER (pad)))) { - unlocked_peer = TRUE; - break; + pads = gst_element_iterate_pads (element); + while (!done) { + gpointer data; + switch (gst_iterator_next (pads, &data)) { + case GST_ITERATOR_OK: + { + GstPad *pad = GST_PAD_CAST (data); + + pad = gst_pad_realize (pad); + if (GST_PAD_IS_SINK (pad) && GST_PAD_PEER (pad) && + !gst_element_is_locked_state (GST_PAD_PARENT (GST_PAD_PEER (pad)))) { + unlocked_peer = TRUE; + done = TRUE; + } + gst_object_unref (GST_OBJECT (pad)); + break; + } + case GST_ITERATOR_RESYNC: + unlocked_peer = FALSE; + gst_iterator_resync (pads); + break; + case GST_ITERATOR_DONE: + done = TRUE; + break; + default: + g_assert_not_reached (); + break; } - } - + } + gst_iterator_free (pads); + if (!(lock && unlocked_peer)) { GST_CAT_DEBUG (GST_CAT_PIPELINE, "setting locked state on element"); gst_element_set_locked_state (element, lock); @@ -325,7 +351,7 @@ gst_parse_element_lock (GstElement *element, gboolean lock) } /* check if there are other pads to (un)lock */ - walk = (GList *) gst_element_get_pad_list (element); + walk = (GList *) element->pads; for (; walk; walk = walk->next) { pad = (GstPad *) GST_PAD_REALIZE (walk->data); if (GST_PAD_IS_SRC (pad) && GST_PAD_PEER (pad)) { diff --git a/tools/gst-compprep.c b/tools/gst-compprep.c index c9073f2ae7..0d5ea5a87f 100644 --- a/tools/gst-compprep.c +++ b/tools/gst-compprep.c @@ -91,7 +91,7 @@ main (int argc, char *argv[]) padtemplate->name_template); } - pads = gst_element_get_pad_list (element); + pads = element->pads; while (pads) { pad = (GstPad *) (pads->data); pads = g_list_next (pads); diff --git a/tools/gst-inspect.c b/tools/gst-inspect.c index fa985b45e7..1609d7d858 100644 --- a/tools/gst-inspect.c +++ b/tools/gst-inspect.c @@ -603,7 +603,7 @@ print_pad_info (GstElement * element) return; } - pads = gst_element_get_pad_list (element); + pads = element->pads; while (pads) { pad = GST_PAD (pads->data); pads = g_list_next (pads); diff --git a/tools/gst-launch.c b/tools/gst-launch.c index 4e21852563..9c3aad87f1 100644 --- a/tools/gst-launch.c +++ b/tools/gst-launch.c @@ -532,6 +532,7 @@ main (int argc, char *argv[]) end: + fprintf (stderr, _("FREEING pipeline ...\n")); gst_object_unref (GST_OBJECT (pipeline)); if (trace) diff --git a/tools/gst-xmlinspect.c b/tools/gst-xmlinspect.c index 1887edb829..723bc8431a 100644 --- a/tools/gst-xmlinspect.c +++ b/tools/gst-xmlinspect.c @@ -576,7 +576,7 @@ print_element_info (GstElementFactory * factory) if (element->numpads) { const GList *pads; - pads = gst_element_get_pad_list (element); + pads = element->pads; while (pads) { pad = GST_PAD (pads->data); pads = g_list_next (pads);