From ce4f9ae53193d4de614e4fa26d4302e82bfc270a Mon Sep 17 00:00:00 2001 From: Haihao Xiang Date: Mon, 9 Sep 2019 13:40:53 +0800 Subject: [PATCH] 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 --- sys/msdk/gstmsdkallocator_libva.c | 11 +++++++++++ sys/msdk/gstmsdkcontext.h | 1 + sys/msdk/gstmsdkvideomemory.c | 28 ++++++++++++++++++++++++++-- sys/msdk/gstmsdkvideomemory.h | 2 ++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/sys/msdk/gstmsdkallocator_libva.c b/sys/msdk/gstmsdkallocator_libva.c index 81f6ce7025..3ddcbb8b20 100644 --- 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 f7c8b661b3..f115b407d2 100644 --- 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 6e173fa905..21d4981837 100644 --- 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 e7a8594ed4..93b9b45738 100644 --- 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; };