buffer: drop parent meta in deep copy/foreach_metadata
authorPhilipp Zabel <p.zabel@pengutronix.de>
Wed, 28 Mar 2018 15:54:15 +0000 (17:54 +0200)
committerJames Hilliard <james.hilliard1@gmail.com>
Wed, 28 Sep 2022 18:34:44 +0000 (12:34 -0600)
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/2928>

subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudiodecoder.c
subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioencoder.c
subprojects/gst-plugins-base/gst-libs/gst/video/gstvideodecoder.c
subprojects/gst-plugins-base/gst-libs/gst/video/gstvideoencoder.c
subprojects/gstreamer/gst/gstbuffer.c
subprojects/gstreamer/gst/gstmeta.c
subprojects/gstreamer/gst/gstmeta.h
subprojects/gstreamer/libs/gst/base/gstadapter.c
subprojects/gstreamer/libs/gst/base/gstbasetransform.c

index ec15d6c..a404256 100644 (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));
index da7f508..618b7eb 100644 (file)
@@ -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));
index 7d7a91f..43b5e52 100644 (file)
@@ -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));
index f99653e..fb86cf0 100644 (file)
@@ -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));
index b6ea6a1..bfb2b35 100644 (file)
 #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);
index 4bac360..6089877 100644 (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
index 71c3095..44edbe0 100644 (file)
@@ -87,12 +87,22 @@ 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
  * @info: pointer to the #GstMetaInfo
@@ -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:
index 33c84f2..89f2e71 100644 (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));
index ec5ef76..3f38c09 100644 (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));