GNOME Bugzilla – Bug 725048
gl: The display handle isn't freed correctly
Last modified: 2014-03-04 10:22:24 UTC
Created attachment 270102 [details] Test case Bug discovered when exploring the GstGlContext memory leak (https://bugzilla.gnome.org/show_bug.cgi?id=724816). Still after the correct release of GstGlContext, the display handler isn't freed correctly.
commit 103235a8ed2efb6cd8a7e5f6ce1d5a9e9815975d Author: Matthew Waters <ystreet00@gmail.com> Date: Mon Feb 24 23:55:58 2014 +1100 x11: close both of the display's we use (the comment doesn't seem to apply anymore) https://bugzilla.gnome.org/show_bug.cgi?id=725048
The previous correction doesn't fix the bug. I still got the message "Maximum number of clients reached" after a while.
(In reply to comment #2) > The previous correction doesn't fix the bug. I still got the message "Maximum > number of clients reached" after a while. Forget my previous comment. The specified modification causes a SEG FAULT in :
+ Trace 233234
See test case application. Note : Every case can be tested in the test application by update the demux_pad_added method.
b31a34a22ed8504fe4b8e645c2d3ef2ba1e122eb Author: Matthew Waters <ystreet00@gmail.com> Date: Fri Feb 28 17:42:51 2014 +1100 context: call window_class->close last We should destroy resources before closing the display connection Fixes https://bugzilla.gnome.org/show_bug.cgi?id=725048
Created attachment 270470 [details] Dead lock threads backtrace Dead appears following the commit : b31a34a22ed8504fe4b8e645c2d3ef2ba1e122eb
Using the test application, I observed a deadlock when the first pipeline is stopping. I start to wonder if the context should be share between the different pipeline and context. It is possible ? May it resolve the issue ?
I've never seen that occur on my system (intel or nvidia). It looks like the concurrent glXMakCurrent calls deadlock on each other which probably requires updating the GL/X stack. For me, your test application works fine most of the time :P. It usually segfaults in gstreamer-vaapi's display cache implementation on startup :/. If it gets past there, everything's fine.
Did you apply you patch against vaapidecode (unref the ref to the pool) ? Because after an update to MESA 10.0.3 I receive a SEGFAULT when the test application unref the pipeline. Back trace :
+ Trace 233253
Thread 4 (Thread 0x7ffff530f700 (LWP 10778))
So it looks like we free the gl context before gstreamer-vaapi is finished with it...and so mesa tries to create another one which involves __glXInitialize...which fails with the spectacular backtrace you have there. Essentially the problem is that we destroy too early and gstreamer-vaapi does not know the context has disappeared. One workaround is to delay context destruction inside the element which is what I think cluttersink/webkit has (probably indirectly). Ideally gstreamer-vaapi would hold a reference to the GL context that it's using however it does not know which context before GstVideoGLTextureUploadMeta::upload is called. (gdb) bt
+ Trace 233254
With my knowledge, I may suggest that the XCloseDisplay that you previously uncoment created this side effect. For me, the X11 display is closed when the GLContext is released but this operation perturbs the vaapi element. So the challenge here is (maybe - again with my little knowledge) to close the display at the very end of the releasing procedure. I will try on my side to apply this hypothesis, but you will have a better solution to resolve this issue.
You're application lives longer then your pipeline, can't you simply create your context before you create your first pipeline and share that. Then you'd keep that context open till the end of your application, when all your pipeline has reached NULL state.(See gst_element_set_context(), sequence of operation is described in GstContext description).
It's possible to have an example ?
Created attachment 270861 [details] test case with gstcontext This is your example with GstContext integration. However, GstGLContext is not propogated through GstContext (a little confusing, yes), but the GstGLDisplay is. Currently there isn't a way of getting the GstGLContext out of the pipeline. We might need to change that.
No, the GstContext version still segfaults with the same error. The display being closed is not the issue, it's the fact that we have destroyed the context that gstreamer-vaapi wants to use to destroy its GL objects.
Reverting the XCloseDisplay change makes things work again albiet with the display leak. However that's a side effect of the real issue.
Mathew, why isn't your context ref-counted combine with a foreign flag for when the display comes from the application ? That's how it used to be in the 0.10.
That's exactly how it happens within GstGLDisplay, it's just that each context/window creates their own Display Connection which is closed as a result of the context being destroyed before vaapi is done with it.
Hmm, ok, you may ignore me, I haven't looked at this code for a long time. So now we have multiple display connection per pipeline by default ? Also, if they are shared, why doesn't VAAPI take a ref on it ?
Yes, multiple by default. VAAPI doesn't take a ref because it doesn't know about the context and it hasn't needed to. Taking a ref on either the GstGLContext or the Display would work. The first requiring the least work. Just some background. The reason it is like this is, with GstVideoOverlay case, we cannot call XInitThreads for integration with gtk+. It will also segfault inside X11 on a lock unless XinitThreads is called before gtk_init which in general, does not happen. See tests/examples/gtk/filtervideooverlay/ for an example to test with. So instead, we use at least one Display connection per thread. If this turns out to be too complicated I'll move to requiring XInitThreads to be called.
Created attachment 270873 [details] [review] use the GObject type to propogate GstGLCOntext This works for me :). You'll also need the patches in the following bugs: https://bugzilla.gnome.org/show_bug.cgi?id=725642 https://bugzilla.gnome.org/show_bug.cgi?id=725643
Validated cases : Single pipeline : Vaapi elements only : OK Vaapi + GL : OK Vaapi + GL + Raw : OK Multiple pipelines : Vaapi elements only : OK Vaapi + GL : OK Vaapi + GL + Raw : OK Good job :)