msdk: Fix increasing memory usage in dynamic pipelines
authorNirbheek Chauhan <nirbheek@centricular.com>
Fri, 17 Jan 2020 05:37:47 +0000 (11:07 +0530)
committerHaihao Xiang <haihao.xiang@intel.com>
Tue, 21 Jan 2020 00:38:41 +0000 (00:38 +0000)
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
sys/msdk/gstmsdkdec.c
sys/msdk/gstmsdkdec.h
sys/msdk/gstmsdkenc.c
sys/msdk/gstmsdkenc.h
sys/msdk/gstmsdkvpp.c
sys/msdk/gstmsdkvpp.h

index 0e25080..6f5c7e4 100644 (file)
@@ -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)
index caa9967..b50457f 100644 (file)
@@ -710,6 +710,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.
    */
@@ -765,6 +771,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;
index 8bb746b..b15c855 100644 (file)
@@ -90,6 +90,7 @@ struct _GstMsdkDec
 
   /* MFX context */
   GstMsdkContext *context;
+  GstMsdkContext *old_context;
   mfxVideoParam param;
   GArray *tasks;
   guint next_task;
index d5a6e87..4a761d0 100644 (file)
@@ -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);
 }
index 9895874..2703fb1 100644 (file)
@@ -99,6 +99,7 @@ struct _GstMsdkEnc
 
   /* MFX context */
   GstMsdkContext *context;
+  GstMsdkContext *old_context;
   mfxVideoParam param;
   guint num_surfaces;
   guint num_tasks;
index 05747d9..dce4934 100644 (file)
@@ -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);
 }
 
index c644084..83ff2b7 100644 (file)
@@ -89,6 +89,7 @@ struct _GstMsdkVPP
 
   /* MFX context */
   GstMsdkContext *context;
+  GstMsdkContext *old_context;
   mfxVideoParam param;
   guint in_num_surfaces;
   guint out_num_surfaces;