From 4e75ca0351750c65b5e7c7b4000b8c3de0bb0d50 Mon Sep 17 00:00:00 2001 From: Seungha Yang Date: Sun, 3 Jul 2022 01:18:19 +0900 Subject: [PATCH] d3d11videosink: Protect window with lock at every place Access to the object should be thread safe to support runtime property update Part-of: --- .../sys/d3d11/gstd3d11videosink.cpp | 136 ++++++++++-------- .../sys/d3d11/gstd3d11videosink.h | 1 + 2 files changed, 77 insertions(+), 60 deletions(-) diff --git a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp index 49a144ed66..901c51b1bc 100644 --- a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp +++ b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp @@ -128,11 +128,23 @@ struct _GstD3D11VideoSink /* For drawing on user texture */ gboolean drawing; GstBuffer *current_buffer; - GRecMutex draw_lock; + GRecMutex lock; gchar *title; }; +#define GST_D3D11_VIDEO_SINK_GET_LOCK(d) (&(GST_D3D11_VIDEO_SINK_CAST(d)->lock)) +#define GST_D3D11_VIDEO_SINK_LOCK(d) G_STMT_START { \ + GST_TRACE_OBJECT (d, "Locking from thread %p", g_thread_self()); \ + g_rec_mutex_lock (GST_D3D11_VIDEO_SINK_GET_LOCK (d)); \ + GST_TRACE_OBJECT (d, "Locked from thread %p", g_thread_self()); \ + } G_STMT_END + +#define GST_D3D11_VIDEO_SINK_UNLOCK(d) G_STMT_START { \ + GST_TRACE_OBJECT (d, "Unlocking from thread %p", g_thread_self()); \ + g_rec_mutex_unlock (GST_D3D11_VIDEO_SINK_GET_LOCK (d)); \ + } G_STMT_END + static void gst_d3d11_videosink_set_property (GObject * object, guint prop_id, const GValue * value, GParamSpec * pspec); static void gst_d3d11_videosink_get_property (GObject * object, guint prop_id, @@ -342,7 +354,7 @@ gst_d3d11_video_sink_init (GstD3D11VideoSink * self) self->fullscreen = DEFAULT_FULLSCREEN; self->draw_on_shared_texture = DEFAULT_DRAW_ON_SHARED_TEXTURE; - g_rec_mutex_init (&self->draw_lock); + g_rec_mutex_init (&self->lock); } static void @@ -351,7 +363,7 @@ gst_d3d11_videosink_set_property (GObject * object, guint prop_id, { GstD3D11VideoSink *self = GST_D3D11_VIDEO_SINK (object); - GST_OBJECT_LOCK (self); + GST_D3D11_VIDEO_SINK_LOCK (self); switch (prop_id) { case PROP_ADAPTER: self->adapter = g_value_get_int (value); @@ -390,7 +402,7 @@ gst_d3d11_videosink_set_property (GObject * object, guint prop_id, G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; } - GST_OBJECT_UNLOCK (self); + GST_D3D11_VIDEO_SINK_UNLOCK (self); } static void @@ -399,6 +411,7 @@ gst_d3d11_videosink_get_property (GObject * object, guint prop_id, { GstD3D11VideoSink *self = GST_D3D11_VIDEO_SINK (object); + GST_D3D11_VIDEO_SINK_LOCK (self); switch (prop_id) { case PROP_ADAPTER: g_value_set_int (value, self->adapter); @@ -426,6 +439,7 @@ gst_d3d11_videosink_get_property (GObject * object, guint prop_id, G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; } + GST_D3D11_VIDEO_SINK_UNLOCK (self); } static void @@ -433,7 +447,7 @@ gst_d3d11_video_sink_finalize (GObject * object) { GstD3D11VideoSink *self = GST_D3D11_VIDEO_SINK (object); - g_rec_mutex_clear (&self->draw_lock); + g_rec_mutex_clear (&self->lock); g_free (self->title); G_OBJECT_CLASS (parent_class)->finalize (object); @@ -521,11 +535,22 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) self->caps_updated = FALSE; - if (!gst_d3d11_video_sink_prepare_window (self)) - goto no_window; + GST_D3D11_VIDEO_SINK_LOCK (self); + if (!gst_d3d11_video_sink_prepare_window (self)) { + GST_D3D11_VIDEO_SINK_UNLOCK (self); - if (!gst_video_info_from_caps (&self->info, caps)) - goto invalid_format; + GST_ELEMENT_ERROR (self, RESOURCE, NOT_FOUND, (nullptr), + ("Failed to open window.")); + + return FALSE; + } + + 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; + } video_width = GST_VIDEO_INFO_WIDTH (&self->info); video_height = GST_VIDEO_INFO_HEIGHT (&self->info); @@ -536,11 +561,15 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) * convert video width and height to a display width and height * using wd / hd = wv / hv * PARv / PARd */ - /* TODO: Get display PAR */ - if (!gst_video_calculate_display_ratio (&num, &den, video_width, - video_height, video_par_n, video_par_d, display_par_n, display_par_d)) - goto no_disp_ratio; + video_height, video_par_n, video_par_d, display_par_n, + display_par_d)) { + GST_D3D11_VIDEO_SINK_UNLOCK (self); + + GST_ELEMENT_ERROR (self, CORE, NEGOTIATION, (nullptr), + ("Error calculating the output display ratio of the video.")); + return FALSE; + } GST_DEBUG_OBJECT (self, "video width/height: %dx%d, calculated display ratio: %d/%d format: %s", @@ -577,25 +606,27 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) self->video_width = video_width; self->video_height = video_height; - if (GST_VIDEO_SINK_WIDTH (self) <= 0 || GST_VIDEO_SINK_HEIGHT (self) <= 0) - goto no_display_size; + if (GST_VIDEO_SINK_WIDTH (self) <= 0 || GST_VIDEO_SINK_HEIGHT (self) <= 0) { + GST_D3D11_VIDEO_SINK_UNLOCK (self); + + GST_ELEMENT_ERROR (self, CORE, NEGOTIATION, (nullptr), + ("Error calculating the output display ratio of the video.")); + return FALSE; + } - GST_OBJECT_LOCK (self); if (self->pending_render_rect) { GstVideoRectangle rect = self->render_rect; self->pending_render_rect = FALSE; - GST_OBJECT_UNLOCK (self); - gst_d3d11_window_set_render_rectangle (self->window, &rect); - } else { - GST_OBJECT_UNLOCK (self); } if (!gst_d3d11_window_prepare (self->window, GST_VIDEO_SINK_WIDTH (self), GST_VIDEO_SINK_HEIGHT (self), caps, &error)) { GstMessage *error_msg; + GST_D3D11_VIDEO_SINK_UNLOCK (self); + GST_ERROR_OBJECT (self, "cannot create swapchain"); error_msg = gst_message_new_error (GST_OBJECT_CAST (self), error, "Failed to prepare d3d11window"); @@ -610,33 +641,9 @@ gst_d3d11_video_sink_update_window (GstD3D11VideoSink * self, GstCaps * caps) g_clear_pointer (&self->title, g_free); } - return TRUE; + GST_D3D11_VIDEO_SINK_UNLOCK (self); - /* ERRORS */ -invalid_format: - { - GST_DEBUG_OBJECT (self, - "Could not locate image format from caps %" GST_PTR_FORMAT, caps); - return FALSE; - } -no_window: - { - GST_ELEMENT_ERROR (self, RESOURCE, NOT_FOUND, (NULL), - ("Failed to open window.")); - return FALSE; - } -no_disp_ratio: - { - GST_ELEMENT_ERROR (self, CORE, NEGOTIATION, (NULL), - ("Error calculating the output display ratio of the video.")); - return FALSE; - } -no_display_size: - { - GST_ELEMENT_ERROR (self, CORE, NEGOTIATION, (NULL), - ("Error calculating the output display ratio of the video.")); - return FALSE; - } + return TRUE; } static void @@ -704,6 +711,7 @@ gst_d3d11_video_sink_start (GstBaseSink * sink) return TRUE; } +/* called with lock */ static gboolean gst_d3d11_video_sink_prepare_window (GstD3D11VideoSink * self) { @@ -771,13 +779,11 @@ done: return FALSE; } - GST_OBJECT_LOCK (self); g_object_set (self->window, "force-aspect-ratio", self->force_aspect_ratio, "fullscreen-toggle-mode", self->fullscreen_toggle_mode, "fullscreen", self->fullscreen, "enable-navigation-events", self->enable_navigation_events, NULL); - GST_OBJECT_UNLOCK (self); g_signal_connect (self->window, "key-event", G_CALLBACK (gst_d3d11_video_sink_key_event), self); @@ -794,11 +800,14 @@ gst_d3d11_video_sink_stop (GstBaseSink * sink) GST_DEBUG_OBJECT (self, "Stop"); + GST_D3D11_VIDEO_SINK_LOCK (self); if (self->window) gst_d3d11_window_unprepare (self->window); - gst_clear_object (&self->device); gst_clear_object (&self->window); + GST_D3D11_VIDEO_SINK_UNLOCK (self); + + gst_clear_object (&self->device); g_clear_pointer (&self->title, g_free); @@ -942,8 +951,10 @@ gst_d3d11_video_sink_unlock (GstBaseSink * sink) { GstD3D11VideoSink *self = GST_D3D11_VIDEO_SINK (sink); + GST_D3D11_VIDEO_SINK_LOCK (self); if (self->window) gst_d3d11_window_unlock (self->window); + GST_D3D11_VIDEO_SINK_UNLOCK (self); return TRUE; } @@ -953,8 +964,10 @@ gst_d3d11_video_sink_unlock_stop (GstBaseSink * sink) { GstD3D11VideoSink *self = GST_D3D11_VIDEO_SINK (sink); + GST_D3D11_VIDEO_SINK_LOCK (self); if (self->window) gst_d3d11_window_unlock_stop (self->window); + GST_D3D11_VIDEO_SINK_UNLOCK (self); return TRUE; } @@ -982,12 +995,14 @@ gst_d3d11_video_sink_event (GstBaseSink * sink, GstEvent * event) title_string = std::string (title); } + GST_D3D11_VIDEO_SINK_LOCK (self); if (self->window) { gst_d3d11_window_set_title (self->window, title_string.c_str ()); } else { g_free (self->title); self->title = g_strdup (title_string.c_str ()); } + GST_D3D11_VIDEO_SINK_UNLOCK (self); g_free (title); } @@ -1068,7 +1083,7 @@ gst_d3d11_video_sink_show_frame (GstVideoSink * sink, GstBuffer * buf) gst_d3d11_window_show (self->window); if (self->draw_on_shared_texture) { - g_rec_mutex_lock (&self->draw_lock); + GST_D3D11_VIDEO_SINK_LOCK (self); self->current_buffer = buf; self->drawing = TRUE; @@ -1081,7 +1096,7 @@ gst_d3d11_video_sink_show_frame (GstVideoSink * sink, GstBuffer * buf) GST_LOG_OBJECT (self, "End drawing"); self->drawing = FALSE; self->current_buffer = nullptr; - g_rec_mutex_unlock (&self->draw_lock); + GST_D3D11_VIDEO_SINK_UNLOCK (self); } else { ret = gst_d3d11_window_render (self->window, buf); } @@ -1117,7 +1132,7 @@ gst_d3d11_video_sink_set_render_rectangle (GstVideoOverlay * overlay, gint x, GST_DEBUG_OBJECT (self, "render rect x: %d, y: %d, width: %d, height %d", x, y, width, height); - GST_OBJECT_LOCK (self); + GST_D3D11_VIDEO_SINK_LOCK (self); if (self->window) { GstVideoRectangle rect; @@ -1127,7 +1142,6 @@ gst_d3d11_video_sink_set_render_rectangle (GstVideoOverlay * overlay, gint x, rect.h = height; self->render_rect = rect; - GST_OBJECT_UNLOCK (self); gst_d3d11_window_set_render_rectangle (self->window, &rect); } else { @@ -1136,8 +1150,9 @@ gst_d3d11_video_sink_set_render_rectangle (GstVideoOverlay * overlay, gint x, self->render_rect.w = width; self->render_rect.h = height; self->pending_render_rect = TRUE; - GST_OBJECT_UNLOCK (self); } + + GST_D3D11_VIDEO_SINK_UNLOCK (self); } static void @@ -1145,9 +1160,10 @@ gst_d3d11_video_sink_expose (GstVideoOverlay * overlay) { GstD3D11VideoSink *self = GST_D3D11_VIDEO_SINK (overlay); - if (self->window && self->window->swap_chain) { - gst_d3d11_window_render (self->window, NULL); - } + GST_D3D11_VIDEO_SINK_LOCK (self); + if (self->window && self->window->swap_chain) + gst_d3d11_window_render (self->window, nullptr); + GST_D3D11_VIDEO_SINK_UNLOCK (self); } static void @@ -1205,10 +1221,10 @@ gst_d3d11_video_sink_draw_action (GstD3D11VideoSink * self, return FALSE; } - g_rec_mutex_lock (&self->draw_lock); + GST_D3D11_VIDEO_SINK_LOCK (self); if (!self->drawing || !self->current_buffer) { GST_WARNING_OBJECT (self, "Nothing to draw"); - g_rec_mutex_unlock (&self->draw_lock); + GST_D3D11_VIDEO_SINK_UNLOCK (self); return FALSE; } @@ -1220,7 +1236,7 @@ gst_d3d11_video_sink_draw_action (GstD3D11VideoSink * self, ret = gst_d3d11_window_render_on_shared_handle (self->window, self->current_buffer, shared_handle, texture_misc_flags, acquire_key, release_key); - g_rec_mutex_unlock (&self->draw_lock); + GST_D3D11_VIDEO_SINK_UNLOCK (self); return ret == GST_FLOW_OK; } diff --git a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.h b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.h index f67b3afa46..36a38361b2 100644 --- a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.h +++ b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.h @@ -37,6 +37,7 @@ G_BEGIN_DECLS #define GST_IS_D3D11_VIDEO_SINK(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj),GST_TYPE_D3D11_VIDEO_SINK)) #define GST_IS_D3D11_VIDEO_SINK_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass), GST_TYPE_D3D11_VIDEO_SINK)) #define GST_D3D11_VIDEO_SINK_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS((obj), GST_TYPE_D3D11_VIDEO_SINK, GstD3D11VideoSinkClass)) +#define GST_D3D11_VIDEO_SINK_CAST(obj) ((GstD3D11VideoSink *) obj) typedef struct _GstD3D11VideoSink GstD3D11VideoSink; typedef struct _GstD3D11VideoSinkClass GstD3D11VideoSinkClass;