GNOME Bugzilla – Bug 748663
wayland: use a thread for event dispatching
Last modified: 2015-05-07 10:57:49 UTC
This patchset use one patch of bug #748564, to keep track of the created frames. Then a thread is added for event dispatching. The surface "done" callback is kept for throttling purposes and the frame is freed when the buffer is released. With these two first patches, the artifacts described in bug #748564 seems to be mitigated a lot, and parts of the pending patch in bug #747492 are included. The last patch was added because of the last comment in bug #748564: we cannot destroy a buffer in the wayland's queue. Hence this patch decouples the frame structure from the wl buffer. In this way we reduce the leaked memory to the queued buffers.
Created attachment 302601 [details] [review] wayland: free all frames on window destruction There can be more than one frame in play now, track all of them.
Created attachment 302602 [details] [review] wayland: move the display dispatch to a thread This patch moves to a thread the the queue dispatching, such as in waylandsink. This patch eliminates some artifacts seen in VC1 streams and reduces the leaking to a one last frame.
Created attachment 302603 [details] [review] wayland: decouple the wl_buffer from the frame This patch takes out the wayland's buffer from the the frame structure. The buffer is queued to wayland and destroyed in the "release" callback. The frame is freed in the surface's "done" callback, avoiding memory leaks.
The main problem with these patches is that it requires glib 2.38 and wayland 1.2. It meas that gstreamer-vaapi ought upgrade its requirements.
Review of attachment 302602 [details] [review]: ::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.c @@ -482,3 @@ - /* Wait for the previous frame to complete redraw */ - if (!gst_vaapi_window_wayland_sync (window)) - return FALSE; I think you have to wait here for the frame to be displayed before pushing the next one to do the throttling. Or am I missing something?
(In reply to Olivier Crête from comment #5) > Review of attachment 302602 [details] [review] [review]: > > ::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.c > @@ -482,3 @@ > - /* Wait for the previous frame to complete redraw */ > - if (!gst_vaapi_window_wayland_sync (window)) > - return FALSE; > > I think you have to wait here for the frame to be displayed before pushing > the next one to do the throttling. Or am I missing something? Perhaps I'm not doing it correctly, but the surface 'done' callback seems to do the throttling, with no need of calling the event dispatching.
Using an extra thread is a unneeded burden since the rendering is sync'ed. Hence I'm closing this issue as not-a-bug.