GNOME Bugzilla – Bug 758984
gl/wayland: fix loop test hang in glimagesink
Last modified: 2018-01-04 04:41:21 UTC
Created attachment 316692 [details] [review] gl/wayland: fix loop test hang in glimagesink gl/wayland: fix loop test hang in glimagesink Don't call gst_gl_wl_display_roundtrip_queue() in window_show(). In glimagesink, gl thread will dispatch event queue and window_show() is called from streaming thread. Gl thread will empty event queue and potentially cause gst_gl_wl_display_roundtrip_queue() blocking the streaming thread to wait for an event occur. Actually, no event can occur because the swap_buffer event is queued by streaming thread but it is blocked. And also we should destroy event queue when destroy surfaces. This patch remove gst_gl_window_wayland_egl_show() from GstGLWindowWaylandEGLClass and call create_surfaces() gst_gl_wl_display_roundtrip_queue() in gst_gl_window_wayland_egl_open(). When when destroy surfaces, destroy event queue also. please help me review. Thanks.
Review of attachment 316692 [details] [review]: Moving create_surfaces() from _show() to _open() is not an option as that will break the GstVideoOverlay use case.
These days everything may implement the multi-threaded wayland event API which may help.
(In reply to Matthew Waters from comment #2) > These days everything may implement the multi-threaded wayland event API > which may help. You mean this issue will be fix by implement multi-threaded wayland event API in the next few days?
I'm not aware of anyone working on making the libgstgl/wayland code use the multi-threaded event loop API. The majority of the API already seems to exist in wayland though.
(In reply to Matthew Waters from comment #4) > I'm not aware of anyone working on making the libgstgl/wayland code use the > multi-threaded event loop API. The majority of the API already seems to > exist in wayland though. I have searched the wayland reference manual, can we create two event queues, one for gl thread and another for the window_show(). I think this will fix the dispatch conflict and also will not break the GstVideoOverlay use case.
(In reply to Matthew Waters from comment #4) > I'm not aware of anyone working on making the libgstgl/wayland code use the > multi-threaded event loop API. The majority of the API already seems to > exist in wayland though. And I searched the wayland master branch, it seems that the multi-threaded event loop API has not been included yet.
Created attachment 316832 [details] [review] gl/wayland: fix loop test hang in glimagesink use two event queue in wayland
Review of attachment 316832 [details] [review]: If we're just adding event queues for multi-threading issues, we should just push everything through the window thread with gst_gl_window_send_message{_async} () instead.
(In reply to Matthew Waters (ystreet00) from comment #8) > Review of attachment 316832 [details] [review] [review]: > > If we're just adding event queues for multi-threading issues, we should just > push everything through the window thread with > gst_gl_window_send_message{_async} () instead. I got your point, you mean we don't need to create another event queue but just put it run in gl thread by calling gst_gl_window_send_message{_async}(). But I just notice the gl thread will dispatch event queue. I wonder if this, why should we to call gst_gl_wl_display_roundtrip_queue() in _show() again, since the event on the created queue will automatically be dispatched by glthread?
Is this still an issue for you and are you working on it?
*** Bug 792156 has been marked as a duplicate of this bug. ***
*** Bug 788754 has been marked as a duplicate of this bug. ***
commit b25413fb1c3e2e442bfe8d4627e9cf7a9d313735 Author: Matthew Waters <matthew@centricular.com> Date: Thu Jan 4 15:33:33 2018 +1100 gl/wayland: move roundtrip on show to window thread This makes it thread safe and fixes a possible deadlock. Keeping the roundtrip off the window thread will result in two different threads call wl_display_dispatch_queue() for the same queue which violates the assumption for _dispatch_queue()'s thread-safety guarantees. https://bugzilla.gnome.org/show_bug.cgi?id=788754 https://bugzilla.gnome.org/show_bug.cgi?id=792156 https://bugzilla.gnome.org/show_bug.cgi?id=758984