[Spice-devel] [PATCH spice-gtk] spice-widget: Make sure we can use X11 from different thread

[Frediano Ziglio]

> 
> Hi,
> 
> On 11/26/18 10:16 AM, Frediano Ziglio wrote:
> > In order to support GStreamer overlay this is necessary as some
> > plugins can use X11 from a different thread.
> >
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >   src/spice-widget.c | 13 +++++++++++++
> >   1 file changed, 13 insertions(+)
> >
> > CI result:
> > https://gitlab.freedesktop.org/fziglio/spice-gtk/pipelines/9559
> >
> > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > index 312c640a..cca0867e 100644
> > --- a/src/spice-widget.c
> > +++ b/src/spice-widget.c
> > @@ -2547,6 +2547,19 @@ static void queue_draw_area(SpiceDisplay *display,
> > gint x, gint y,
> >                                  x, y, width, height);
> >   }
> >   
> > +#ifdef GDK_WINDOWING_X11
> > +/* See
> > https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/GstGLDisplay.html
> > + * (look for XInitThreads). We call it into a constructor to make sure
> > + * we call before any X11 function.
> > + * In case of GStreamer some plugins will use X11 from different
> > + * threads.
> > + */
> > +SPICE_CONSTRUCTOR_FUNC(x11_threads_init)
> > +{
> > +    XInitThreads();
> > +}
> > +#endif
> > +
> 
> 
> So, I'd ack but i still have two small worries 1. would it harm to call
> it also when it's not really needed?
> 

In theory it could impact slightly performances as locks are used. On the
other way:
- how can you make sure program is not using X11 from multiple threads?
  You safely can't!
- if GStreamer is used (which now should) program is likely to use multiple
  thread so not calling this function is more likely to be a bug;
- do we really need all these performance to risk program crash?


> 2. Wouldn't be more correct to call it by gtk/gdk interface (i think
> teuf has mentioned such function)
> 

I did a search and somebody suggested to not use gdk_threads_init
which currently is deprecated.

Waiting for a comment from teuf too.

> 
> Thanks, Snir.
> 
> 
> >   static void* prepare_streaming_mode(SpiceChannel *channel, bool
> >   streaming_mode, gpointer data)
> >   {
> >   #ifdef GDK_WINDOWING_X11

Frediano