From 3b900e1fa4fd888012dc005fa26ae2532a89b7a7 Mon Sep 17 00:00:00 2001 From: Philipp Zabel Date: Wed, 28 Mar 2018 17:54:15 +0200 Subject: [PATCH] buffer: drop parent meta in deep copy/foreach_metadata The purpose of a deep buffer copy is to be able to release the source buffer and all its dependencies. Attaching the parent buffer meta to the newly created deep copy needlessly keeps holding a reference to the parent buffer. The issue this solves is the fact you need to allocate more buffers, as you have free buffers being held for no reason. In the good cases it will use more memory, in the bad case it will stall your pipeline (since codecs often need a minimum number of buffers to actually work). Fixes #283 Part-of: --- .../gst-libs/gst/audio/gstaudiodecoder.c | 3 ++- .../gst-libs/gst/audio/gstaudioencoder.c | 3 ++- .../gst-libs/gst/video/gstvideodecoder.c | 3 ++- .../gst-libs/gst/video/gstvideoencoder.c | 3 ++- subprojects/gstreamer/gst/gstbuffer.c | 11 ++++++++++- subprojects/gstreamer/gst/gstmeta.c | 3 +++ subprojects/gstreamer/gst/gstmeta.h | 11 +++++++++++ subprojects/gstreamer/libs/gst/base/gstadapter.c | 3 ++- .../gstreamer/libs/gst/base/gstbasetransform.c | 6 ++++-- 9 files changed, 38 insertions(+), 8 deletions(-) diff --git a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudiodecoder.c b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudiodecoder.c index ec15d6c53e..a404256f8f 100644 --- a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudiodecoder.c +++ b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudiodecoder.c @@ -1245,7 +1245,8 @@ foreach_metadata (GstBuffer * inbuf, GstMeta ** meta, gpointer user_data) const GstMetaInfo *info = (*meta)->info; gboolean do_copy = FALSE; - if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory)) { + if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory) + || gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory_reference)) { /* never call the transform_meta with memory specific metadata */ GST_DEBUG_OBJECT (decoder, "not copying memory specific metadata %s", g_type_name (info->api)); diff --git a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioencoder.c b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioencoder.c index da7f508b04..618b7ebd6a 100644 --- a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioencoder.c +++ b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioencoder.c @@ -735,7 +735,8 @@ foreach_metadata (GstBuffer * inbuf, GstMeta ** meta, gpointer user_data) const GstMetaInfo *info = (*meta)->info; gboolean do_copy = FALSE; - if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory)) { + if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory) + || gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory_reference)) { /* never call the transform_meta with memory specific metadata */ GST_DEBUG_OBJECT (encoder, "not copying memory specific metadata %s", g_type_name (info->api)); diff --git a/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideodecoder.c b/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideodecoder.c index 7d7a91f1c7..43b5e52622 100644 --- a/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideodecoder.c +++ b/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideodecoder.c @@ -3347,7 +3347,8 @@ foreach_metadata (GstBuffer * inbuf, GstMeta ** meta, gpointer user_data) const GstMetaInfo *info = (*meta)->info; gboolean do_copy = FALSE; - if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory)) { + if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory) + || gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory_reference)) { /* never call the transform_meta with memory specific metadata */ GST_DEBUG_OBJECT (decoder, "not copying memory specific metadata %s", g_type_name (info->api)); diff --git a/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c b/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c index f99653ed82..fb86cf0764 100644 --- a/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c +++ b/subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c @@ -2186,7 +2186,8 @@ foreach_metadata (GstBuffer * inbuf, GstMeta ** meta, gpointer user_data) const GstMetaInfo *info = (*meta)->info; gboolean do_copy = FALSE; - if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory)) { + if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory) + || gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory_reference)) { /* never call the transform_meta with memory specific metadata */ GST_DEBUG_OBJECT (encoder, "not copying memory specific metadata %s", g_type_name (info->api)); diff --git a/subprojects/gstreamer/gst/gstbuffer.c b/subprojects/gstreamer/gst/gstbuffer.c index b6ea6a1229..bfb2b35da2 100644 --- a/subprojects/gstreamer/gst/gstbuffer.c +++ b/subprojects/gstreamer/gst/gstbuffer.c @@ -130,6 +130,7 @@ #include "gstbuffer.h" #include "gstbufferpool.h" #include "gstinfo.h" +#include "gstmeta.h" #include "gstutils.h" #include "gstversion.h" @@ -666,6 +667,10 @@ gst_buffer_copy_into (GstBuffer * dest, GstBuffer * src, } if (flags & GST_BUFFER_COPY_META) { + gboolean deep; + + deep = (flags & GST_BUFFER_COPY_DEEP) != 0; + /* NOTE: GstGLSyncMeta copying relies on the meta * being copied now, after the buffer data, * so this has to happen last */ @@ -683,6 +688,10 @@ gst_buffer_copy_into (GstBuffer * dest, GstBuffer * src, GST_CAT_DEBUG (GST_CAT_BUFFER, "don't copy memory meta %p of API type %s", meta, g_type_name (info->api)); + } else if (deep && gst_meta_api_type_has_tag (info->api, + _gst_meta_tag_memory_reference)) { + GST_CAT_DEBUG (GST_CAT_BUFFER, + "don't copy meta with memory references %" GST_PTR_FORMAT, meta); } else if (info->transform_func) { GstMetaTransformCopy copy_data; @@ -2667,7 +2676,7 @@ GType gst_parent_buffer_meta_api_get_type (void) { static GType type = 0; - static const gchar *tags[] = { NULL }; + static const gchar *tags[] = { GST_META_TAG_MEMORY_REFERENCE_STR, NULL }; if (g_once_init_enter (&type)) { GType _type = gst_meta_api_type_register ("GstParentBufferMetaAPI", tags); diff --git a/subprojects/gstreamer/gst/gstmeta.c b/subprojects/gstreamer/gst/gstmeta.c index 4bac360876..60898770e7 100644 --- a/subprojects/gstreamer/gst/gstmeta.c +++ b/subprojects/gstreamer/gst/gstmeta.c @@ -57,6 +57,7 @@ static GRWLock lock; GQuark _gst_meta_transform_copy; GQuark _gst_meta_tag_memory; +GQuark _gst_meta_tag_memory_reference; typedef struct { @@ -88,6 +89,8 @@ _priv_gst_meta_initialize (void) _gst_meta_transform_copy = g_quark_from_static_string ("gst-copy"); _gst_meta_tag_memory = g_quark_from_static_string ("memory"); + _gst_meta_tag_memory_reference = + g_quark_from_static_string ("memory-reference"); } static gboolean diff --git a/subprojects/gstreamer/gst/gstmeta.h b/subprojects/gstreamer/gst/gstmeta.h index 71c3095bfc..44edbe0b4a 100644 --- a/subprojects/gstreamer/gst/gstmeta.h +++ b/subprojects/gstreamer/gst/gstmeta.h @@ -87,11 +87,21 @@ typedef enum { * GST_META_TAG_MEMORY_STR: * * This metadata stays relevant as long as memory layout is unchanged. + * In hindsight, this tag should have been called "memory-layout". * * Since: 1.2 */ #define GST_META_TAG_MEMORY_STR "memory" +/** + * GST_META_TAG_MEMORY_REFERENCE_STR: + * + * This metadata stays relevant until a deep copy is made. + * + * Since: 1.20.4 + */ +#define GST_META_TAG_MEMORY_REFERENCE_STR "memory-reference" + /** * GstMeta: * @flags: extra flags for the metadata @@ -282,6 +292,7 @@ gint gst_meta_compare_seqnum (const GstMeta * meta1, /* some default tags */ GST_API GQuark _gst_meta_tag_memory; +GST_API GQuark _gst_meta_tag_memory_reference; /** * GST_META_TAG_MEMORY: diff --git a/subprojects/gstreamer/libs/gst/base/gstadapter.c b/subprojects/gstreamer/libs/gst/base/gstadapter.c index 33c84f25a1..89f2e7160f 100644 --- a/subprojects/gstreamer/libs/gst/base/gstadapter.c +++ b/subprojects/gstreamer/libs/gst/base/gstadapter.c @@ -919,7 +919,8 @@ foreach_metadata (GstBuffer * inbuf, GstMeta ** meta, gpointer user_data) const GstMetaInfo *info = (*meta)->info; gboolean do_copy = FALSE; - if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory)) { + if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory) + || gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory_reference)) { /* never call the transform_meta with memory specific metadata */ GST_DEBUG ("not copying memory specific metadata %s", g_type_name (info->api)); diff --git a/subprojects/gstreamer/libs/gst/base/gstbasetransform.c b/subprojects/gstreamer/libs/gst/base/gstbasetransform.c index ec5ef76950..3f38c094b2 100644 --- a/subprojects/gstreamer/libs/gst/base/gstbasetransform.c +++ b/subprojects/gstreamer/libs/gst/base/gstbasetransform.c @@ -810,7 +810,8 @@ gst_base_transform_default_decide_allocation (GstBaseTransform * trans, /* by default we remove all metadata, subclasses should implement a * filter_meta function */ - if (gst_meta_api_type_has_tag (api, _gst_meta_tag_memory)) { + if (gst_meta_api_type_has_tag (api, _gst_meta_tag_memory) + || gst_meta_api_type_has_tag (api, _gst_meta_tag_memory_reference)) { /* remove all memory dependent metadata because we are going to have to * allocate different memory for input and output. */ GST_LOG_OBJECT (trans, "removing memory specific metadata %s", @@ -1766,7 +1767,8 @@ foreach_metadata (GstBuffer * inbuf, GstMeta ** meta, gpointer user_data) klass = GST_BASE_TRANSFORM_GET_CLASS (trans); - if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory)) { + if (gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory) + || gst_meta_api_type_has_tag (info->api, _gst_meta_tag_memory_reference)) { /* never call the transform_meta with memory specific metadata */ GST_DEBUG_OBJECT (trans, "not copying memory specific metadata %s", g_type_name (info->api));