GNOME Bugzilla – Bug 595427
avoid x event thread if not needed
Last modified: 2009-10-07 15:01:14 UTC
The attached patch starts/stops the event thread in xvimagesink as needed. This reduces context switches. Right now the thread polls the event 20 times a second. I have this applied in my system and see no regression so far. Would be nice to get a review and some testing.
Created attachment 143320 [details] [review] make the event thread conditial
Looks good but those GST_WARNINGs should really be GST_DEBUG, right? :)
Also, isn't it possible to let the X event thread block until an event happens or until some timeout happens? Polling is never a good idea :)
Created attachment 144398 [details] [review] make the event thread conditial GST_WARNING -> GST_DEBUG_OBJECT Regarding the polling. I am not an x-exper, but it looks like I could use XWindowEvent instead of XCheckWindowEvent. This needs rewriting the while loops in gst_xvimagesink_handle_xevents() so that there is a single loop. [1] http://tronche.com/gui/x/xlib/event-handling/manipulating-event-queue/XWindowEvent.html [2] http://tronche.com/gui/x/xlib/event-handling/manipulating-event-queue/XCheckWindowEvent.html
The reason it uses XWindowCheckEvent is precisely to avoid blocking. The thread putting buffers on the screen is trying to use that same X display connection. It will be stalled waiting for the input thread whenever the input thread is holding the xlock. It might be possible instead to poll/select on the X display file descriptor (XConnectionNumber()) and avoid using a timeout. It probably needs to be done in a way that uses GstPoll(), and disables it when the display thread is doing any X operations.
Jan, thanks for the info. Do you see any issue with the patch as it is. I would prefer to address the polling as a separate patch if at all.
No, the current patch to entirely enable/disable the polling thread seems like a good idea.
I am pushing this now to get some wider testing. commit 99b4483b8cc2584617131a468d38556ef0db154b Author: Stefan Kost <ensonic@users.sf.net> Date: Tue Sep 15 15:26:06 2009 +0300 xvimagesink: only start event thread if needed The event thread is doing 20 wakeups per second to poll the events. If one runs xvimagesink with handle-events=false and handle-expose=false then we can avoid the extra thread.
Shouldn't this also be fixed in ximagesink then?
Good catch and probably in vdpau and other derivatives too. I'll fix it in ximagesink as well.
Done for ximagesink too.