From 095ee27bb56b3aa9f265b34648f992b18a003e20 Mon Sep 17 00:00:00 2001 From: Seungha Yang Date: Thu, 15 Dec 2022 01:15:10 +0900 Subject: [PATCH] d3d11videosink: Fix deadlock when parent window is busy Deadlock sequence: * From a streaming thread, d3d11videosink sends synchronous message to the parent window, so that internal (child) window can be constructed on the parent window's thread * App thread (parent window thread) is waiting for pipeline's state change (to GST_STATE_NULL) but streaming thread is blocked and waiting for app thread To avoid the deadlock, GstD3D11WindowWin32 should send message to the parent window asynchronously. Part-of: --- .../sys/d3d11/gstd3d11videosink.cpp | 53 ++++--- .../sys/d3d11/gstd3d11window.cpp | 14 +- .../sys/d3d11/gstd3d11window.h | 4 +- .../sys/d3d11/gstd3d11window_dummy.cpp | 8 +- .../sys/d3d11/gstd3d11window_win32.cpp | 148 +++++++++++++++--- 5 files changed, 174 insertions(+), 53 deletions(-) diff --git a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp index 5cac8a24ee..1a28c938e8 100644 --- a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp +++ b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp @@ -526,7 +526,7 @@ gst_d3d11_video_sink_set_caps (GstBaseSink * sink, GstCaps * caps) return TRUE; } -static gboolean +static GstFlowReturn gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) { gint video_width, video_height; @@ -534,6 +534,8 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) gint display_par_n = 1, display_par_d = 1; /* display's PAR */ guint num, den; GError *error = NULL; + GstD3D11Window *window; + GstFlowReturn ret = GST_FLOW_OK; GST_DEBUG_OBJECT (self, "Updating window with caps %" GST_PTR_FORMAT, caps); @@ -546,14 +548,14 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) GST_ELEMENT_ERROR (self, RESOURCE, NOT_FOUND, (nullptr), ("Failed to open window.")); - return FALSE; + return GST_FLOW_ERROR; } if (!gst_video_info_from_caps (&self->info, caps)) { GST_DEBUG_OBJECT (self, "Could not locate image format from caps %" GST_PTR_FORMAT, caps); GST_D3D11_VIDEO_SINK_UNLOCK (self); - return FALSE; + return GST_FLOW_ERROR; } video_width = GST_VIDEO_INFO_WIDTH (&self->info); @@ -572,7 +574,7 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) GST_ELEMENT_ERROR (self, CORE, NEGOTIATION, (nullptr), ("Error calculating the output display ratio of the video.")); - return FALSE; + return GST_FLOW_ERROR; } GST_DEBUG_OBJECT (self, @@ -615,7 +617,7 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) GST_ELEMENT_ERROR (self, CORE, NEGOTIATION, (nullptr), ("Error calculating the output display ratio of the video.")); - return FALSE; + return GST_FLOW_ERROR; } if (self->pending_render_rect) { @@ -626,20 +628,32 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) } self->have_video_processor = FALSE; - if (!gst_d3d11_window_prepare (self->window, GST_VIDEO_SINK_WIDTH (self), - GST_VIDEO_SINK_HEIGHT (self), caps, &self->have_video_processor, - &error)) { + window = (GstD3D11Window *) gst_object_ref (self->window); + GST_D3D11_VIDEO_SINK_UNLOCK (self); + + ret = gst_d3d11_window_prepare (window, GST_VIDEO_SINK_WIDTH (self), + GST_VIDEO_SINK_HEIGHT (self), caps, &self->have_video_processor, &error); + if (ret != GST_FLOW_OK) { GstMessage *error_msg; - GST_D3D11_VIDEO_SINK_UNLOCK (self); + if (ret == GST_FLOW_FLUSHING) { + GST_D3D11_VIDEO_SINK_LOCK (self); + GST_WARNING_OBJECT (self, "Couldn't prepare window but we are flushing"); + gst_clear_object (&self->window); + gst_object_unref (window); + GST_D3D11_VIDEO_SINK_UNLOCK (self); + + return GST_FLOW_FLUSHING; + } GST_ERROR_OBJECT (self, "cannot create swapchain"); error_msg = gst_message_new_error (GST_OBJECT_CAST (self), error, "Failed to prepare d3d11window"); g_clear_error (&error); gst_element_post_message (GST_ELEMENT (self), error_msg); + gst_object_unref (window); - return FALSE; + return GST_FLOW_ERROR; } if (self->fallback_pool) { @@ -675,19 +689,20 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) if (!self->fallback_pool) { GST_ERROR_OBJECT (self, "Failed to configure fallback pool"); - return FALSE; + gst_object_unref (window); + return GST_FLOW_ERROR; } self->processor_in_use = FALSE; if (self->title) { - gst_d3d11_window_set_title (self->window, self->title); + gst_d3d11_window_set_title (window, self->title); g_clear_pointer (&self->title, g_free); } - GST_D3D11_VIDEO_SINK_UNLOCK (self); + gst_object_unref (window); - return TRUE; + return GST_FLOW_OK; } static void @@ -807,6 +822,9 @@ done: g_signal_connect (self->window, "mouse-event", G_CALLBACK (gst_d3d11_video_mouse_key_event), self); + GST_DEBUG_OBJECT (self, + "Have prepared window %" GST_PTR_FORMAT, self->window); + return TRUE; } @@ -1197,17 +1215,16 @@ gst_d3d11_video_sink_show_frame (GstVideoSink * sink, GstBuffer * buf) if (self->caps_updated || !self->window) { GstCaps *caps = gst_pad_get_current_caps (GST_BASE_SINK_PAD (sink)); - gboolean update_ret; /* shouldn't happen */ if (!caps) return GST_FLOW_NOT_NEGOTIATED; - update_ret = gst_d3d11_video_sink_update_window (self, caps); + ret = gst_d3d11_video_sink_update_window (self, caps); gst_caps_unref (caps); - if (!update_ret) - return GST_FLOW_NOT_NEGOTIATED; + if (ret != GST_FLOW_OK) + return ret; } if (!gst_d3d11_buffer_can_access_device (buf, device_handle)) { diff --git a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window.cpp b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window.cpp index 25f8ab1b78..8fde250040 100644 --- a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window.cpp +++ b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window.cpp @@ -107,7 +107,7 @@ static GstFlowReturn gst_d3d111_window_present (GstD3D11Window * self, ID3D11RenderTargetView * rtv); static void gst_d3d11_window_on_resize_default (GstD3D11Window * window, guint width, guint height); -static gboolean gst_d3d11_window_prepare_default (GstD3D11Window * window, +static GstFlowReturn gst_d3d11_window_prepare_default (GstD3D11Window * window, guint display_width, guint display_height, GstCaps * caps, gboolean * video_processor_available, GError ** error); @@ -417,14 +417,14 @@ typedef struct gboolean supported; } GstD3D11WindowDisplayFormat; -gboolean +GstFlowReturn gst_d3d11_window_prepare (GstD3D11Window * window, guint display_width, guint display_height, GstCaps * caps, gboolean * video_processor_available, GError ** error) { GstD3D11WindowClass *klass; - g_return_val_if_fail (GST_IS_D3D11_WINDOW (window), FALSE); + g_return_val_if_fail (GST_IS_D3D11_WINDOW (window), GST_FLOW_ERROR); klass = GST_D3D11_WINDOW_GET_CLASS (window); g_assert (klass->prepare != NULL); @@ -436,7 +436,7 @@ gst_d3d11_window_prepare (GstD3D11Window * window, guint display_width, video_processor_available, error); } -static gboolean +static GstFlowReturn gst_d3d11_window_prepare_default (GstD3D11Window * window, guint display_width, guint display_height, GstCaps * caps, gboolean * video_processor_available, GError ** error) @@ -496,7 +496,7 @@ gst_d3d11_window_prepare_default (GstD3D11Window * window, guint display_width, GST_ERROR_OBJECT (window, "Cannot determine render format"); g_set_error (error, GST_RESOURCE_ERROR, GST_RESOURCE_ERROR_FAILED, "Cannot determine render format"); - return FALSE; + return GST_FLOW_ERROR; } for (i = 0; i < GST_VIDEO_INFO_N_COMPONENTS (&window->info); i++) { @@ -765,12 +765,12 @@ gst_d3d11_window_prepare_default (GstD3D11Window * window, guint display_width, GST_DEBUG_OBJECT (window, "New swap chain 0x%p created", window->swap_chain); - return TRUE; + return GST_FLOW_OK; error: gst_d3d11_device_unlock (window->device); - return FALSE; + return GST_FLOW_ERROR; } void diff --git a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window.h b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window.h index 8d35b4ef8c..39bce78d66 100644 --- a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window.h +++ b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window.h @@ -153,7 +153,7 @@ struct _GstD3D11WindowClass guint width, guint height); - gboolean (*prepare) (GstD3D11Window * window, + GstFlowReturn (*prepare) (GstD3D11Window * window, guint display_width, guint display_height, GstCaps * caps, @@ -185,7 +185,7 @@ void gst_d3d11_window_set_render_rectangle (GstD3D11Window * window, void gst_d3d11_window_set_title (GstD3D11Window * window, const gchar *title); -gboolean gst_d3d11_window_prepare (GstD3D11Window * window, +GstFlowReturn gst_d3d11_window_prepare (GstD3D11Window * window, guint display_width, guint display_height, GstCaps * caps, diff --git a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window_dummy.cpp b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window_dummy.cpp index 1c53313563..b877cf46ee 100644 --- a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window_dummy.cpp +++ b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window_dummy.cpp @@ -48,7 +48,7 @@ G_DEFINE_TYPE (GstD3D11WindowDummy, gst_d3d11_window_dummy, static void gst_d3d11_window_dummy_on_resize (GstD3D11Window * window, guint width, guint height); -static gboolean gst_d3d11_window_dummy_prepare (GstD3D11Window * window, +static GstFlowReturn gst_d3d11_window_dummy_prepare (GstD3D11Window * window, guint display_width, guint display_height, GstCaps * caps, gboolean * video_processor_available, GError ** error); static void gst_d3d11_window_dummy_unprepare (GstD3D11Window * window); @@ -80,7 +80,7 @@ gst_d3d11_window_dummy_init (GstD3D11WindowDummy * self) { } -static gboolean +static GstFlowReturn gst_d3d11_window_dummy_prepare (GstD3D11Window * window, guint display_width, guint display_height, GstCaps * caps, gboolean * video_processor_available, GError ** error) @@ -192,12 +192,12 @@ gst_d3d11_window_dummy_prepare (GstD3D11Window * window, gst_d3d11_device_unlock (window->device); - return TRUE; + return GST_FLOW_OK; error: gst_d3d11_device_unlock (window->device); - return FALSE; + return GST_FLOW_ERROR; } static void diff --git a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window_win32.cpp b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window_win32.cpp index a74a1bb519..71ff5a0f0a 100644 --- a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window_win32.cpp +++ b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11window_win32.cpp @@ -93,6 +93,9 @@ struct _GstD3D11WindowWin32 /* Handle set_render_rectangle */ GstVideoRectangle render_rect; + + gboolean flushing; + gboolean setup_external_hwnd; }; #define gst_d3d11_window_win32_parent_class parent_class @@ -120,17 +123,21 @@ gst_d3d11_window_win32_create_internal_window (GstD3D11WindowWin32 * self); static void gst_d3d11_window_win32_destroy_internal_window (HWND hwnd); static void gst_d3d11_window_win32_release_external_handle (HWND hwnd); static void -gst_d3d11_window_win32_set_window_handle (GstD3D11WindowWin32 * self, - guintptr handle); -static void gst_d3d11_window_win32_on_resize (GstD3D11Window * window, guint width, guint height); +static GstFlowReturn gst_d3d11_window_win32_prepare (GstD3D11Window * window, + guint display_width, guint display_height, GstCaps * caps, + gboolean * video_processor_available, GError ** error); static void gst_d3d11_window_win32_unprepare (GstD3D11Window * window); static void gst_d3d11_window_win32_set_render_rectangle (GstD3D11Window * window, const GstVideoRectangle * rect); static void gst_d3d11_window_win32_set_title (GstD3D11Window * window, const gchar * title); +static gboolean gst_d3d11_window_win32_unlock (GstD3D11Window * window); +static gboolean gst_d3d11_window_win32_unlock_stop (GstD3D11Window * window); +static GstFlowReturn +gst_d3d11_window_win32_set_external_handle (GstD3D11WindowWin32 * self); static void gst_d3d11_window_win32_class_init (GstD3D11WindowWin32Class * klass) @@ -152,12 +159,16 @@ gst_d3d11_window_win32_class_init (GstD3D11WindowWin32Class * klass) window_class->present = GST_DEBUG_FUNCPTR (gst_d3d11_window_win32_present); window_class->on_resize = GST_DEBUG_FUNCPTR (gst_d3d11_window_win32_on_resize); + window_class->prepare = GST_DEBUG_FUNCPTR (gst_d3d11_window_win32_prepare); window_class->unprepare = GST_DEBUG_FUNCPTR (gst_d3d11_window_win32_unprepare); window_class->set_render_rectangle = GST_DEBUG_FUNCPTR (gst_d3d11_window_win32_set_render_rectangle); window_class->set_title = GST_DEBUG_FUNCPTR (gst_d3d11_window_win32_set_title); + window_class->unlock = GST_DEBUG_FUNCPTR (gst_d3d11_window_win32_unlock); + window_class->unlock_stop = + GST_DEBUG_FUNCPTR (gst_d3d11_window_win32_unlock_stop); } static void @@ -176,7 +187,9 @@ gst_d3d11_window_win32_constructed (GObject * object) GstD3D11WindowWin32 *self = GST_D3D11_WINDOW_WIN32 (object); if (window->external_handle) { - gst_d3d11_window_win32_set_window_handle (self, window->external_handle); + /* Will setup internal child window on ::prepare() */ + self->setup_external_hwnd = TRUE; + window->initialized = TRUE; goto done; } @@ -200,6 +213,51 @@ gst_d3d11_window_win32_dispose (GObject * object) G_OBJECT_CLASS (parent_class)->dispose (object); } +static GstFlowReturn +gst_d3d11_window_win32_prepare (GstD3D11Window * window, guint display_width, + guint display_height, GstCaps * caps, gboolean * video_processor_available, + GError ** error) +{ + GstD3D11WindowWin32 *self = GST_D3D11_WINDOW_WIN32 (window); + HWND hwnd; + GstFlowReturn ret; + + if (!self->setup_external_hwnd) + goto done; + + hwnd = (HWND) window->external_handle; + if (!IsWindow (hwnd)) { + GST_ERROR_OBJECT (self, "Invalid window handle"); + g_set_error (error, GST_RESOURCE_ERROR, GST_RESOURCE_ERROR_FAILED, + "Invalid window handle"); + return GST_FLOW_ERROR; + } + + self->overlay_state = GST_D3D11_WINDOW_WIN32_OVERLAY_STATE_NONE; + self->external_hwnd = hwnd; + + GST_DEBUG_OBJECT (self, "Preparing external handle"); + ret = gst_d3d11_window_win32_set_external_handle (self); + if (ret != GST_FLOW_OK) { + if (ret == GST_FLOW_FLUSHING) { + GST_WARNING_OBJECT (self, "Flushing"); + return GST_FLOW_FLUSHING; + } + + GST_ERROR_OBJECT (self, "Couldn't configure internal window"); + g_set_error (error, GST_RESOURCE_ERROR, GST_RESOURCE_ERROR_FAILED, + "Window handle configuration failed"); + return GST_FLOW_ERROR; + } + + GST_DEBUG_OBJECT (self, "External handle got prepared"); + self->setup_external_hwnd = FALSE; + +done: + return GST_D3D11_WINDOW_CLASS (parent_class)->prepare (window, display_width, + display_height, caps, video_processor_available, error); +} + static void gst_d3d11_window_win32_unprepare (GstD3D11Window * window) { @@ -310,6 +368,36 @@ gst_d3d11_window_win32_finalize (GObject * object) G_OBJECT_CLASS (parent_class)->finalize (object); } +static gboolean +gst_d3d11_window_win32_unlock (GstD3D11Window * window) +{ + GstD3D11WindowWin32 *self = GST_D3D11_WINDOW_WIN32 (window); + g_mutex_lock (&self->lock); + + GST_DEBUG_OBJECT (self, "Unlock"); + + self->flushing = TRUE; + g_cond_broadcast (&self->cond); + g_mutex_unlock (&self->lock); + + return TRUE; +} + +static gboolean +gst_d3d11_window_win32_unlock_stop (GstD3D11Window * window) +{ + GstD3D11WindowWin32 *self = GST_D3D11_WINDOW_WIN32 (window); + g_mutex_lock (&self->lock); + + GST_DEBUG_OBJECT (self, "Unlock stop"); + + self->flushing = FALSE; + g_cond_broadcast (&self->cond); + g_mutex_unlock (&self->lock); + + return TRUE; +} + static gboolean running_cb (gpointer user_data) { @@ -401,10 +489,11 @@ gst_d3d11_window_win32_destroy_internal_window (HWND hwnd) ", 0x%x", (guintptr) hwnd, (guint) GetLastError ()); } -static void +static GstFlowReturn gst_d3d11_window_win32_set_external_handle (GstD3D11WindowWin32 * self) { WNDPROC external_window_proc; + GstFlowReturn ret = GST_FLOW_OK; external_window_proc = (WNDPROC) GetWindowLongPtrA (self->external_hwnd, GWLP_WNDPROC); @@ -419,9 +508,28 @@ gst_d3d11_window_win32_set_external_handle (GstD3D11WindowWin32 * self) SetWindowLongPtrA (self->external_hwnd, GWLP_WNDPROC, (LONG_PTR) sub_class_proc); - /* Will create our internal window on parent window's thread */ - SendMessageA (self->external_hwnd, WM_GST_D3D11_CONSTRUCT_INTERNAL_WINDOW, + /* SendMessage() may cause deadlock if parent window thread is busy + * for changing pipeline's state. Post our message instead, and wait for + * the parent window's thread or flushing */ + PostMessageA (self->external_hwnd, WM_GST_D3D11_CONSTRUCT_INTERNAL_WINDOW, 0, 0); + + g_mutex_lock (&self->lock); + while (self->external_hwnd && + self->overlay_state == GST_D3D11_WINDOW_WIN32_OVERLAY_STATE_NONE && + !self->flushing) { + g_cond_wait (&self->cond, &self->lock); + } + + if (self->overlay_state != GST_D3D11_WINDOW_WIN32_OVERLAY_STATE_OPENED) { + if (self->flushing) + ret = GST_FLOW_FLUSHING; + else + ret = GST_FLOW_ERROR; + } + g_mutex_unlock (&self->lock); + + return ret; } static void @@ -827,6 +935,11 @@ sub_class_proc (HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) MoveWindow (self->internal_hwnd, rect.left, rect.top, rect.right, rect.bottom, FALSE); + g_mutex_lock (&self->lock); + self->overlay_state = GST_D3D11_WINDOW_WIN32_OVERLAY_STATE_OPENED; + g_cond_broadcast (&self->cond); + g_mutex_unlock (&self->lock); + /* don't need to be chained up to parent window procedure, * as this is our custom message */ return 0; @@ -840,13 +953,16 @@ sub_class_proc (HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) gst_d3d11_window_win32_release_external_handle (self->external_hwnd); self->external_hwnd = NULL; - RemovePropA (self->internal_hwnd, D3D11_WINDOW_PROP_NAME); - ShowWindow (self->internal_hwnd, SW_HIDE); - gst_d3d11_window_win32_destroy_internal_window (self->internal_hwnd); + if (self->internal_hwnd) { + RemovePropA (self->internal_hwnd, D3D11_WINDOW_PROP_NAME); + ShowWindow (self->internal_hwnd, SW_HIDE); + gst_d3d11_window_win32_destroy_internal_window (self->internal_hwnd); + } self->internal_hwnd = NULL; self->internal_hwnd_thread = NULL; self->overlay_state = GST_D3D11_WINDOW_WIN32_OVERLAY_STATE_CLOSED; + g_cond_broadcast (&self->cond); g_mutex_unlock (&self->lock); } else { gst_d3d11_window_win32_handle_window_proc (self, hWnd, uMsg, wParam, @@ -1022,18 +1138,6 @@ gst_d3d11_window_win32_create_swap_chain (GstD3D11Window * window, return TRUE; } -static void -gst_d3d11_window_win32_set_window_handle (GstD3D11WindowWin32 * self, - guintptr handle) -{ - self->overlay_state = GST_D3D11_WINDOW_WIN32_OVERLAY_STATE_NONE; - - self->external_hwnd = (HWND) handle; - gst_d3d11_window_win32_set_external_handle (self); - - self->overlay_state = GST_D3D11_WINDOW_WIN32_OVERLAY_STATE_OPENED; -} - static void gst_d3d11_window_win32_show (GstD3D11Window * window) {