buffer: store sequence number for metas

For metas where order might be significant if multiple metas are
attached to the same buffer, so store a sequence number with the
meta when adding it to the buffer. This allows users of the meta
to make sure metas are processed in the right order.

We need a 64-bit integer for the sequence number here in the API,
a 32-bit one might overflow too easily with high packet/buffer
rates. We could do it rtp-seqnum style of course, but that's a
bit of a pain.

We could also make it so that gst_buffer_add_meta() just keeps metas in
order or rely on the order we add the metas in, but that seems too
fragile overall, when buffers (incl. metas) get merged or split.

Also add a compare function for easier sorting.

We store the seqnum in the MetaItem struct here and not in the
GstMeta struct since there's no padding in the GstMeta struct.
We could add a private struct to GstMeta before the start of
GstMeta, but that's what MetaItem effectively is implementation-
wise. We can still change this later if we want, since it's all
private.

Fixes #262

diff --git a/docs/gst/gstreamer-sections.txt b/docs/gst/gstreamer-sections.txt
index a32b06b..9d172ae 100644 (file)
--- a/docs/gst/gstreamer-sections.txt
+++ b/docs/gst/gstreamer-sections.txt
@@ -322,6 +322,8 @@ GST_META_TAG_MEMORY
 GST_META_TAG_MEMORY_STR
 gst_meta_register
 gst_meta_get_info
+gst_meta_get_seqnum
+gst_meta_compare_seqnum
 <SUBSECTION Standard>
 GST_META_CAST
 <SUBSECTION Private>

diff --git a/gst/gst_private.h b/gst/gst_private.h
index 2a2e8fa..0edc93c 100644 (file)
--- a/gst/gst_private.h
+++ b/gst/gst_private.h
@@ -92,6 +92,14 @@ struct _GstPluginPrivate {
   GstStructure *cache_data;
 };
 
+/* Needed by GstMeta (to access meta seq) and GstBuffer (create/free/iterate) */
+typedef struct _GstMetaItem GstMetaItem;
+struct _GstMetaItem {
+  GstMetaItem *next;
+  guint64 seq_num;
+  GstMeta meta;
+};
+
 /* FIXME: could rename all priv_gst_* functions to __gst_* now */
 G_GNUC_INTERNAL  gboolean priv_gst_plugin_loading_have_whitelist (void);
 

diff --git a/gst/gstbuffer.c b/gst/gstbuffer.c
index 696eddf..1e79acf 100644 (file)
--- a/gst/gstbuffer.c
+++ b/gst/gstbuffer.c
@@ -133,14 +133,6 @@
 
 GType _gst_buffer_type = 0;
 
-typedef struct _GstMetaItem GstMetaItem;
-
-struct _GstMetaItem
-{
-  GstMetaItem *next;
-  GstMeta meta;
-};
-
 /* info->size will be sizeof(FooMeta) which contains a GstMeta at the beginning
  * too, and then there is again a GstMeta in GstMetaItem, so subtract one. */
 #define ITEM_SIZE(info) ((info)->size + sizeof (GstMetaItem) - sizeof (GstMeta))
@@ -172,6 +164,38 @@ typedef struct
   GstMetaItem *item;
 } GstBufferImpl;
 
+static gint64 meta_seq;         /* 0 *//* ATOMIC */
+
+/* TODO: use GLib's once https://gitlab.gnome.org/GNOME/glib/issues/1076 lands */
+#if defined(__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8)
+static inline gint64
+gst_atomic_int64_inc (volatile gint64 * atomic)
+{
+  return __sync_fetch_and_add (atomic, 1);
+}
+#elif defined (G_PLATFORM_WIN32)
+#include <windows.h>
+static inline gint64
+gst_atomic_int64_inc (volatile gint64 * atomic)
+{
+  InterlockedExchangeAdd (atomic, 1);
+}
+#else
+#warning No 64-bit atomic int defined for this platform/toolchain!
+#define NO_64BIT_ATOMIC_INT_FOR_PLATFORM
+G_LOCK_DEFINE_STATIC (meta_seq);
+static inline gint64
+gst_atomic_int64_inc (volatile gint64 * atomic)
+{
+  gint64 ret;
+
+  G_LOCK (meta_seq);
+  ret = *atomic++;
+  G_UNLOCK (meta_seq);
+
+  return ret;
+}
+#endif
 
 static gboolean
 _is_span (GstMemory ** mem, gsize len, gsize * poffset, GstMemory ** parent)
@@ -454,6 +478,11 @@ void
 _priv_gst_buffer_initialize (void)
 {
   _gst_buffer_type = gst_buffer_get_type ();
+
+#ifdef NO_64BIT_ATOMIC_INT_FOR_PLATFORM
+  GST_CAT_WARNING (GST_CAT_PERFORMANCE,
+      "No 64-bit atomic int defined for this platform/toolchain!");
+#endif
 }
 
 /**
@@ -2254,6 +2283,8 @@ gst_buffer_add_meta (GstBuffer * buffer, const GstMetaInfo * info,
     if (!info->init_func (result, params, buffer))
       goto init_failed;
 
+  item->seq_num = gst_atomic_int64_inc (&meta_seq);
+
   /* and add to the list of metadata */
   item->next = GST_BUFFER_META (buffer);
   GST_BUFFER_META (buffer) = item;

diff --git a/gst/gstmeta.c b/gst/gstmeta.c
index 89ebd9d..4bcaa7a 100644 (file)
--- a/gst/gstmeta.c
+++ b/gst/gstmeta.c
@@ -227,3 +227,51 @@ gst_meta_get_info (const gchar * impl)
 
   return info;
 }
