taglist: make GstTagList a GstMiniObject

Which adds refcounting support, and other things.

docs/gst/gstreamer-sections.txt

@@ -2532,13 +2532,15 @@ gst_tag_list_new_empty
 gst_tag_list_new_valist
 gst_tag_list_new_from_string
 gst_tag_list_to_string
-gst_is_tag_list
 gst_tag_list_is_empty
 gst_tag_list_is_equal
 gst_tag_list_copy
+gst_tag_list_ref
+gst_tag_list_unref
+gst_tag_list_is_writable
+gst_tag_list_make_writable
 gst_tag_list_insert
 gst_tag_list_merge
-gst_tag_list_free
 gst_tag_list_get_tag_size
 gst_tag_list_n_tags
 gst_tag_list_nth_tag_name

docs/random/porting-to-0.11.txt

@@ -453,12 +453,13 @@ The 0.11 porting guide
     implementations no longer need to check that as well.
 
 * GstTagList
-    is now an opaque object instead of being typedefed to a GstStructure. Cast
+    is now an opaque mini object instead of being typedefed to a GstStructure.
-    to GstStructure or use gst_structure_* API on it at your own peril (it may
+    Cast to GstStructure or use gst_structure_* API on it at your own peril
-    still work for now, but might be changed in future).
+    (it might crash if you do that)
 
     gst_tag_list_new() has been renamed to gst_tag_list_new_empty().
     gst_tag_list_new_full*() have been renamed to gst_tag_list_new*().
+    gst_tag_list_free() has been replaced by gst_tag_list_unref().
 
 * GstController:
     has now been merged into GstObject. It does not exists as a individual

gst/gstcompat.h

@@ -56,6 +56,8 @@ G_BEGIN_DECLS
 
 #define gst_adapter_prev_timestamp    gst_adapter_prev_pts
 
+#define gst_tag_list_free(taglist)    gst_tag_list_unref(taglist)
+
 #ifndef GST_DISABLE_DEPRECATED
 
 #endif /* not GST_DISABLE_DEPRECATED */

gst/gsttaglist.c

@@ -47,15 +47,22 @@
 #include <gobject/gvaluecollector.h>
 #include <string.h>
 
+/* FIXME: add category for tags */
+#define GST_CAT_TAGS GST_CAT_DEFAULT
+
 #define GST_TAG_IS_VALID(tag)           (gst_tag_get_info (tag) != NULL)
 
 /* FIXME 0.11: make taglists refcounted maybe? */
 /* a tag list is basically a structure, but we don't make this fact public */
-struct _GstTagList
+typedef struct _GstTagListImpl
 {
-  GType type;
+  GstTagList taglist;
+
   GstStructure *structure;
-};
+} GstTagListImpl;
+
+#define GST_TAG_LIST_STRUCTURE(taglist) ((GstTagListImpl*)(taglist))->structure
+
 
 /* FIXME 0.11: use GParamSpecs or something similar for tag registrations,
  * possibly even gst_tag_register(). Especially value ranges might be
@@ -83,8 +90,10 @@ static GMutex __tag_mutex;
 /* tags hash table: maps tag name string => GstTagInfo */
 static GHashTable *__tags;
 
-G_DEFINE_BOXED_TYPE (GstTagList, gst_tag_list,
+GST_DEFINE_MINI_OBJECT_TYPE (GstTagList, gst_tag_list);
-    (GBoxedCopyFunc) gst_tag_list_copy, (GBoxedFreeFunc) gst_tag_list_free);
+
+static void __gst_tag_list_free (GstTagList * list);
+static GstTagList *__gst_tag_list_copy (const GstTagList * list);
 
 /* FIXME: had code:
  *    g_value_register_transform_func (_gst_tag_list_type, G_TYPE_STRING,
@@ -622,6 +631,17 @@ gst_tag_is_fixed (const gchar * tag)
   return info->merge_func == NULL;
 }
 
+static void
+gst_tag_list_init (GstTagList * taglist, gsize size)
+{
+  gst_mini_object_init (GST_MINI_OBJECT_CAST (taglist),
+      gst_tag_list_get_type (), size);
+
+  taglist->mini_object.copy = (GstMiniObjectCopyFunction) __gst_tag_list_copy;
+  taglist->mini_object.dispose = NULL;
+  taglist->mini_object.free = (GstMiniObjectFreeFunction) __gst_tag_list_free;
+}
+
 /* takes ownership of the structure */
 static GstTagList *
 gst_tag_list_new_internal (GstStructure * s)
