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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3090>
This commit is contained in:
Philipp Zabel 2018-03-28 17:54:15 +02:00 committed by Tim-Philipp Müller
parent b19116b786
commit 1c8244a19d
9 changed files with 38 additions and 8 deletions

View file

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

View file

@ -706,7 +706,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));

View file

@ -3359,7 +3359,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));

View file

@ -2198,7 +2198,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));

View file

@ -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;
@ -2672,7 +2681,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);

View file

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

View file

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

View file

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

View file

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