GNOME Bugzilla – Bug 774029
vaapiwindow: wayland: fix surface leaks.
Last modified: 2017-04-24 11:35:51 UTC
Reproduce step: 1. running GST_DEBUG="GST_TRACER:7,2" GST_TRACERS="leaks" gst-play-1.0 sample.mp4 on *Wayland*. 2. Just quit. We can see the following log. 0:00:01.269680384 465 0x1c0aac0 TRACE GST_TRACER :0:: object-alive, type-name=(string)GstVaapiDisplayWayland, address=(gpointer)0x7fc964064230, description=(string)<vaapidisplaywayland1>, ref-count=(uint)4, trace=(string); Note that this leak happens only whe the tested media is rendering with crop. In addition, this leak leads to crash when playing videos in playlist by gst-play. Reproduce step: 1. Running gst-play-1.0 sample1.mp4 sample2.mp4 on *Wayland* 2. Press 'n' to skip to next video. 3. Crashed after about 1 sec. This is because VaapiDisplayWayland is not destroyed properly(because of leak) when skipping to the next one.
Created attachment 339219 [details] [review] libs: window: wayland: fix leaks of surfaces during destory When destroying window, it should release surfaces, which was created to filter out.
Review of attachment 339219 [details] [review]: ::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.c @@ +384,2 @@ { wl_buffer_destroy (wl_buffer); I don't understand this change. Please check the commit e4ed64b7, when this was done because the wayland protocol is heavily async. @@ +528,3 @@ + if (need_vpp && priv->use_vpp) { + gst_vaapi_object_unref (surface); + } Here you are hiding the problem. The surface should be unref when the callback is called from the following wl_buffer_destroy() Perhaps we have to ensure that the buffer queue is destroyed at error.
Review of attachment 339219 [details] [review]: ::: gst-libs/gst/vaapi/gstvaapiwindow_wayland.c @@ +383,3 @@ frame_release_callback (void *data, struct wl_buffer *wl_buffer) { wl_buffer_destroy (wl_buffer); That's what I was wondering. The problem in this test case (which is simply using vaapisink on gst-launch) is, The frame_release_callback for the last wl_buffer is always not called. What I understand is that this callback is usually called later, probably when the next buffer is shown. I need to investigate more for proper fix. @@ +528,3 @@ + if (need_vpp && priv->use_vpp) { + gst_vaapi_object_unref (surface); + } I don't understand. There's no callback to unref this surface at this moment, since it is before even creating frame and adding listener to the wl buffer.
(In reply to Hyunjun Ko from comment #3) > > @@ +528,3 @@ > + if (need_vpp && priv->use_vpp) { > + gst_vaapi_object_unref (surface); > + } > > I don't understand. > There's no callback to unref this surface at this moment, since it is before > even creating frame and adding listener to the wl buffer. And it should be gst_vaapi_video_pool_put_object instead of gst_vaapi_object_unref to return it to the surface pool.
check the patch in bug 773689 I just closed it, but I'm having second thoughts. What do you think?
commit ca314a25ccf81c9740d147c6b20de4c24b354301 Author: Hyunjun Ko <zzoon@igalia.com> Date: Wed Apr 19 10:37:19 2017 +0900 libs: window: wayland: null buffer at destroy() Fix leakage of the last wl buffer. VAAPI wayland sink needs to send a null buffer while destruction, it assures that all the wl buffers are released. Otherwise, the last buffer's callback might be not called, which leads to leak of GstVaapiDisplay. This was inspired by gstwaylandsink. https://bugzilla.gnome.org/show_bug.cgi?id=774029 Signed-off-by: Víctor Manuel Jáquez Leal <vjaquez@igalia.com>