@@ -630,18 +650,51 @@ gst_tag_list_new_internal (GstStructure * s)
 
   g_assert (s != NULL);
 
-  tag_list = g_slice_new0 (GstTagList);
+  tag_list = (GstTagList *) g_slice_new (GstTagListImpl);
-  tag_list->type = GST_TYPE_TAG_LIST;
+
-  tag_list->structure = s;
+  gst_tag_list_init (tag_list, sizeof (GstTagListImpl));
+
+  GST_TAG_LIST_STRUCTURE (tag_list) = s;
+
+#ifdef DEBUG_REFCOUNT
+  GST_CAT_TRACE (GST_CAT_TAGS, "created taglist %p", tag_list);
+#endif
+
   return tag_list;
 }
 
+static void
+__gst_tag_list_free (GstTagList * list)
+{
+  g_return_if_fail (GST_IS_TAG_LIST (list));
+
+#ifdef DEBUG_REFCOUNT
+  GST_CAT_TRACE (GST_CAT_TAGS, "freeing caps %p", list);
+#endif
+
+  gst_structure_free (GST_TAG_LIST_STRUCTURE (list));
+
+  /* why not just pass sizeof (GstTagListImpl) here? */
+  g_slice_free1 (GST_MINI_OBJECT_SIZE (list), list);
+}
+
+static GstTagList *
+__gst_tag_list_copy (const GstTagList * list)
+{
+  const GstStructure *s;
+
+  g_return_val_if_fail (GST_IS_TAG_LIST (list), NULL);
+
+  s = GST_TAG_LIST_STRUCTURE (list);
+  return gst_tag_list_new_internal (gst_structure_copy (s));
+}
+
 /**
  * gst_tag_list_new_empty:
  *
  * Creates a new empty GstTagList.
  *
- * Free-function: gst_tag_list_free
+ * Free-function: gst_tag_list_unref
  *
  * Returns: (transfer full): An empty tag list
  */
@@ -669,9 +722,9 @@ gst_tag_list_new_empty (void)
  * function. The tag list will make copies of any arguments passed
  * (e.g. strings, buffers).
  *
- * Free-function: gst_tag_list_free
+ * Free-function: gst_tag_list_unref
  *
- * Returns: (transfer full): a new #GstTagList. Free with gst_tag_list_free()
+ * Returns: (transfer full): a new #GstTagList. Free with gst_tag_list_unref()
  *     when no longer needed.
  *
  * Since: 0.10.24
@@ -699,9 +752,9 @@ gst_tag_list_new (const gchar * tag, ...)
  * Just like gst_tag_list_new(), only that it takes a va_list argument.
  * Useful mostly for language bindings.
  *
- * Free-function: gst_tag_list_free
+ * Free-function: gst_tag_list_unref
  *
- * Returns: (transfer full): a new #GstTagList. Free with gst_tag_list_free()
+ * Returns: (transfer full): a new #GstTagList. Free with gst_tag_list_unref()
  *     when no longer needed.
  *
  * Since: 0.10.24
