[Bug 731525] multiple glimagesink elements aborts due to no XInitThreads.

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=731525
  GStreamer | gst-plugins-bad | git

--- Comment #4 from Matthew Waters <ystreet00 at gmail.com> 2014-06-13 09:13:37 UTC ---
(In reply to comment #3)
> I'm curious why egl/x11 works fine whereas not for glx. Would it be possible to
> fix glx ? Or is there a deep incompatibility ?

The following points as I've iterated above, contribute to the incompatibility.
1. X Display's are not thread safe
2. Context sharing for glx on Intel drivers requires the display to be the same
3. These multiple contexts share the same display connection, which are
accessed from different threads.
4. xcb aborts the app.

The way to fix is to call XInitThreads() to provide the synchronisation
structures to let the X server deal with multiple concurrent access to an X
display connection from multiple threads.  That's how every other display
server out there deals with this (without the function call).

> Oki I have not considered the case sharing with external gl context, and when
> you are required to pass the X11 Display connection from the external tool kit
> (through gst_gl_ensure_display /gst_gl_display_x11_new_with_display). So in
> that case that means only those functions are concerned:
> gst_gl_context_glx_create_context/gst_gl_context_glx_destroy_context
> (creation/destruction)
> Others like gst_gl_context_glx_swap_buffers are not called because its bound to
> the external context managed by the UI tool kit.

No, that's wrong.  It's all glX functions called by GstGLContextGLX can
potentially abort the app.

> So we could pass the external locking mechanism. Cairo will pass
> cairo_device_acquire/release, clutter will pass clutter_threads_add_idle_full
> (clutter_threads_enter/leave are deprecated). Passed through gstcontext like
> for the X11 connection.
> For cairo it's simple to wrap it with gst_gl_display_acquire_handle/release.
> It's also possible for clutter but it requires more abstraction like the
> following:
> 
> I think for a GstGLContext that wraps an external GL context, we should still
> be able to call gst_gl_context_thread_add on it. For the clutter case it will
> internally call clutter_threads_add_idle_full+a_blocking_mecanism. We could
> define a func prototype and then the user needs to implement them and pass to
> gstgl through gstcontext like for X11 connection. (for well known UI tool kit
> we could predefine some implementations to help the user)

And get an optional dependency on clutter or cairo, no thanks.

> Then in gst_gl_context_create we would need to call gst_gl_context_thread_add
> (external_gstglcontext, ), so basically we create the gstgl internal context
> into the external thread, and then we create the gl thread and make the
> internal gl context current in there as usual.
> 
> Anyway I think at some point it will be useful to have
> gst_gl_context_thread_add support on wrapped external gl context. Also because
> it will enable the possibility that gstgl do not create any internal GL
> context, just use the external one. It's just to be flexible. Especially if the
> driver does not support context sharing, so we should provide this possibility
> too even if it's not the optimal way.

The entire gst_gl_context_thread_add should disappear entirely and is not at
all relevant to this discussion.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.