d3d11videosink: Protect window with lock at every place

Access to the object should be thread safe to support runtime
property update

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2708>

diff --git a/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp b/subprojects/gst-plugins-bad/sys/d3d11/gstd3d11videosink.cpp
index 49a144e..901c51b 100644 (file)
--- 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);
+
+    GST_ELEMENT_ERROR (self, RESOURCE, NOT_FOUND, (nullptr),
+        ("Failed to open window."));
+
+    return FALSE;
+  }
 
-  if (!gst_video_info_from_caps (&self->info, caps))
-    goto invalid_format;
+  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 f67b3af..36a3836 100644 (file)
--- 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;