From 1bcf44bb1f38ca71709f2f6439f558aac1c9692b Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan 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 0e25080..6f5c7e4 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 caa9967..b50457f 100644 --- a/sys/msdk/gstmsdkdec.c +++ b/sys/msdk/gstmsdkdec.c @@ -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; diff --git a/sys/msdk/gstmsdkdec.h b/sys/msdk/gstmsdkdec.h index 8bb746b..b15c855 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 d5a6e87..4a761d0 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 9895874..2703fb1 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 05747d9..dce4934 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 c644084..83ff2b7 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; -- 2.7.4