From 6fa351407a06593ace25e64602b7703d282bd5c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Tue, 3 Jul 2018 20:07:31 +0300 Subject: [PATCH] miniobject: Add parent pointers to the miniobject to influence writability Every container of miniobjects now needs to store itself as parent in the child object, and remove itself again at a later time. A miniobject is only writable if there is at most one parent, and that parent is writable itself, and if the reference count of the miniobject is 1. GstBuffer (for memories), GstBufferList (for buffers) and GstSample (for caps, buffer, bufferlist) was updated accordingly. Without this it was possible to have e.g. a bufferlist with refcount 2 in two places, modifying the same buffer with refcount 1 at the same time. https://bugzilla.gnome.org/show_bug.cgi?id=796692 --- docs/gst/gstreamer-sections.txt | 3 + gst/gstbuffer.c | 18 ++ gst/gstbufferlist.c | 71 +++++- gst/gstminiobject.c | 414 +++++++++++++++++++++++++++++--- gst/gstminiobject.h | 11 +- gst/gstsample.c | 96 ++++++-- win32/common/libgstreamer.def | 2 + 7 files changed, 551 insertions(+), 64 deletions(-) diff --git a/docs/gst/gstreamer-sections.txt b/docs/gst/gstreamer-sections.txt index a627fce09e..f901f3476d 100644 --- a/docs/gst/gstreamer-sections.txt +++ b/docs/gst/gstreamer-sections.txt @@ -1820,6 +1820,9 @@ gst_mini_object_unlock gst_mini_object_is_writable gst_mini_object_make_writable +gst_mini_object_add_parent +gst_mini_object_remove_parent + gst_mini_object_copy gst_mini_object_set_qdata diff --git a/gst/gstbuffer.c b/gst/gstbuffer.c index 38a7a70607..e18e9bb849 100644 --- a/gst/gstbuffer.c +++ b/gst/gstbuffer.c @@ -291,11 +291,15 @@ _replace_memory (GstBuffer * buffer, guint len, guint idx, guint length, GstMemory *old = GST_BUFFER_MEM_PTR (buffer, i); gst_memory_unlock (old, GST_LOCK_FLAG_EXCLUSIVE); + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (old), + GST_MINI_OBJECT_CAST (buffer)); gst_memory_unref (old); } if (mem != NULL) { /* replace with single memory */ + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (mem), + GST_MINI_OBJECT_CAST (buffer)); gst_memory_lock (mem, GST_LOCK_FLAG_EXCLUSIVE); GST_BUFFER_MEM_PTR (buffer, idx) = mem; idx++; @@ -438,6 +442,8 @@ _memory_add (GstBuffer * buffer, gint idx, GstMemory * mem) /* and insert the new buffer */ GST_BUFFER_MEM_PTR (buffer, idx) = mem; GST_BUFFER_MEM_LEN (buffer) = len + 1; + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (mem), + GST_MINI_OBJECT_CAST (buffer)); GST_BUFFER_FLAG_SET (buffer, GST_BUFFER_FLAG_TAG_MEMORY); } @@ -749,6 +755,8 @@ _gst_buffer_free (GstBuffer * buffer) len = GST_BUFFER_MEM_LEN (buffer); for (i = 0; i < len; i++) { gst_memory_unlock (GST_BUFFER_MEM_PTR (buffer, i), GST_LOCK_FLAG_EXCLUSIVE); + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (GST_BUFFER_MEM_PTR + (buffer, i)), GST_MINI_OBJECT_CAST (buffer)); gst_memory_unref (GST_BUFFER_MEM_PTR (buffer, i)); } @@ -1063,10 +1071,14 @@ _get_mapped (GstBuffer * buffer, guint idx, GstMapInfo * info, if (mapped != mem) { /* memory changed, lock new memory */ + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (mapped), + GST_MINI_OBJECT_CAST (buffer)); gst_memory_lock (mapped, GST_LOCK_FLAG_EXCLUSIVE); GST_BUFFER_MEM_PTR (buffer, idx) = mapped; /* unlock old memory */ gst_memory_unlock (mem, GST_LOCK_FLAG_EXCLUSIVE); + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (mem), + GST_MINI_OBJECT_CAST (buffer)); GST_BUFFER_FLAG_SET (buffer, GST_BUFFER_FLAG_TAG_MEMORY); } gst_memory_unref (mem); @@ -1636,9 +1648,13 @@ gst_buffer_resize_range (GstBuffer * buffer, guint idx, gint length, if (newmem == NULL) return FALSE; + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (newmem), + GST_MINI_OBJECT_CAST (buffer)); gst_memory_lock (newmem, GST_LOCK_FLAG_EXCLUSIVE); GST_BUFFER_MEM_PTR (buffer, i) = newmem; gst_memory_unlock (mem, GST_LOCK_FLAG_EXCLUSIVE); + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (mem), + GST_MINI_OBJECT_CAST (buffer)); gst_memory_unref (mem); GST_BUFFER_FLAG_SET (buffer, GST_BUFFER_FLAG_TAG_MEMORY); @@ -2098,6 +2114,8 @@ gst_buffer_append_region (GstBuffer * buf1, GstBuffer * buf2, gssize offset, GstMemory *mem; mem = GST_BUFFER_MEM_PTR (buf2, i); + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (mem), + GST_MINI_OBJECT_CAST (buf2)); GST_BUFFER_MEM_PTR (buf2, i) = NULL; _memory_add (buf1, -1, mem); } diff --git a/gst/gstbufferlist.c b/gst/gstbufferlist.c index 16de4cecf0..b9e5d1fae1 100644 --- a/gst/gstbufferlist.c +++ b/gst/gstbufferlist.c @@ -88,8 +88,11 @@ _gst_buffer_list_copy (GstBufferList * list) copy = gst_buffer_list_new_sized (list->n_allocated); /* add and ref all buffers in the array */ - for (i = 0; i < len; i++) + for (i = 0; i < len; i++) { copy->buffers[i] = gst_buffer_ref (list->buffers[i]); + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (copy->buffers[i]), + GST_MINI_OBJECT_CAST (copy)); + } copy->n_buffers = len; @@ -105,8 +108,11 @@ _gst_buffer_list_free (GstBufferList * list) /* unrefs all buffers too */ len = list->n_buffers; - for (i = 0; i < len; i++) + for (i = 0; i < len; i++) { + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (list->buffers[i]), + GST_MINI_OBJECT_CAST (list)); gst_buffer_unref (list->buffers[i]); + } if (GST_BUFFER_LIST_IS_USING_DYNAMIC_ARRAY (list)) g_free (list->buffers); @@ -205,8 +211,11 @@ gst_buffer_list_remove_range_internal (GstBufferList * list, guint idx, guint i; if (unref_old) { - for (i = idx; i < idx + length; ++i) + for (i = idx; i < idx + length; ++i) { + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (list->buffers[i]), + GST_MINI_OBJECT_CAST (list)); gst_buffer_unref (list->buffers[i]); + } } if (idx + length != list->n_buffers) { @@ -238,25 +247,57 @@ gst_buffer_list_foreach (GstBufferList * list, GstBufferListFunc func, { guint i, len; gboolean ret = TRUE; + gboolean list_was_writable; g_return_val_if_fail (GST_IS_BUFFER_LIST (list), FALSE); g_return_val_if_fail (func != NULL, FALSE); + list_was_writable = gst_buffer_list_is_writable (list); + len = list->n_buffers; for (i = 0; i < len;) { GstBuffer *buf, *buf_ret; + gboolean was_writable; buf = buf_ret = list->buffers[i]; + + /* If the buffer is writable, we remove us as parent for now to + * allow the callback to destroy the buffer. If we get the buffer + * back, we add ourselves as parent again. + * + * Non-writable buffers just get another reference as they were not + * writable to begin with, and they would possibly become writable + * by removing ourselves as parent + */ + was_writable = list_was_writable && gst_buffer_is_writable (buf); + + if (was_writable) + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (buf), + GST_MINI_OBJECT_CAST (list)); + else + gst_buffer_ref (buf); + ret = func (&buf_ret, i, user_data); /* Check if the function changed the buffer */ if (buf != buf_ret) { if (buf_ret == NULL) { - gst_buffer_list_remove_range_internal (list, i, 1, FALSE); + gst_buffer_list_remove_range_internal (list, i, 1, !was_writable); --len; } else { + if (!was_writable) + gst_buffer_unref (buf); + list->buffers[i] = buf_ret; + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (buf_ret), + GST_MINI_OBJECT_CAST (list)); } + } else { + if (was_writable) + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (buf), + GST_MINI_OBJECT_CAST (list)); + else + gst_buffer_unref (buf); } if (!ret) @@ -311,14 +352,26 @@ gst_buffer_list_get (GstBufferList * list, guint idx) GstBuffer * gst_buffer_list_get_writable (GstBufferList * list, guint idx) { - GstBuffer **p_buf; + GstBuffer *new_buf; g_return_val_if_fail (GST_IS_BUFFER_LIST (list), NULL); g_return_val_if_fail (gst_buffer_list_is_writable (list), NULL); g_return_val_if_fail (idx < list->n_buffers, NULL); - p_buf = &list->buffers[idx]; - return (*p_buf = gst_buffer_make_writable (*p_buf)); + /* We have to implement this manually here to correctly add/remove the + * parent */ + if (gst_buffer_is_writable (list->buffers[idx])) + return list->buffers[idx]; + + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (list->buffers[idx]), + GST_MINI_OBJECT_CAST (list)); + new_buf = gst_buffer_copy (list->buffers[idx]); + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (new_buf), + GST_MINI_OBJECT_CAST (list)); + gst_buffer_unref (list->buffers[idx]); + list->buffers[idx] = new_buf; + + return new_buf; } /** @@ -349,6 +402,8 @@ gst_buffer_list_insert (GstBufferList * list, gint idx, GstBuffer * buffer) g_return_if_fail (gst_buffer_list_is_writable (list)); if (idx == -1 && list->n_buffers < list->n_allocated) { + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (buffer), + GST_MINI_OBJECT_CAST (list)); list->buffers[list->n_buffers++] = buffer; return; } @@ -379,6 +434,8 @@ gst_buffer_list_insert (GstBufferList * list, gint idx, GstBuffer * buffer) ++list->n_buffers; list->buffers[idx] = buffer; + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (buffer), + GST_MINI_OBJECT_CAST (list)); } /** diff --git a/gst/gstminiobject.c b/gst/gstminiobject.c index 89fc4f1c4a..251d5262ce 100644 --- a/gst/gstminiobject.c +++ b/gst/gstminiobject.c @@ -37,9 +37,10 @@ * A copy can be made with gst_mini_object_copy(). * * gst_mini_object_is_writable() will return %TRUE when the refcount of the - * object is exactly 1, meaning the current caller has the only reference to the - * object. gst_mini_object_make_writable() will return a writable version of the - * object, which might be a new copy when the refcount was not 1. + * object is exactly 1 and there is no parent or a single parent exists and is + * writable itself, meaning the current caller has the only reference to the + * object. gst_mini_object_make_writable() will return a writable version of + * the object, which might be a new copy when the refcount was not 1. * * Opaque data can be associated with a #GstMiniObject with * gst_mini_object_set_qdata() and gst_mini_object_get_qdata(). The data is @@ -71,6 +72,35 @@ static GQuark weak_ref_quark; #define LOCK_MASK ((SHARE_ONE - 1) - FLAG_MASK) #define LOCK_FLAG_MASK (SHARE_ONE - 1) +/* For backwards compatibility reasons we use the + * guint and gpointer in the GstMiniObject struct in + * a rather complicated way to store the parent(s) and qdata. + * Originally the were just the number of qdatas and the qdata. + * + * The guint is used as an atomic state integer with the following + * states: + * - Locked: 0, basically a spinlock + * - No parent, no qdata: 1 (pointer is NULL) + * - One parent: 2 (pointer contains the parent) + * - Multiple parents or qdata: 3 (pointer contains a PrivData struct) + * + * Unless we're in state 3, we always have to move to Locking state + * atomically and release that again later to the target state whenever + * accessing the pointer. When we're in state 3, we will never move to lower + * states again + * + * FIXME 2.0: We should store this directly inside the struct, possibly + * keeping space directly allocated for a couple of parents + */ + +enum +{ + PRIV_DATA_STATE_LOCKED = 0, + PRIV_DATA_STATE_NO_PARENT = 1, + PRIV_DATA_STATE_ONE_PARENT = 2, + PRIV_DATA_STATE_PARENTS_OR_QDATA = 3, +}; + typedef struct { GQuark quark; @@ -79,7 +109,18 @@ typedef struct GDestroyNotify destroy; } GstQData; -#define QDATA(o,i) ((GstQData *)(o)->qdata)[(i)] +typedef struct +{ + /* Atomic spinlock: 1 if locked, 0 otherwise */ + gint parent_lock; + guint n_parents, n_parents_len; + GstMiniObject **parents; + + guint n_qdata, n_qdata_len; + GstQData *qdata; +} PrivData; + +#define QDATA(q,i) (q->qdata)[(i)] #define QDATA_QUARK(o,i) (QDATA(o,i).quark) #define QDATA_NOTIFY(o,i) (QDATA(o,i).notify) #define QDATA_DATA(o,i) (QDATA(o,i).data) @@ -118,8 +159,9 @@ gst_mini_object_init (GstMiniObject * mini_object, guint flags, GType type, mini_object->dispose = dispose_func; mini_object->free = free_func; - mini_object->n_qdata = 0; - mini_object->qdata = NULL; + g_atomic_int_set ((gint *) & mini_object->priv_uint, + PRIV_DATA_STATE_NO_PARENT); + mini_object->priv_pointer = NULL; GST_TRACER_MINI_OBJECT_CREATED (mini_object); } @@ -257,6 +299,32 @@ gst_mini_object_unlock (GstMiniObject * object, GstLockFlags flags) newstate)); } +/* Locks the priv pointer and sets the priv uint to PRIV_DATA_STATE_LOCKED, + * unless the full struct was already stored in the priv pointer. + * + * Returns the previous state of the priv uint + */ +static guint +lock_priv_pointer (GstMiniObject * object) +{ + gint priv_state = g_atomic_int_get ((gint *) & object->priv_uint); + + if (priv_state != PRIV_DATA_STATE_PARENTS_OR_QDATA) { + /* As long as the struct was not allocated yet and either someone else + * locked it or our priv_state is out of date, try to lock it */ + while (priv_state != PRIV_DATA_STATE_PARENTS_OR_QDATA && + (priv_state == PRIV_DATA_STATE_LOCKED || + !g_atomic_int_compare_and_exchange ((gint *) & object->priv_uint, + priv_state, PRIV_DATA_STATE_LOCKED))) + priv_state = g_atomic_int_get ((gint *) & object->priv_uint); + + /* Note that if we got the full struct, we did not store + * PRIV_DATA_STATE_LOCKED and did not actually lock the priv pointer */ + } + + return priv_state; +} + /** * gst_mini_object_is_writable: * @mini_object: the mini-object to check @@ -278,14 +346,57 @@ gboolean gst_mini_object_is_writable (const GstMiniObject * mini_object) { gboolean result; + gint priv_state; g_return_val_if_fail (mini_object != NULL, FALSE); + /* Let's first check our own writability. If this already fails there's + * no point in checking anything else */ if (GST_MINI_OBJECT_IS_LOCKABLE (mini_object)) { result = !IS_SHARED (g_atomic_int_get (&mini_object->lockstate)); } else { result = (GST_MINI_OBJECT_REFCOUNT_VALUE (mini_object) == 1); } + if (!result) + return result; + + /* We are writable ourselves, but are there parents and are they all + * writable too? */ + priv_state = lock_priv_pointer (GST_MINI_OBJECT_CAST (mini_object)); + + /* Now we either have to check the full struct and all the + * parents in there, or if there is exactly one parent we + * can check that one */ + if (priv_state == PRIV_DATA_STATE_PARENTS_OR_QDATA) { + PrivData *priv_data = mini_object->priv_pointer; + + /* Lock parents */ + while (!g_atomic_int_compare_and_exchange (&priv_data->parent_lock, 0, 1)); + + /* If we have one parent, we're only writable if that parent is writable. + * Otherwise if we have multiple parents we are not writable, and if + * we have no parent, we are writable */ + if (priv_data->n_parents == 1) + result = gst_mini_object_is_writable (priv_data->parents[0]); + else if (priv_data->n_parents == 0) + result = TRUE; + else + result = FALSE; + + /* Unlock again */ + g_atomic_int_set (&priv_data->parent_lock, 0); + } else { + if (priv_state == PRIV_DATA_STATE_ONE_PARENT) { + result = gst_mini_object_is_writable (mini_object->priv_pointer); + } else { + g_assert (priv_state == PRIV_DATA_STATE_NO_PARENT); + result = TRUE; + } + + /* Unlock again */ + g_atomic_int_set ((gint *) & mini_object->priv_uint, priv_state); + } + return result; } @@ -359,17 +470,25 @@ gst_mini_object_ref (GstMiniObject * mini_object) return mini_object; } +/* Called with global qdata lock */ static gint find_notify (GstMiniObject * object, GQuark quark, gboolean match_notify, GstMiniObjectNotify notify, gpointer data) { guint i; + gint priv_state = g_atomic_int_get ((gint *) & object->priv_uint); + PrivData *priv_data; - for (i = 0; i < object->n_qdata; i++) { - if (QDATA_QUARK (object, i) == quark) { + if (priv_state != PRIV_DATA_STATE_PARENTS_OR_QDATA) + return -1; + + priv_data = object->priv_pointer; + + for (i = 0; i < priv_data->n_qdata; i++) { + if (QDATA_QUARK (priv_data, i) == quark) { /* check if we need to match the callback too */ - if (!match_notify || (QDATA_NOTIFY (object, i) == notify && - QDATA_DATA (object, i) == data)) + if (!match_notify || (QDATA_NOTIFY (priv_data, i) == notify && + QDATA_DATA (priv_data, i) == data)) return i; } } @@ -379,42 +498,125 @@ find_notify (GstMiniObject * object, GQuark quark, gboolean match_notify, static void remove_notify (GstMiniObject * object, gint index) { + gint priv_state = g_atomic_int_get ((gint *) & object->priv_uint); + PrivData *priv_data; + + g_assert (priv_state == PRIV_DATA_STATE_PARENTS_OR_QDATA); + priv_data = object->priv_pointer; + /* remove item */ - if (--object->n_qdata == 0) { - /* we don't shrink but free when everything is gone */ - g_free (object->qdata); - object->qdata = NULL; - } else if (index != object->n_qdata) - QDATA (object, index) = QDATA (object, object->n_qdata); + priv_data->n_qdata--; + if (index != priv_data->n_qdata) { + QDATA (priv_data, index) = QDATA (priv_data, priv_data->n_qdata); + } +} + +/* Make sure we allocate the PrivData of this object if not happened yet */ +static void +ensure_priv_data (GstMiniObject * object) +{ + gint priv_state; + PrivData *priv_data; + GstMiniObject *parent = NULL; + + GST_CAT_DEBUG (GST_CAT_PERFORMANCE, + "allocating private data %s miniobject %p", + g_type_name (GST_MINI_OBJECT_TYPE (object)), object); + + priv_state = lock_priv_pointer (object); + if (priv_state == PRIV_DATA_STATE_PARENTS_OR_QDATA) + return; + + /* Now we're either locked, or someone has already allocated the struct + * before us and we can just go ahead + * + * Note: if someone else allocated it in the meantime, we don't have to + * unlock as we didn't lock! */ + if (priv_state != PRIV_DATA_STATE_PARENTS_OR_QDATA) { + if (priv_state == PRIV_DATA_STATE_ONE_PARENT) + parent = object->priv_pointer; + + object->priv_pointer = priv_data = g_new0 (PrivData, 1); + + if (parent) { + priv_data->parents = g_new (GstMiniObject *, 16); + priv_data->n_parents_len = 16; + priv_data->n_parents = 1; + priv_data->parents[0] = parent; + } + + /* Unlock */ + g_atomic_int_set ((gint *) & object->priv_uint, + PRIV_DATA_STATE_PARENTS_OR_QDATA); + } } static void set_notify (GstMiniObject * object, gint index, GQuark quark, GstMiniObjectNotify notify, gpointer data, GDestroyNotify destroy) { + PrivData *priv_data; + + ensure_priv_data (object); + priv_data = object->priv_pointer; + if (index == -1) { /* add item */ - index = object->n_qdata++; - object->qdata = - g_realloc (object->qdata, sizeof (GstQData) * object->n_qdata); + index = priv_data->n_qdata++; + if (index >= priv_data->n_qdata_len) { + priv_data->n_qdata_len *= 2; + if (priv_data->n_qdata_len == 0) + priv_data->n_qdata_len = 16; + + priv_data->qdata = + g_realloc (priv_data->qdata, + sizeof (GstQData) * priv_data->n_qdata_len); + } } - QDATA_QUARK (object, index) = quark; - QDATA_NOTIFY (object, index) = notify; - QDATA_DATA (object, index) = data; - QDATA_DESTROY (object, index) = destroy; + + QDATA_QUARK (priv_data, index) = quark; + QDATA_NOTIFY (priv_data, index) = notify; + QDATA_DATA (priv_data, index) = data; + QDATA_DESTROY (priv_data, index) = destroy; } static void -call_finalize_notify (GstMiniObject * obj) +free_priv_data (GstMiniObject * obj) { guint i; + gint priv_state = g_atomic_int_get ((gint *) & obj->priv_uint); + PrivData *priv_data; - for (i = 0; i < obj->n_qdata; i++) { - if (QDATA_QUARK (obj, i) == weak_ref_quark) - QDATA_NOTIFY (obj, i) (QDATA_DATA (obj, i), obj); - if (QDATA_DESTROY (obj, i)) - QDATA_DESTROY (obj, i) (QDATA_DATA (obj, i)); + if (priv_state != PRIV_DATA_STATE_PARENTS_OR_QDATA) { + if (priv_state == PRIV_DATA_STATE_LOCKED) { + g_warning + ("%s: object finalizing but has locked private data (object:%p)", + G_STRFUNC, obj); + } else if (priv_state == PRIV_DATA_STATE_ONE_PARENT) { + g_warning + ("%s: object finalizing but still has parent (object:%p, parent:%p)", + G_STRFUNC, obj, obj->priv_pointer); + } + + return; } + + priv_data = obj->priv_pointer; + + for (i = 0; i < priv_data->n_qdata; i++) { + if (QDATA_QUARK (priv_data, i) == weak_ref_quark) + QDATA_NOTIFY (priv_data, i) (QDATA_DATA (priv_data, i), obj); + if (QDATA_DESTROY (priv_data, i)) + QDATA_DESTROY (priv_data, i) (QDATA_DATA (priv_data, i)); + } + g_free (priv_data->qdata); + + if (priv_data->n_parents) + g_warning ("%s: object finalizing but still has %d parents (object:%p)", + G_STRFUNC, priv_data->n_parents, obj); + g_free (priv_data->parents); + + g_free (priv_data); } /** @@ -457,10 +659,8 @@ gst_mini_object_unref (GstMiniObject * mini_object) g_return_if_fail ((g_atomic_int_get (&mini_object->lockstate) & LOCK_MASK) < 4); - if (mini_object->n_qdata) { - call_finalize_notify (mini_object); - g_free (mini_object->qdata); - } + free_priv_data (mini_object); + GST_TRACER_MINI_OBJECT_DESTROYED (mini_object); if (mini_object->free) mini_object->free (mini_object); @@ -670,9 +870,10 @@ gst_mini_object_set_qdata (GstMiniObject * object, GQuark quark, G_LOCK (qdata_mutex); if ((i = find_notify (object, quark, FALSE, NULL, NULL)) != -1) { + PrivData *priv_data = object->priv_pointer; - old_data = QDATA_DATA (object, i); - old_notify = QDATA_DESTROY (object, i); + old_data = QDATA_DATA (priv_data, i); + old_notify = QDATA_DESTROY (priv_data, i); if (data == NULL) remove_notify (object, i); @@ -706,10 +907,12 @@ gst_mini_object_get_qdata (GstMiniObject * object, GQuark quark) g_return_val_if_fail (quark > 0, NULL); G_LOCK (qdata_mutex); - if ((i = find_notify (object, quark, FALSE, NULL, NULL)) != -1) - result = QDATA_DATA (object, i); - else + if ((i = find_notify (object, quark, FALSE, NULL, NULL)) != -1) { + PrivData *priv_data = object->priv_pointer; + result = QDATA_DATA (priv_data, i); + } else { result = NULL; + } G_UNLOCK (qdata_mutex); return result; @@ -738,7 +941,8 @@ gst_mini_object_steal_qdata (GstMiniObject * object, GQuark quark) G_LOCK (qdata_mutex); if ((i = find_notify (object, quark, FALSE, NULL, NULL)) != -1) { - result = QDATA_DATA (object, i); + PrivData *priv_data = object->priv_pointer; + result = QDATA_DATA (priv_data, i); remove_notify (object, i); } else { result = NULL; @@ -747,3 +951,135 @@ gst_mini_object_steal_qdata (GstMiniObject * object, GQuark quark) return result; } + +/** + * gst_mini_object_add_parent: + * @object: a #GstMiniObject + * @parent: a parent #GstMiniObject + * + * This adds @parent as a parent for @object. Having one ore more parents affects the + * writability of @object: if a @parent is not writable, @object is also not + * writable, regardless of its refcount. @object is only writable if all + * the parents are writable and its own refcount is exactly 1. + * + * Note: This function does not take ownership of @parent and also does not + * take an additional reference. It is the responsibility of the caller to + * remove the parent again at a later time. + * + * Since: 1.16 + */ +void +gst_mini_object_add_parent (GstMiniObject * object, GstMiniObject * parent) +{ + gint priv_state; + + g_return_if_fail (object != NULL); + + GST_CAT_TRACE (GST_CAT_REFCOUNTING, "adding parent %p to object %p", parent, + object); + + priv_state = lock_priv_pointer (object); + /* If we already had one parent, we need to allocate the full struct now */ + if (priv_state == PRIV_DATA_STATE_ONE_PARENT) { + /* Unlock again */ + g_atomic_int_set ((gint *) & object->priv_uint, priv_state); + + ensure_priv_data (object); + priv_state = PRIV_DATA_STATE_PARENTS_OR_QDATA; + } + + /* Now we either have to add the new parent to the full struct, or add + * our one and only parent to the pointer field */ + if (priv_state == PRIV_DATA_STATE_PARENTS_OR_QDATA) { + PrivData *priv_data = object->priv_pointer; + + /* Lock parents */ + while (!g_atomic_int_compare_and_exchange (&priv_data->parent_lock, 0, 1)); + + if (priv_data->n_parents >= priv_data->n_parents_len) { + priv_data->n_parents_len *= 2; + if (priv_data->n_parents_len == 0) + priv_data->n_parents_len = 16; + + priv_data->parents = + g_realloc (priv_data->parents, + priv_data->n_parents_len * sizeof (GstMiniObject *)); + } + priv_data->parents[priv_data->n_parents] = parent; + priv_data->n_parents++; + + /* Unlock again */ + g_atomic_int_set (&priv_data->parent_lock, 0); + } else if (priv_state == PRIV_DATA_STATE_NO_PARENT) { + object->priv_pointer = parent; + + /* Unlock again */ + g_atomic_int_set ((gint *) & object->priv_uint, PRIV_DATA_STATE_ONE_PARENT); + } else { + g_assert_not_reached (); + } +} + +/** + * gst_mini_object_remove_parent: + * @object: a #GstMiniObject + * @parent: a parent #GstMiniObject + * + * This removes @parent as a parent for @object. See + * gst_mini_object_add_parent(). + * + * Since: 1.16 + */ +void +gst_mini_object_remove_parent (GstMiniObject * object, GstMiniObject * parent) +{ + gint priv_state; + + g_return_if_fail (object != NULL); + + GST_CAT_TRACE (GST_CAT_REFCOUNTING, "removing parent %p from object %p", + parent, object); + + priv_state = lock_priv_pointer (object); + + /* Now we either have to add the new parent to the full struct, or add + * our one and only parent to the pointer field */ + if (priv_state == PRIV_DATA_STATE_PARENTS_OR_QDATA) { + PrivData *priv_data = object->priv_pointer; + guint i; + + /* Lock parents */ + while (!g_atomic_int_compare_and_exchange (&priv_data->parent_lock, 0, 1)); + + for (i = 0; i < priv_data->n_parents; i++) + if (parent == priv_data->parents[i]) + break; + + if (i != priv_data->n_parents) { + priv_data->n_parents--; + if (priv_data->n_parents != i) + priv_data->parents[i] = priv_data->parents[priv_data->n_parents]; + } else { + g_warning ("%s: couldn't find parent %p (object:%p)", G_STRFUNC, + object, parent); + } + + /* Unlock again */ + g_atomic_int_set (&priv_data->parent_lock, 0); + } else if (priv_state == PRIV_DATA_STATE_ONE_PARENT) { + if (object->priv_pointer != parent) { + g_warning ("%s: couldn't find parent %p (object:%p)", G_STRFUNC, + object, parent); + /* Unlock again */ + g_atomic_int_set ((gint *) & object->priv_uint, priv_state); + } else { + object->priv_pointer = NULL; + /* Unlock again */ + g_atomic_int_set ((gint *) & object->priv_uint, + PRIV_DATA_STATE_NO_PARENT); + } + } else { + /* Unlock again */ + g_atomic_int_set ((gint *) & object->priv_uint, PRIV_DATA_STATE_NO_PARENT); + } +} diff --git a/gst/gstminiobject.h b/gst/gstminiobject.h index f7aa87a00e..736e7b4472 100644 --- a/gst/gstminiobject.h +++ b/gst/gstminiobject.h @@ -213,9 +213,9 @@ struct _GstMiniObject { GstMiniObjectFreeFunction free; /* < private > */ - /* Used to keep track of weak ref notifies and qdata */ - guint n_qdata; - gpointer qdata; + /* Used to keep track of parents, weak ref notifies and qdata */ + guint priv_uint; + gpointer priv_pointer; }; GST_API @@ -272,6 +272,11 @@ gpointer gst_mini_object_get_qdata (GstMiniObject *object, GQuark q GST_API gpointer gst_mini_object_steal_qdata (GstMiniObject *object, GQuark quark); +GST_API +void gst_mini_object_add_parent (GstMiniObject *object, GstMiniObject *parent); +GST_API +void gst_mini_object_remove_parent (GstMiniObject *object, GstMiniObject *parent); + GST_API gboolean gst_mini_object_replace (GstMiniObject **olddata, GstMiniObject *newdata); diff --git a/gst/gstsample.c b/gst/gstsample.c index 016ac5e674..eca4683b9e 100644 --- a/gst/gstsample.c +++ b/gst/gstsample.c @@ -66,9 +66,11 @@ _gst_sample_copy (GstSample * sample) copy = gst_sample_new (sample->buffer, sample->caps, &sample->segment, (sample->info) ? gst_structure_copy (sample->info) : NULL); - if (sample->buffer_list) - copy->buffer_list = (GstBufferList *) - gst_mini_object_ref (GST_MINI_OBJECT_CAST (sample->buffer_list)); + if (sample->buffer_list) { + copy->buffer_list = gst_buffer_list_ref (sample->buffer_list); + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (copy->buffer_list), + GST_MINI_OBJECT_CAST (copy)); + } return copy; } @@ -78,16 +80,27 @@ _gst_sample_free (GstSample * sample) { GST_LOG ("free %p", sample); - if (sample->buffer) + if (sample->buffer) { + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (sample->buffer), + GST_MINI_OBJECT_CAST (sample)); gst_buffer_unref (sample->buffer); - if (sample->caps) + } + + if (sample->caps) { + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (sample->caps), + GST_MINI_OBJECT_CAST (sample)); gst_caps_unref (sample->caps); + } + if (sample->info) { gst_structure_set_parent_refcount (sample->info, NULL); gst_structure_free (sample->info); } - if (sample->buffer_list) - gst_mini_object_unref (GST_MINI_OBJECT_CAST (sample->buffer_list)); + if (sample->buffer_list) { + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (sample->buffer_list), + GST_MINI_OBJECT_CAST (sample)); + gst_buffer_list_unref (sample->buffer_list); + } g_slice_free1 (sizeof (GstSample), sample); } @@ -120,8 +133,17 @@ gst_sample_new (GstBuffer * buffer, GstCaps * caps, const GstSegment * segment, (GstMiniObjectCopyFunction) _gst_sample_copy, NULL, (GstMiniObjectFreeFunction) _gst_sample_free); - sample->buffer = buffer ? gst_buffer_ref (buffer) : NULL; - sample->caps = caps ? gst_caps_ref (caps) : NULL; + if (buffer) { + sample->buffer = gst_buffer_ref (buffer); + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (sample->buffer), + GST_MINI_OBJECT_CAST (sample)); + } + + if (caps) { + sample->caps = gst_caps_ref (caps); + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (sample->caps), + GST_MINI_OBJECT_CAST (sample)); + } /* FIXME 2.0: initialize with GST_FORMAT_UNDEFINED by default */ if (segment) @@ -257,11 +279,21 @@ gst_sample_set_buffer_list (GstSample * sample, GstBufferList * buffer_list) g_return_if_fail (gst_sample_is_writable (sample)); old = sample->buffer_list; - sample->buffer_list = buffer_list ? (GstBufferList *) - gst_mini_object_ref (GST_MINI_OBJECT_CAST (buffer_list)) : NULL; - if (old) - gst_mini_object_unref (GST_MINI_OBJECT_CAST (old)); + if (old == buffer_list) + return; + + if (buffer_list) { + sample->buffer_list = gst_buffer_list_ref (buffer_list); + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (sample->buffer_list), + GST_MINI_OBJECT_CAST (sample)); + } + + if (old) { + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (old), + GST_MINI_OBJECT_CAST (sample)); + gst_buffer_list_unref (old); + } } /** @@ -276,10 +308,27 @@ gst_sample_set_buffer_list (GstSample * sample, GstBufferList * buffer_list) void gst_sample_set_buffer (GstSample * sample, GstBuffer * buffer) { + GstBuffer *old = NULL; + g_return_if_fail (GST_IS_SAMPLE (sample)); g_return_if_fail (gst_sample_is_writable (sample)); - gst_buffer_replace (&sample->buffer, buffer); + old = sample->buffer; + + if (old == buffer) + return; + + if (buffer) { + sample->buffer = gst_buffer_ref (buffer); + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (sample->buffer), + GST_MINI_OBJECT_CAST (sample)); + } + + if (old) { + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (old), + GST_MINI_OBJECT_CAST (sample)); + gst_buffer_unref (old); + } } /** @@ -294,10 +343,27 @@ gst_sample_set_buffer (GstSample * sample, GstBuffer * buffer) void gst_sample_set_caps (GstSample * sample, GstCaps * caps) { + GstCaps *old = NULL; + g_return_if_fail (GST_IS_SAMPLE (sample)); g_return_if_fail (gst_sample_is_writable (sample)); - gst_caps_replace (&sample->caps, caps); + old = sample->caps; + + if (old == caps) + return; + + if (caps) { + sample->caps = gst_caps_ref (caps); + gst_mini_object_add_parent (GST_MINI_OBJECT_CAST (sample->caps), + GST_MINI_OBJECT_CAST (sample)); + } + + if (old) { + gst_mini_object_remove_parent (GST_MINI_OBJECT_CAST (old), + GST_MINI_OBJECT_CAST (sample)); + gst_caps_unref (old); + } } /** diff --git a/win32/common/libgstreamer.def b/win32/common/libgstreamer.def index 62a8ece971..1823fba4eb 100644 --- a/win32/common/libgstreamer.def +++ b/win32/common/libgstreamer.def @@ -835,6 +835,7 @@ EXPORTS gst_meta_flags_get_type gst_meta_get_info gst_meta_register + gst_mini_object_add_parent gst_mini_object_copy gst_mini_object_flags_get_type gst_mini_object_get_qdata @@ -843,6 +844,7 @@ EXPORTS gst_mini_object_lock gst_mini_object_make_writable gst_mini_object_ref + gst_mini_object_remove_parent gst_mini_object_replace gst_mini_object_set_qdata gst_mini_object_steal