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

@@ -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

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);
   }

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];
+  /* We have to implement this manually here to correctly add/remove the
-  return (*p_buf = gst_buffer_make_writable (*p_buf));
+   * 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));
 }
 
 /**

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 is exactly 1 and there is no parent or a single parent exists and is
- * object. gst_mini_object_make_writable() will return a writable version of the
+ * writable itself, meaning the current caller has the only reference to the
- * object, which might be a new copy when the refcount was not 1.
+ * 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;
+  g_atomic_int_set ((gint *) & mini_object->priv_uint,
-  mini_object->qdata = NULL;
+      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 (priv_state != PRIV_DATA_STATE_PARENTS_OR_QDATA)
-    if (QDATA_QUARK (object, i) == quark) {
+    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 &&
+      if (!match_notify || (QDATA_NOTIFY (priv_data, i) == notify &&
-              QDATA_DATA (object, i) == data))
+              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) {
+  priv_data->n_qdata--;
-    /* we don't shrink but free when everything is gone */
+  if (index != priv_data->n_qdata) {
-    g_free (object->qdata);
+    QDATA (priv_data, index) = QDATA (priv_data, priv_data->n_qdata);
-    object->qdata = NULL;
+  }
-  } else if (index != object->n_qdata)
+}
-    QDATA (object, index) = QDATA (object, object->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++;
+    index = priv_data->n_qdata++;
-    object->qdata =
+    if (index >= priv_data->n_qdata_len) {
-        g_realloc (object->qdata, sizeof (GstQData) * object->n_qdata);
+      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_QUARK (priv_data, index) = quark;
-  QDATA_DATA (object, index) = data;
+  QDATA_NOTIFY (priv_data, index) = notify;
-  QDATA_DESTROY (object, index) = destroy;
+  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 (priv_state != PRIV_DATA_STATE_PARENTS_OR_QDATA) {
-    if (QDATA_QUARK (obj, i) == weak_ref_quark)
+    if (priv_state == PRIV_DATA_STATE_LOCKED) {
-      QDATA_NOTIFY (obj, i) (QDATA_DATA (obj, i), obj);
+      g_warning
-    if (QDATA_DESTROY (obj, i))
+          ("%s: object finalizing but has locked private data (object:%p)",
-      QDATA_DESTROY (obj, i) (QDATA_DATA (obj, i));
+          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) {
+      free_priv_data (mini_object);
-        call_finalize_notify (mini_object);
+
-        g_free (mini_object->qdata);
-      }
       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_data = QDATA_DATA (priv_data, i);
-    old_notify = QDATA_DESTROY (object, 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)
+  if ((i = find_notify (object, quark, FALSE, NULL, NULL)) != -1) {
-    result = QDATA_DATA (object, i);
+    PrivData *priv_data = object->priv_pointer;
-  else
+    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);
+  }
+}

gst/gstminiobject.h

@@ -213,9 +213,9 @@ struct _GstMiniObject {
   GstMiniObjectFreeFunction free;
 
   /* < private > */
-  /* Used to keep track of weak ref notifies and qdata */
+  /* Used to keep track of parents, weak ref notifies and qdata */
-  guint n_qdata;
+  guint priv_uint;
-  gpointer qdata;
+  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);
 

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)
+  if (sample->buffer_list) {
-    copy->buffer_list = (GstBufferList *)
+    copy->buffer_list = gst_buffer_list_ref (sample->buffer_list);
-        gst_mini_object_ref (GST_MINI_OBJECT_CAST (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)
+  if (sample->buffer_list) {
-    gst_mini_object_unref (GST_MINI_OBJECT_CAST (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;
+  if (buffer) {
-  sample->caps = caps ? gst_caps_ref (caps) : NULL;
+    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)
+  if (old == buffer_list)
-    gst_mini_object_unref (GST_MINI_OBJECT_CAST (old));
+    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);
+  }
 }
 
 /**

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