From 1dd3fb273341e7f182e018d90c2152ce7847388c Mon Sep 17 00:00:00 2001
From: Nirbheek Chauhan <nirbheek@centricular.com>
Date: Fri, 17 Jan 2020 11:07:47 +0530
Subject: [PATCH] msdk: Fix increasing memory usage in dynamic pipelines
Our context is non-persistent, and we propagate it throughout the
pipeline. This means that if we try to reuse any gstmsdk element by
removing it from the pipeline and then re-adding it, we'll clone the
mfxSession and create a new gstmsdk context as a child of the old one
inside `gst_msdk_context_new_with_parent()`.
Normally this only allocates a few KB inside the driver, but on
Windows it seems to allocate tens of MBs which leads to linearly
increasing memory usage for each PLAYING->NULL->PLAYING state cycle
for the process. The contexts will only be freed when the pipeline
itself goes to `NULL`, which would defeat the purpose of dynamic
pipelines.
Essentially, we need to optimize the case in which the element is
removed from the pipeline and re-added and the same context is re-set
on it. To detect that case, we set the context on `old_context`, and
compare it to the new one when preparing the context. If they're the
same, we don't need to do anything.
Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/946
---
sys/msdk/gstmsdkcontextutil.c | 1 +
sys/msdk/gstmsdkdec.c | 11 +++++++++++
sys/msdk/gstmsdkdec.h | 1 +
sys/msdk/gstmsdkenc.c | 12 ++++++++++++
sys/msdk/gstmsdkenc.h | 1 +
sys/msdk/gstmsdkvpp.c | 15 +++++++++++++++
sys/msdk/gstmsdkvpp.h | 1 +
7 files changed, 42 insertions(+)
diff --git a/sys/msdk/gstmsdkcontextutil.c b/sys/msdk/gstmsdkcontextutil.c
index 0e25080efb7..6f5c7e41d74 100644
--- a/sys/msdk/gstmsdkcontextutil.c
+++ b/sys/msdk/gstmsdkcontextutil.c
@@ -161,6 +161,7 @@ gst_msdk_context_find (GstElement * element, GstMsdkContext ** context_ptr)
return TRUE;
}
+ /* This may indirectly set *context_ptr, see function body */
_gst_context_query (element, GST_MSDK_CONTEXT_TYPE_NAME);
if (*context_ptr)
diff --git a/sys/msdk/gstmsdkdec.c b/sys/msdk/gstmsdkdec.c
index 923b5c0ca13..97da6cd6bf9 100644
--- a/sys/msdk/gstmsdkdec.c
+++ b/sys/msdk/gstmsdkdec.c
@@ -684,6 +684,12 @@ gst_msdkdec_context_prepare (GstMsdkDec * thiz)
if (!gst_msdk_context_find (GST_ELEMENT_CAST (thiz), &thiz->context))
return FALSE;
+ if (thiz->context == thiz->old_context) {
+ GST_INFO_OBJECT (thiz, "Found old context %" GST_PTR_FORMAT
+ ", reusing as-is", thiz->context);
+ return TRUE;
+ }
+
/* TODO: Currently d3d allocator is not implemented.
* So decoder uses system memory by default on Windows.
*/
@@ -740,6 +746,11 @@ gst_msdkdec_start (GstVideoDecoder * decoder)
thiz->context);
}
+ /* Save the current context in a separate field so that we know whether it
+ * has changed between calls to _start() */
+ gst_object_replace ((GstObject **) & thiz->old_context,
+ (GstObject *) thiz->context);
+
gst_msdk_context_add_shared_async_depth (thiz->context, thiz->async_depth);
return TRUE;
diff --git a/sys/msdk/gstmsdkdec.h b/sys/msdk/gstmsdkdec.h
index 8bb746b858f..b15c855c04b 100644
--- a/sys/msdk/gstmsdkdec.h
+++ b/sys/msdk/gstmsdkdec.h
@@ -90,6 +90,7 @@ struct _GstMsdkDec
/* MFX context */
GstMsdkContext *context;
+ GstMsdkContext *old_context;
mfxVideoParam param;
GArray *tasks;
guint next_task;
diff --git a/sys/msdk/gstmsdkenc.c b/sys/msdk/gstmsdkenc.c
index 967e7e9d7cd..17732643702 100644
--- a/sys/msdk/gstmsdkenc.c
+++ b/sys/msdk/gstmsdkenc.c
@@ -1605,6 +1605,12 @@ gst_msdkenc_context_prepare (GstMsdkEnc * thiz)
if (!gst_msdk_context_find (GST_ELEMENT_CAST (thiz), &thiz->context))
return FALSE;
+ if (thiz->context == thiz->old_context) {
+ GST_INFO_OBJECT (thiz, "Found old context %" GST_PTR_FORMAT
+ ", reusing as-is", thiz->context);
+ return TRUE;
+ }
+
GST_INFO_OBJECT (thiz, "Found context %" GST_PTR_FORMAT " from neighbour",
thiz->context);
@@ -1656,6 +1662,11 @@ gst_msdkenc_start (GstVideoEncoder * encoder)
thiz->context);
}
+ /* Save the current context in a separate field so that we know whether it
+ * has changed between calls to _start() */
+ gst_object_replace ((GstObject **) & thiz->old_context,
+ (GstObject *) thiz->context);
+
gst_msdk_context_add_shared_async_depth (thiz->context, thiz->async_depth);
/* Set the minimum pts to some huge value (1000 hours). This keeps
@@ -1779,6 +1790,7 @@ gst_msdkenc_finalize (GObject * object)
gst_clear_object (&thiz->msdk_pool);
gst_clear_object (&thiz->msdk_converted_pool);
+ gst_clear_object (&thiz->old_context);
G_OBJECT_CLASS (parent_class)->finalize (object);
}
diff --git a/sys/msdk/gstmsdkenc.h b/sys/msdk/gstmsdkenc.h
index 98958744970..2703fb188a2 100644
--- a/sys/msdk/gstmsdkenc.h
+++ b/sys/msdk/gstmsdkenc.h
@@ -99,6 +99,7 @@ struct _GstMsdkEnc
/* MFX context */
GstMsdkContext *context;
+ GstMsdkContext *old_context;
mfxVideoParam param;
guint num_surfaces;
guint num_tasks;
diff --git a/sys/msdk/gstmsdkvpp.c b/sys/msdk/gstmsdkvpp.c
index 05747d9b858..dce4934dfae 100644
--- a/sys/msdk/gstmsdkvpp.c
+++ b/sys/msdk/gstmsdkvpp.c
@@ -197,6 +197,12 @@ gst_msdkvpp_context_prepare (GstMsdkVPP * thiz)
if (!gst_msdk_context_find (GST_ELEMENT_CAST (thiz), &thiz->context))
return FALSE;
+ if (thiz->context == thiz->old_context) {
+ GST_INFO_OBJECT (thiz, "Found old context %" GST_PTR_FORMAT
+ ", reusing as-is", thiz->context);
+ return TRUE;
+ }
+
GST_INFO_OBJECT (thiz, "Found context %" GST_PTR_FORMAT " from neighbour",
thiz->context);
@@ -247,6 +253,11 @@ ensure_context (GstBaseTransform * trans)
thiz->context);
}
+ /* Save the current context in a separate field so that we know whether it
+ * has changed between calls to _start() */
+ gst_object_replace ((GstObject **) & thiz->old_context,
+ (GstObject *) thiz->context);
+
gst_msdk_context_add_shared_async_depth (thiz->context, thiz->async_depth);
return TRUE;
@@ -1476,6 +1487,10 @@ gst_msdkvpp_get_property (GObject * object, guint prop_id,
static void
gst_msdkvpp_finalize (GObject * object)
{
+ GstMsdkVPP *thiz = GST_MSDKVPP (object);
+
+ gst_clear_object (&thiz->old_context);
+
G_OBJECT_CLASS (parent_class)->finalize (object);
}
diff --git a/sys/msdk/gstmsdkvpp.h b/sys/msdk/gstmsdkvpp.h
index c644084d3d8..83ff2b7f5c4 100644
--- a/sys/msdk/gstmsdkvpp.h
+++ b/sys/msdk/gstmsdkvpp.h
@@ -89,6 +89,7 @@ struct _GstMsdkVPP
/* MFX context */
GstMsdkContext *context;
+ GstMsdkContext *old_context;
mfxVideoParam param;
guint in_num_surfaces;
guint out_num_surfaces;
--
GitLab