msdk: fix for mfx frame alloc response

Both MSDK and this plugin use mfxFrameAllocResponse for video and DMABuf
memory, it is possible that some GST buffers are still in use when calling
gst_msdk_frame_free, so add a reference count in the wrapper of
mfxFrameAllocResponse (GstMsdkAllocResponse) to make sure the underlying
mfx resources are still available if the corresponding buffer pool is in
use.

In addtion, currently all allocators for input or output share the same
mfxFrameAllocResponse pointer in an element, so it is possible that
the content of mfxFrameAllocResponse is updated for a new caps then all
GST buffers allocated from an old allocator will use this new content of
mfxFrameAllocResponse, which will result in unexpected behavior. In this
fix, we save the the content of mfxFrameAllocResponse in the corresponding
tructure to avoid such issue

Sample pipeline:

gst-launch-1.0 filesrc location=vp9_multi_resolutions.ivf ! ivfparse ! msdkvp9dec !
msdkvpp ! video/x-raw\(memory:DMABuf\),format=NV12 ! glimagesink

diff --git a/sys/msdk/gstmsdkallocator_libva.c b/sys/msdk/gstmsdkallocator_libva.c
index 81f6ce7..3ddcbb8 100644 (file)
--- a/sys/msdk/gstmsdkallocator_libva.c
+++ b/sys/msdk/gstmsdkallocator_libva.c
@@ -73,6 +73,7 @@ gst_msdk_frame_alloc (mfxHDL pthis, mfxFrameAllocRequest * req,
         return MFX_ERR_MEMORY_ALLOC;
 
       *resp = cached->response;
+      g_atomic_int_inc (&cached->refcount);
       return MFX_ERR_NONE;
     }
   }
