After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 747482 - waylandsink: need exception code in gst_wayland_sink_set_window_handle() and gst_wayland_sink_set_context()
waylandsink: need exception code in gst_wayland_sink_set_window_handle() and ...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-04-08 02:29 UTC by Hyunil Park
Modified: 2015-11-03 10:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] waylandsink : Add exception code for setting wl_display and wl_surface (1.46 KB, patch)
2015-04-09 04:26 UTC, Hyunil Park
committed Details | Review
waylandsink: add exception code for setting wl_display and wl_surface (1.52 KB, patch)
2015-05-04 00:27 UTC, Hyunil Park
none Details | Review

Description Hyunil Park 2015-04-08 02:29:37 UTC
Waylandsink need exception code in gst_wayland_sink_set_window_handle().
After making sink->window, User can call gst_wayland_sink_set_window_handle().
It is user's fault. but Waylandsink need to exception.
If Waylandsink does not exception then sink->window is changed and rendering is failed.
So Could you add some code as below.

static void
gst_wayland_sink_set_window_handle (GstVideoOverlay * overlay, guintptr handle)
{
  GstWaylandSink *sink = GST_WAYLAND_SINK (overlay);
  struct wl_surface *surface = (struct wl_surface *) handle;

  g_return_if_fail (sink != NULL);

+  if (sink->window != NULL){
+  	return;
+  }

  g_mutex_lock (&sink->render_lock);

  GST_DEBUG_OBJECT (sink, "Setting window handle %" GST_PTR_FORMAT,

Waylandsink need exception code in gst_wayland_sink_set_context()
After calling gst_wayland_sink_set_context(), below code is set.
GST_ELEMENT_CLASS (parent_class)->set_context (element, context);
but, If user can call onemore. It is user's fault. but waylandsink need to exception.
So Could you add some code as below.
static void
gst_wayland_sink_set_context (GstElement * element, GstContext * context)
{
  GstWaylandSink *sink = GST_WAYLAND_SINK (element);
  GST_INFO(" ");
  if (gst_context_has_context_type (context,
          GST_WAYLAND_DISPLAY_HANDLE_CONTEXT_TYPE)) {
    g_mutex_lock (&sink->display_lock);
    if (G_LIKELY (!sink->display)) {
      gst_wayland_sink_set_display_from_context (sink, context);
    } else {
      GST_WARNING_OBJECT (element, "changing display handle is not supported");
+	  g_mutex_unlock (&sink->display_lock);
+	  return ;
  	}
    g_mutex_unlock (&sink->display_lock);
  }
  GST_ERROR("element %p context %p", element, context);
  if (GST_ELEMENT_CLASS (parent_class)->set_context)
    GST_ELEMENT_CLASS (parent_class)->set_context (element, context);
}


Thank you
Comment 1 Tim-Philipp Müller 2015-04-08 07:47:50 UTC
Are you planning on making a proper patch? (Ideally against git master)
Comment 2 Hyunil Park 2015-04-08 08:45:30 UTC
Okay,I can do that.
Comment 3 Hyunil Park 2015-04-09 04:26:58 UTC
Created attachment 301176 [details] [review]
[PATCH] waylandsink : Add exception code for setting wl_display and wl_surface

Waylandsink need exception code in gst_wayland_sink_set_window_handle().
After making sink->window, User can call gst_wayland_sink_set_window_handle().
It is user's fault. but Waylandsink need to exception.
If Waylandsink does not exception then sink->window is changed and rendering is failed.

Waylandsink need exception code in gst_wayland_sink_set_context()
After calling gst_wayland_sink_set_context(), below code is set.
GST_ELEMENT_CLASS (parent_class)->set_context (element, context);
but, If user can call onemore. It is user's fault. but waylandsink need to exception.

--- a/ext/wayland/gstwaylandsink.c
+++ b/ext/wayland/gstwaylandsink.c
@@ -370,10 +370,13 @@ gst_wayland_sink_set_context (GstElement * element, GstContext * context)
   if (gst_context_has_context_type (context,
           GST_WAYLAND_DISPLAY_HANDLE_CONTEXT_TYPE)) {
     g_mutex_lock (&sink->display_lock);
-    if (G_LIKELY (!sink->display))
+    if (G_LIKELY (!sink->display)) {
       gst_wayland_sink_set_display_from_context (sink, context);
-    else
+    } else {
       GST_WARNING_OBJECT (element, "changing display handle is not supported");
+      g_mutex_unlock (&sink->display_lock);
+      return;
+    }
     g_mutex_unlock (&sink->display_lock);
   }
 
@@ -730,6 +733,11 @@ gst_wayland_sink_set_window_handle (GstVideoOverlay * overlay, guintptr handle)
 
   g_return_if_fail (sink != NULL);
 
+  if (sink->window != NULL) {
+    GST_WARNING_OBJECT (sink, "changing window handle is not supported");
+    return;
+  }
+
   g_mutex_lock (&sink->render_lock);
 
   GST_DEBUG_OBJECT (sink, "Setting window handle %" GST_PTR_FORMAT,
Comment 4 Reynaldo H. Verdejo Pinochet 2015-05-01 23:19:59 UTC
Review of attachment 301176 [details] [review]:

Please fix commit message: Start with "waylandsink: " upfront,
drop the capital A in Add and add the bug URL on another line
Comment 5 Hyunil Park 2015-05-04 00:27:44 UTC
Created attachment 302827 [details] [review]
waylandsink: add exception code for setting wl_display and wl_surface

Dear. Reynaldo H. Verdejo Pinochet

I fix the commit message and added new patch base on git latest source code.
thank you.
Comment 6 Hyunil Park 2015-11-03 01:25:34 UTC
I saw the version 1.6.0
I think that it is need.
Comment 7 Luis de Bethencourt 2015-11-03 10:47:25 UTC
Review of attachment 301176 [details] [review]:

Sorry this got lost in the noise until now.

I split it into 2 patches since it is two different changes. Each commit should be one atomic fix.
I also fixed the commit message, so you can see the format for your next contribution :)

Thanks for the fix!

commit c9aaa4189b147314129e67adb0e20c1275c19051
Author: Hyunil Park <hyunil46.park@samsung.com>
Date:   Tue Nov 3 10:42:40 2015 +0000

    waylandsink: Add exception code for setting wl_surface

    Waylandsink needs exception code in gst_wayland_sink_set_window_handle().
    After making sink->window, User can call
    gst_wayland_sink_set_window_handle(). It is the user's fault, but
    Waylandsink needs to handle the exception, if not then sink->window is
    changed and rendering fails.

    https://bugzilla.gnome.org/show_bug.cgi?id=747482

commit d3db3df320de4a8450d2c23039d99dd0a0ddcca9
Author: Hyunil Park <hyunil46.park@samsung.com>
Date:   Thu Apr 9 13:17:01 2015 +0900

    waylandsink: Add exception code for setting wl_display

    Waylandsink needs exception code in gst_wayland_sink_set_context(). After
    calling gst_wayland_sink_set_context(), below code is set.
    GST_ELEMENT_CLASS (parent_class)->set_context (element, context); but, If
    user can call onemore. It is user's fault. but waylandsink need to
    exception.

    https://bugzilla.gnome.org/show_bug.cgi?id=747482