From a819f05851a71a43514809d5081a49d7ab7e6cd4 Mon Sep 17 00:00:00 2001
From: Haihao Xiang <haihao.xiang@intel.com>
Date: Mon, 23 Dec 2019 14:09:25 +0800
Subject: [PATCH] msdkdec: free unlocked msdk surface before output buffer
 allocation

https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/merge_requests/924
is trying to use video memory for decoding on Linux, which reveals a
hidden bug in msdkdec.

For video memory, it is possible that a locked mfx surface is not used
indeed and it will be un-locked later in MSDK, so we have to check the
associated MSDK surface to find out and free un-used surfaces, otherwise
it is easy to exhaust all pre-allocated mfx surfaces and get errors below:

0:00:00.777324879 27290 0x564b65a510a0 ERROR                default
gstmsdkvideomemory.c:77:gst_msdk_video_allocator_get_surface: failed to
get surface available
0:00:00.777429079 27290 0x564b65a510a0 ERROR         msdkbufferpool
gstmsdkbufferpool.c:260:gst_msdk_buffer_pool_alloc_buffer:<msdkbufferpool0>
failed to create new MSDK memory

Note the sample code in MSDK does similar thing in
CBuffering::SyncFrameSurfaces()
---
 sys/msdk/gstmsdkdec.c | 75 ++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/sys/msdk/gstmsdkdec.c b/sys/msdk/gstmsdkdec.c
index e723787bc7..cad4a61345 100644
--- a/sys/msdk/gstmsdkdec.c
+++ b/sys/msdk/gstmsdkdec.c
@@ -114,6 +114,39 @@ gst_msdkdec_get_oldest_frame (GstVideoDecoder * decoder)
   return frame;
 }
 
+static void
+free_surface (GstMsdkDec * thiz, MsdkSurface * s)
+{
+  if (s->copy.buffer) {
+    gst_video_frame_unmap (&s->copy);
+    gst_buffer_unref (s->copy.buffer);
+  }
+
+  if (s->data.buffer)
+    gst_video_frame_unmap (&s->data);
+
+  gst_buffer_unref (s->buf);
+  thiz->decoded_msdk_surfaces = g_list_remove (thiz->decoded_msdk_surfaces, s);
+
+  g_slice_free (MsdkSurface, s);
+}
+
+static void
+gst_msdkdec_free_unlocked_msdk_surfaces (GstMsdkDec * thiz,
+    MsdkSurface * curr_surface)
+{
+  GList *l;
+  MsdkSurface *surface;
+
+  for (l = thiz->decoded_msdk_surfaces; l;) {
+    surface = l->data;
+    l = l->next;
+
+    if (surface != curr_surface && surface->surface->Data.Locked == 0)
+      free_surface (thiz, surface);
+  }
+}
+
 static GstFlowReturn
 allocate_output_buffer (GstMsdkDec * thiz, GstBuffer ** buffer)
 {
@@ -130,6 +163,11 @@ allocate_output_buffer (GstMsdkDec * thiz, GstBuffer ** buffer)
   }
 
   if (!frame->output_buffer) {
+    /* Free un-unsed msdk surfaces firstly, hence the associated mfx
+     * surfaces will be moved from used list to available list */
+    if (thiz->postpone_free_surface || thiz->use_video_memory)
+      gst_msdkdec_free_unlocked_msdk_surfaces (thiz, NULL);
+
     flow = gst_video_decoder_allocate_output_frame (decoder, frame);
     if (flow != GST_FLOW_OK) {
       gst_video_codec_frame_unref (frame);
@@ -147,23 +185,6 @@ allocate_output_buffer (GstMsdkDec * thiz, GstBuffer ** buffer)
   return GST_FLOW_OK;
 }
 
-static void
-free_surface (GstMsdkDec * thiz, MsdkSurface * s)
-{
-  if (s->copy.buffer) {
-    gst_video_frame_unmap (&s->copy);
-    gst_buffer_unref (s->copy.buffer);
-  }
-
-  if (s->data.buffer)
-    gst_video_frame_unmap (&s->data);
-
-  gst_buffer_unref (s->buf);
-  thiz->decoded_msdk_surfaces = g_list_remove (thiz->decoded_msdk_surfaces, s);
-
-  g_slice_free (MsdkSurface, s);
-}
-
 static MsdkSurface *
 get_surface (GstMsdkDec * thiz, GstBuffer * buffer)
 {
@@ -593,22 +614,6 @@ _find_msdk_surface (gconstpointer msdk_surface, gconstpointer comp_surface)
   return cached_surface ? cached_surface->surface != _surface : -1;
 }
 
-static void
-gst_msdkdec_free_unlocked_msdk_surfaces (GstMsdkDec * thiz,
-    MsdkSurface * curr_surface)
-{
-  GList *l;
-  MsdkSurface *surface;
-
-  for (l = thiz->decoded_msdk_surfaces; l;) {
-    surface = l->data;
-    l = l->next;
-
-    if (surface != curr_surface && surface->surface->Data.Locked == 0)
-      free_surface (thiz, surface);
-  }
-}
-
 static GstFlowReturn
 gst_msdkdec_finish_task (GstMsdkDec * thiz, MsdkDecTask * task)
 {
@@ -1016,8 +1021,6 @@ gst_msdkdec_handle_frame (GstVideoDecoder * decoder, GstVideoCodecFrame * frame)
   gst_video_codec_frame_unref (frame);
   frame = NULL;
   for (;;) {
-    if (thiz->postpone_free_surface)
-      gst_msdkdec_free_unlocked_msdk_surfaces (thiz, surface);
     task = &g_array_index (thiz->tasks, MsdkDecTask, thiz->next_task);
     flow = gst_msdkdec_finish_task (thiz, task);
     if (flow != GST_FLOW_OK) {
@@ -1435,8 +1438,6 @@ gst_msdkdec_drain (GstVideoDecoder * decoder)
   session = gst_msdk_context_get_session (thiz->context);
 
   for (;;) {
-    if (thiz->postpone_free_surface)
-      gst_msdkdec_free_unlocked_msdk_surfaces (thiz, surface);
     task = &g_array_index (thiz->tasks, MsdkDecTask, thiz->next_task);
     if ((flow = gst_msdkdec_finish_task (thiz, task)) != GST_FLOW_OK) {
       if (flow != GST_FLOW_FLUSHING)