@@ -207,6 +208,7 @@ gst_msdk_frame_alloc (mfxHDL pthis, mfxFrameAllocRequest * req,
 
   msdk_resp->response = *resp;
   msdk_resp->request = *req;
+  msdk_resp->refcount = 1;
 
   gst_msdk_context_add_alloc_response (context, msdk_resp);
 
@@ -222,6 +224,15 @@ gst_msdk_frame_free (mfxHDL pthis, mfxFrameAllocResponse * resp)
   GstMsdkMemoryID *mem_id;
   VADisplay dpy;
   gint i;
+  GstMsdkAllocResponse *cached = NULL;
+
+  cached = gst_msdk_context_get_cached_alloc_responses (context, resp);
+
+  if (cached) {
+    if (!g_atomic_int_dec_and_test (&cached->refcount))
+      return MFX_ERR_NONE;
+  } else
+    return MFX_ERR_NONE;
 
   if (!gst_msdk_context_remove_alloc_response (context, resp))
     return MFX_ERR_NONE;

diff --git a/sys/msdk/gstmsdkcontext.h b/sys/msdk/gstmsdkcontext.h
index f7c8b66..f115b40 100644 (file)
--- a/sys/msdk/gstmsdkcontext.h
+++ b/sys/msdk/gstmsdkcontext.h
@@ -97,6 +97,7 @@ gint gst_msdk_context_get_fd (GstMsdkContext * context);
 typedef struct _GstMsdkAllocResponse GstMsdkAllocResponse;
 
 struct _GstMsdkAllocResponse {
+  gint refcount;
   mfxFrameAllocResponse response;
   mfxFrameAllocRequest request;
   GList *surfaces_avail;

diff --git a/sys/msdk/gstmsdkvideomemory.c b/sys/msdk/gstmsdkvideomemory.c
index 6e173fa..21d4981 100644 (file)
--- a/sys/msdk/gstmsdkvideomemory.c
+++ b/sys/msdk/gstmsdkvideomemory.c
@@ -363,6 +363,8 @@ gst_msdk_video_allocator_finalize (GObject * object)
 {
   GstMsdkVideoAllocator *allocator = GST_MSDK_VIDEO_ALLOCATOR_CAST (object);
 
+  gst_msdk_frame_free (allocator->context, allocator->alloc_response);
+
   gst_object_unref (allocator->context);
   G_OBJECT_CLASS (gst_msdk_video_allocator_parent_class)->finalize (object);
 }
@@ -405,17 +407,27 @@ gst_msdk_video_allocator_new (GstMsdkContext * context,
     GstVideoInfo * image_info, mfxFrameAllocResponse * alloc_resp)
 {
   GstMsdkVideoAllocator *allocator;
+  GstMsdkAllocResponse *cached = NULL;
 
   g_return_val_if_fail (context != NULL, NULL);
   g_return_val_if_fail (image_info != NULL, NULL);
 
+  cached = gst_msdk_context_get_cached_alloc_responses (context, alloc_resp);
+
+  if (!cached) {
+    GST_ERROR ("Failed to get the cached alloc response");
+    return NULL;
+  }
+
   allocator = g_object_new (GST_TYPE_MSDK_VIDEO_ALLOCATOR, NULL);
   if (!allocator)
     return NULL;
 
+  g_atomic_int_inc (&cached->refcount);
   allocator->context = gst_object_ref (context);
   allocator->image_info = *image_info;
-  allocator->alloc_response = alloc_resp;
+  allocator->mfx_response = *alloc_resp;
+  allocator->alloc_response = &allocator->mfx_response;
 
   return GST_ALLOCATOR_CAST (allocator);
 }
@@ -487,6 +499,8 @@ gst_msdk_dmabuf_allocator_finalize (GObject * object)
 {
   GstMsdkDmaBufAllocator *allocator = GST_MSDK_DMABUF_ALLOCATOR_CAST (object);
 
+  gst_msdk_frame_free (allocator->context, allocator->alloc_response);
+
   gst_object_unref (allocator->context);
   G_OBJECT_CLASS (gst_msdk_dmabuf_allocator_parent_class)->finalize (object);
 }
@@ -511,17 +525,27 @@ gst_msdk_dmabuf_allocator_new (GstMsdkContext * context,
     GstVideoInfo * image_info, mfxFrameAllocResponse * alloc_resp)
 {
   GstMsdkDmaBufAllocator *allocator;
+  GstMsdkAllocResponse *cached = NULL;
 
   g_return_val_if_fail (context != NULL, NULL);
   g_return_val_if_fail (image_info != NULL, NULL);
 
+  cached = gst_msdk_context_get_cached_alloc_responses (context, alloc_resp);
+
+  if (!cached) {
+    GST_ERROR ("Failed to get the cached alloc response");
+    return NULL;
+  }
+
   allocator = g_object_new (GST_TYPE_MSDK_DMABUF_ALLOCATOR, NULL);
   if (!allocator)
     return NULL;
 
+  g_atomic_int_inc (&cached->refcount);
   allocator->context = gst_object_ref (context);
   allocator->image_info = *image_info;
-  allocator->alloc_response = alloc_resp;
+  allocator->mfx_response = *alloc_resp;
+  allocator->alloc_response = &allocator->mfx_response;
 
   return GST_ALLOCATOR_CAST (allocator);
 }

diff --git a/sys/msdk/gstmsdkvideomemory.h b/sys/msdk/gstmsdkvideomemory.h
index e7a8594..93b9b45 100644 (file)
--- a/sys/msdk/gstmsdkvideomemory.h
+++ b/sys/msdk/gstmsdkvideomemory.h
@@ -116,6 +116,7 @@ struct _GstMsdkVideoAllocator
 
   GstMsdkContext *context;
   GstVideoInfo image_info;
+  mfxFrameAllocResponse mfx_response;
   mfxFrameAllocResponse *alloc_response;
 };
 
@@ -172,6 +173,7 @@ struct _GstMsdkDmaBufAllocator
 
   GstMsdkContext *context;
   GstVideoInfo image_info;
+  mfxFrameAllocResponse mfx_response;
   mfxFrameAllocResponse *alloc_response;
 };