From: Tim-Philipp Müller Date: Fri, 2 Dec 2016 17:56:59 +0000 (+0000) Subject: buffer: store sequence number for metas X-Git-Tag: 1.16.2~120 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f62ee975922ec6c790d587c43d268cf6a373fb92;p=platform%2Fupstream%2Fgstreamer.git 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 --- 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 GST_META_CAST diff --git a/gst/gst_private.h b/gst/gst_private.h index 2a2e8fa..0edc93c 100644 --- 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 --- 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 +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 --- 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 --- 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 --- 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; }