@@ -736,7 +789,7 @@ gst_tag_list_to_string (const GstTagList * list)
 {
   g_return_val_if_fail (GST_IS_TAG_LIST (list), NULL);
 
-  return gst_structure_to_string (list->structure);
+  return gst_structure_to_string (GST_TAG_LIST_STRUCTURE (list));
 }
 
 /**
@@ -775,7 +828,7 @@ gst_tag_list_n_tags (const GstTagList * list)
   g_return_val_if_fail (list != NULL, 0);
   g_return_val_if_fail (GST_IS_TAG_LIST (list), 0);
 
-  return gst_structure_n_fields (list->structure);
+  return gst_structure_n_fields (GST_TAG_LIST_STRUCTURE (list));
 }
 
 /**
@@ -793,7 +846,7 @@ gst_tag_list_nth_tag_name (const GstTagList * list, guint index)
   g_return_val_if_fail (list != NULL, 0);
   g_return_val_if_fail (GST_IS_TAG_LIST (list), 0);
 
-  return gst_structure_nth_field_name (list->structure, index);
+  return gst_structure_nth_field_name (GST_TAG_LIST_STRUCTURE (list), index);
 }
 
 /**
@@ -812,7 +865,7 @@ gst_tag_list_is_empty (const GstTagList * list)
   g_return_val_if_fail (list != NULL, FALSE);
   g_return_val_if_fail (GST_IS_TAG_LIST (list), FALSE);
 
-  return (gst_structure_n_fields (list->structure) == 0);
+  return (gst_structure_n_fields (GST_TAG_LIST_STRUCTURE (list)) == 0);
 }
 
 static gboolean
@@ -859,8 +912,8 @@ gst_tag_list_is_equal (const GstTagList * list1, const GstTagList * list2)
   /* we don't just use gst_structure_is_equal() here so we can add some
    * tolerance for doubles, though maybe we should just add that to
    * gst_value_compare_double() as well? */
-  s1 = list1->structure;
+  s1 = GST_TAG_LIST_STRUCTURE (list1);
-  s2 = list2->structure;
+  s2 = GST_TAG_LIST_STRUCTURE (list2);
 
   num_fields1 = gst_structure_n_fields (s1);
   num_fields2 = gst_structure_n_fields (s2);
@@ -886,22 +939,6 @@ gst_tag_list_is_equal (const GstTagList * list1, const GstTagList * list2)
   return TRUE;
 }
 