+
+/**
+ * gst_meta_get_seqnum:
+ * @meta: a #GstMeta
+ *
+ * Gets seqnum for this meta.
+ *
+ * Since: 1.16
+ */
+guint64
+gst_meta_get_seqnum (const GstMeta * meta)
+{
+  GstMetaItem *meta_item;
+  guint8 *p;
+
+  g_return_val_if_fail (meta != NULL, 0);
+
+  p = (guint8 *) meta;
+  p -= G_STRUCT_OFFSET (GstMetaItem, meta);
+  meta_item = (GstMetaItem *) p;
+  return meta_item->seq_num;
+}
+
+/**
+ * gst_meta_compare_seqnum:
+ * @meta1: a #GstMeta
+ * @meta2: a #GstMeta
+ *
+ * Meta sequence number compare function. Can be used as #GCompareFunc
+ * or a #GCompareDataFunc.
+ *
+ * Returns: a negative number if @meta1 comes before @meta2, 0 if both metas
+ *   have an equal sequence number, or a positive integer if @meta1 comes
+ *   after @meta2.
+ *
+ * Since: 1.16
+ */
+gint
+gst_meta_compare_seqnum (const GstMeta * meta1, const GstMeta * meta2)
+{
+  guint64 seqnum1 = gst_meta_get_seqnum (meta1);
+  guint64 seqnum2 = gst_meta_get_seqnum (meta2);
+
+  if (seqnum1 == seqnum2)
+    return 0;
+
+  return (seqnum1 < seqnum2) ? -1 : 1;
+}

diff --git a/gst/gstmeta.h b/gst/gstmeta.h
index 60268f8..d617ef8 100644 (file)
--- a/gst/gstmeta.h
+++ b/gst/gstmeta.h
@@ -222,6 +222,13 @@ const GstMetaInfo *  gst_meta_get_info          (const gchar * impl);
 GST_API
 const gchar* const*  gst_meta_api_type_get_tags (GType api);
 
+GST_API
+guint64              gst_meta_get_seqnum        (const GstMeta * meta);
+
+GST_API
+gint                 gst_meta_compare_seqnum    (const GstMeta * meta1,
+                                                 const GstMeta * meta2);
+
 /* some default tags */
 
 GST_API GQuark _gst_meta_tag_memory;

diff --git a/tests/check/gst/gstmeta.c b/tests/check/gst/gstmeta.c
index 9e96bf7..080f9d8 100644 (file)
--- a/tests/check/gst/gstmeta.c
+++ b/tests/check/gst/gstmeta.c
@@ -633,6 +633,61 @@ GST_START_TEST (test_meta_iterate)
 
 GST_END_TEST;
 
+#define test_meta_compare_seqnum(a,b) \
+    gst_meta_compare_seqnum((GstMeta*)(a),(GstMeta*)(b))
+
+GST_START_TEST (test_meta_seqnum)
+{
+  GstMetaTest *meta1, *meta2, *meta3;
+  GstBuffer *buffer;
+
+  buffer = gst_buffer_new_and_alloc (4);
+  fail_unless (buffer != NULL);
+
+  /* add some metadata */
+  meta1 = GST_META_TEST_ADD (buffer);
+  fail_unless (meta1 != NULL);
+  meta2 = GST_META_TEST_ADD (buffer);
+  fail_unless (meta2 != NULL);
+  meta3 = GST_META_TEST_ADD (buffer);
+  fail_unless (meta3 != NULL);
+
+  fail_unless (test_meta_compare_seqnum (meta1, meta2) < 0);
+  fail_unless (test_meta_compare_seqnum (meta2, meta3) < 0);
+  fail_unless (test_meta_compare_seqnum (meta1, meta3) < 0);
+
+  fail_unless_equals_int (test_meta_compare_seqnum (meta1, meta1), 0);
+  fail_unless_equals_int (test_meta_compare_seqnum (meta2, meta2), 0);
+  fail_unless_equals_int (test_meta_compare_seqnum (meta3, meta3), 0);
+
+  fail_unless (test_meta_compare_seqnum (meta2, meta1) > 0);
+  fail_unless (test_meta_compare_seqnum (meta3, meta2) > 0);
+  fail_unless (test_meta_compare_seqnum (meta3, meta1) > 0);
+
+  /* Check that gst_meta_compare_seqnum() works correctly as a GCompareFunc */
+  {
+    GList *list;
+
+    /* Make list: 3, 1, 2 */
+    list = g_list_prepend (NULL, meta2);
+    list = g_list_prepend (list, meta1);
+    list = g_list_prepend (list, meta3);
+
+    list = g_list_sort (list, (GCompareFunc) gst_meta_compare_seqnum);
+
+    fail_unless (g_list_nth_data (list, 0) == meta1);
+    fail_unless (g_list_nth_data (list, 1) == meta2);
+    fail_unless (g_list_nth_data (list, 2) == meta3);
+
+    g_list_free (list);
+  }
+
+  /* clean up */
+  gst_buffer_unref (buffer);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_buffermeta_suite (void)
 {
@@ -649,6 +704,7 @@ gst_buffermeta_suite (void)
   tcase_add_test (tc_chain, test_meta_foreach_remove_head_and_tail_of_three);
   tcase_add_test (tc_chain, test_meta_foreach_remove_several);
   tcase_add_test (tc_chain, test_meta_iterate);
+  tcase_add_test (tc_chain, test_meta_seqnum);
 
   return s;
 }