-/**
- * gst_is_tag_list:
- * @taglist: Object that might be a taglist
- *
- * Checks if the given pointer is a taglist.
- *
- * Returns: TRUE, if the given pointer is a taglist
- */
-gboolean
-gst_is_tag_list (GstTagList * taglist)
-{
-  g_return_val_if_fail (taglist != NULL, FALSE);
-
-  return taglist->type == GST_TYPE_TAG_LIST;
-}
-
 typedef struct
 {
   GstTagList *list;
@@ -913,7 +950,7 @@ static void
 gst_tag_list_add_value_internal (GstTagList * tag_list, GstTagMergeMode mode,
     const gchar * tag, const GValue * value, GstTagInfo * info)
 {
-  GstStructure *list = tag_list->structure;
+  GstStructure *list = GST_TAG_LIST_STRUCTURE (tag_list);
   const GValue *value2;
   GQuark tag_quark;
 
@@ -1008,28 +1045,10 @@ gst_tag_list_insert (GstTagList * into, const GstTagList * from,
   data.list = into;
   data.mode = mode;
   if (mode == GST_TAG_MERGE_REPLACE_ALL) {
-    gst_structure_remove_all_fields (into->structure);
+    gst_structure_remove_all_fields (GST_TAG_LIST_STRUCTURE (into));
   }
-  gst_structure_foreach (from->structure, gst_tag_list_copy_foreach, &data);
+  gst_structure_foreach (GST_TAG_LIST_STRUCTURE (from),
-}
+      gst_tag_list_copy_foreach, &data);
-
-/**
- * gst_tag_list_copy:
- * @list: list to copy
- *
- * Copies a given #GstTagList.
- *
- * Free-function: gst_tag_list_free
- *
- * Returns: (transfer full): copy of the given list
- */
-GstTagList *
-gst_tag_list_copy (const GstTagList * list)
-{
-  g_return_val_if_fail (GST_IS_TAG_LIST (list), NULL);
-
-
-  return gst_tag_list_new_internal (gst_structure_copy (list->structure));
 }
 
 /**
@@ -1041,7 +1060,7 @@ gst_tag_list_copy (const GstTagList * list)
  * Merges the two given lists into a new list. If one of the lists is NULL, a
  * copy of the other is returned. If both lists are NULL, NULL is returned.
  *
- * Free-function: gst_tag_list_free
+ * Free-function: gst_tag_list_unref
  *
  * Returns: (transfer full): the new list
  */
@@ -1068,25 +1087,11 @@ gst_tag_list_merge (const GstTagList * list1, const GstTagList * list2,
   gst_tag_list_insert (list1_cp, list2_cp, mode);
 
   if (!list2)
-    gst_tag_list_free ((GstTagList *) list2_cp);
+    gst_tag_list_unref ((GstTagList *) list2_cp);
 
   return list1_cp;
 }
 
-/**
- * gst_tag_list_free:
- * @list: (in) (transfer full): the list to free
- *
- * Frees the given list and all associated values.
- */
-void
-gst_tag_list_free (GstTagList * list)
-{
-  g_return_if_fail (GST_IS_TAG_LIST (list));
-  gst_structure_free (list->structure);
-  g_slice_free (GstTagList, list);
-}
-
 /**
  * gst_tag_list_get_tag_size:
  * @list: a taglist
@@ -1103,7 +1108,7 @@ gst_tag_list_get_tag_size (const GstTagList * list, const gchar * tag)
 
   g_return_val_if_fail (GST_IS_TAG_LIST (list), 0);
 
-  value = gst_structure_get_value (list->structure, tag);
+  value = gst_structure_get_value (GST_TAG_LIST_STRUCTURE (list), tag);
   if (value == NULL)
     return 0;
   if (G_VALUE_TYPE (value) != GST_TYPE_LIST)
@@ -1181,7 +1186,7 @@ gst_tag_list_add_valist (GstTagList * list, GstTagMergeMode mode,
   g_return_if_fail (tag != NULL);
 
   if (mode == GST_TAG_MERGE_REPLACE_ALL) {
-    gst_structure_remove_all_fields (list->structure);
+    gst_structure_remove_all_fields (GST_TAG_LIST_STRUCTURE (list));
   }
 
   while (tag != NULL) {
@@ -1225,7 +1230,7 @@ gst_tag_list_add_valist_values (GstTagList * list, GstTagMergeMode mode,
   g_return_if_fail (tag != NULL);
 
   if (mode == GST_TAG_MERGE_REPLACE_ALL) {
-    gst_structure_remove_all_fields (list->structure);
+    gst_structure_remove_all_fields (GST_TAG_LIST_STRUCTURE (list));
   }
 
   while (tag != NULL) {
@@ -1277,7 +1282,7 @@ gst_tag_list_remove_tag (GstTagList * list, const gchar * tag)
   g_return_if_fail (GST_IS_TAG_LIST (list));
   g_return_if_fail (tag != NULL);
 
-  gst_structure_remove_field (list->structure, tag);
+  gst_structure_remove_field (GST_TAG_LIST_STRUCTURE (list), tag);
 }
 
 typedef struct
@@ -1319,7 +1324,8 @@ gst_tag_list_foreach (const GstTagList * list, GstTagForeachFunc func,
   data.func = func;
   data.tag_list = list;
   data.data = user_data;
-  gst_structure_foreach (list->structure, structure_foreach_wrapper, &data);
+  gst_structure_foreach (GST_TAG_LIST_STRUCTURE (list),
+      structure_foreach_wrapper, &data);
 }
 
 /**
@@ -1343,7 +1349,7 @@ gst_tag_list_get_value_index (const GstTagList * list, const gchar * tag,
   g_return_val_if_fail (GST_IS_TAG_LIST (list), NULL);
   g_return_val_if_fail (tag != NULL, NULL);
 
-  value = gst_structure_get_value (list->structure, tag);
+  value = gst_structure_get_value (GST_TAG_LIST_STRUCTURE (list), tag);
   if (value == NULL)
     return NULL;
 
@@ -1383,7 +1389,7 @@ gst_tag_list_copy_value (GValue * dest, const GstTagList * list,
   g_return_val_if_fail (dest != NULL, FALSE);
   g_return_val_if_fail (G_VALUE_TYPE (dest) == 0, FALSE);
 
-  src = gst_structure_get_value (list->structure, tag);
+  src = gst_structure_get_value (GST_TAG_LIST_STRUCTURE (list), tag);
   if (!src)
     return FALSE;
 

gst/gsttaglist.h

@@ -144,14 +144,18 @@ typedef enum {
 
 /**
  * GstTagList:
+ * @mini_object: the parent type
  *
- * Opaque #GstTagList data structure.
+ * Object describing tags / metadata.
  */
 typedef struct _GstTagList GstTagList;
+struct _GstTagList {
+  GstMiniObject mini_object;
+};
 
 #define GST_TAG_LIST(x)       ((GstTagList *) (x))
-#define GST_IS_TAG_LIST(x)    ((x) != NULL && gst_is_tag_list (GST_TAG_LIST (x)))
 #define GST_TYPE_TAG_LIST     (gst_tag_list_get_type ())
+#define GST_IS_TAG_LIST(obj)  (GST_IS_MINI_OBJECT_TYPE((obj), GST_TYPE_TAG_LIST))
 
 /**
  * GstTagForeachFunc:
@@ -206,8 +210,6 @@ GstTagList * gst_tag_list_new_valist        (va_list var_args) G_GNUC_MALLOC;
 gchar      * gst_tag_list_to_string         (const GstTagList * list) G_GNUC_MALLOC;
 GstTagList * gst_tag_list_new_from_string   (const gchar      * str) G_GNUC_MALLOC;
 
-gboolean     gst_is_tag_list                (GstTagList * taglist);
-GstTagList * gst_tag_list_copy              (const GstTagList * list) G_GNUC_MALLOC;
 gint         gst_tag_list_n_tags            (const GstTagList * list);
 const gchar* gst_tag_list_nth_tag_name      (const GstTagList * list, guint index);
 gboolean     gst_tag_list_is_empty          (const GstTagList * list);
@@ -219,7 +221,6 @@ void         gst_tag_list_insert            (GstTagList       * into,
 GstTagList * gst_tag_list_merge             (const GstTagList * list1,
                                              const GstTagList * list2,
                                              GstTagMergeMode    mode) G_GNUC_MALLOC;
-void         gst_tag_list_free              (GstTagList       * list);
 guint        gst_tag_list_get_tag_size      (const GstTagList * list,
                                              const gchar      * tag);
 void         gst_tag_list_add               (GstTagList       * list,
@@ -346,6 +347,104 @@ gboolean     gst_tag_list_get_buffer_index  (const GstTagList * list,
                                              guint              index,
                                              GstBuffer       ** value);
 
+/* refcounting */
+/**
+ * gst_tag_list_ref:
+ * @taglist: the #GstTagList to reference
+ *
+ * Add a reference to a #GstTagList mini object.
+ *
+ * From this point on, until the caller calls gst_tag_list_unref() or
+ * gst_tag_list_make_writable(), it is guaranteed that the taglist object will
+ * not change. To use a #GstTagList object, you must always have a refcount on
+ * it -- either the one made implicitly by e.g. gst_tag_list_new(), or via
+ * taking one explicitly with this function.
+ *
+ * Returns: the same #GstTagList mini object.
+ */
+#ifdef _FOOL_GTK_DOC_
+G_INLINE_FUNC GstTagList * gst_tag_list_ref (GstTagList * taglist);
+#endif
+
+static inline GstTagList *
+gst_tag_list_ref (GstTagList * taglist)
+{
+  return (GstTagList *) gst_mini_object_ref (GST_MINI_OBJECT_CAST (taglist));
+}
+
+/**
+ * gst_tag_list_unref:
+ * @taglist: a #GstTagList.
+ *
+ * Unref a #GstTagList, and and free all its memory when the refcount reaches 0.
+ */
+#ifdef _FOOL_GTK_DOC_
+G_INLINE_FUNC void gst_tag_list_unref (GstTagList * taglist);
+#endif
+
+static inline void
+gst_tag_list_unref (GstTagList * taglist)
+{
+  gst_mini_object_unref (GST_MINI_OBJECT_CAST (taglist));
+}
+
+/**
+ * gst_tag_list_copy:
+ * @taglist: a #GstTagList.
+ *
+ * Creates a new #GstTagList as a copy of the old @taglist. The new taglist
+ * will have a refcount of 1, owned by the caller, and will be writable as
+ * a result.
+ *
+ * Note that this function is the semantic equivalent of a gst_tag_list_ref()
+ * followed by a gst_tag_list_make_writable(). If you only want to hold on to a
+ * reference to the data, you should use gst_tag_list_ref().
+ *
+ * When you are finished with the taglist, call gst_tag_list_unref() on it.
+ *
+ * Returns: the new #GstTagList
+ */
+#ifdef _FOOL_GTK_DOC_
+G_INLINE_FUNC GstTagList * gst_tag_list_copy (const GstTagList * taglist);
+#endif
+
+static inline GstTagList *
+gst_tag_list_copy (const GstTagList * taglist)
+{
+  return GST_TAG_LIST (gst_mini_object_copy (GST_MINI_OBJECT_CAST (taglist)));
+}
+
+/**
+ * gst_tag_list_is_writable:
+ * @taglist: a #GstTagList
+ *
+ * Tests if you can safely modify @taglist. It is only safe to modify taglist
+ * when there is only one owner of the taglist - ie, the refcount is 1.
+ */
+#define gst_tag_list_is_writable(taglist)    gst_mini_object_is_writable (GST_MINI_OBJECT_CAST (taglist))
+
+/**
+ * gst_tag_list_make_writable:
+ * @taglist: (transfer full): a #GstTagList
+ *
+ * Returns a writable copy of @taglist.
+ *
+ * If there is only one reference count on @taglist, the caller must be the
+ * owner, and so this function will return the taglist object unchanged. If on
+ * the other hand there is more than one reference on the object, a new taglist
+ * object will be returned (which will be a copy of @taglist). The caller's
+ * reference on @taglist will be removed, and instead the caller will own a
+ * reference to the returned object.
+ *
+ * In short, this function unrefs the taglist in the argument and refs the
+ * taglist that it returns. Don't access the argument after calling this
+ * function. See also: gst_tag_list_ref().
+ *
+ * Returns: (transfer full): a writable taglist which may or may not be the
+ *     same as @taglist
+ */
+#define gst_tag_list_make_writable(taglist)   GST_TAG_LIST (gst_mini_object_make_writable (GST_MINI_OBJECT_CAST (taglist)))
+
 /* GStreamer core tags */
 /**
  * GST_TAG_TITLE:
@@ -723,7 +822,7 @@ gboolean     gst_tag_list_get_buffer_index  (const GstTagList * list,
 /**
  * GST_TAG_IMAGE:
  *
- * image (sample) (sample caps should specify the content type and preferably
+ * image (sample) (sample taglist should specify the content type and preferably
  * also set "image-type" field as #GstTagImageType)
  *
  * Since: 0.10.6
@@ -733,7 +832,7 @@ gboolean     gst_tag_list_get_buffer_index  (const GstTagList * list,
  * GST_TAG_PREVIEW_IMAGE:
  *
  * image that is meant for preview purposes, e.g. small icon-sized version
- * (sample) (sample caps should specify the content type)
+ * (sample) (sample taglist should specify the content type)
  *
  * Since: 0.10.7
  */
@@ -742,7 +841,7 @@ gboolean     gst_tag_list_get_buffer_index  (const GstTagList * list,
 /**
  * GST_TAG_ATTACHMENT:
  *
- * generic file attachment (sample) (sample caps should specify the content
+ * generic file attachment (sample) (sample taglist should specify the content
  * type and if possible set "filename" to the file name of the
  * attachment)
  *

tests/check/gst/gsttag.c

@@ -306,13 +306,9 @@ GST_START_TEST (test_type)
 
   taglist = gst_tag_list_new_empty ();
   fail_unless (GST_IS_TAG_LIST (taglist));
-  fail_unless (gst_is_tag_list (taglist));
   gst_tag_list_free (taglist);
 
-  /* this isn't okay */
+  /* this should be fine */
-  ASSERT_CRITICAL (fail_if (gst_is_tag_list (NULL)));
-
-  /* this however should be fine */
   fail_if (GST_IS_TAG_LIST (NULL));
 
   /* check gst_tag_list_is_empty */

win32/common/libgstreamer.def

@@ -484,7 +484,6 @@ EXPORTS
 	gst_int64_range_get_type
 	gst_int_range_get_type
 	gst_is_initialized
-	gst_is_tag_list
 	gst_iterator_copy
 	gst_iterator_filter
 	gst_iterator_find_custom
@@ -1043,10 +1042,8 @@ EXPORTS
 	gst_tag_list_add_valist_values
 	gst_tag_list_add_value
 	gst_tag_list_add_values
-	gst_tag_list_copy
 	gst_tag_list_copy_value
 	gst_tag_list_foreach
-	gst_tag_list_free
 	gst_tag_list_get_boolean
 	gst_tag_list_get_boolean_index
 	gst_tag_list_get_buffer