GNOME Bugzilla – Bug 562170
GstBus watch doesn't work with non-default main context
Last modified: 2009-01-17 21:06:45 UTC
Some events are not delivered to a watcher when using a non-default GMainContext, like this: GMainContext *mainContext = g_main_context_new(); GMainLoop *mainLoop = g_main_loop_new(mainContext, FALSE); ... GSource *source = gst_bus_create_watch(bus); g_source_attach(source, mainContext); g_source_set_callback(source, (GSourceFunc)cb_bus_call, data, NULL); Many events are received once the pipeline runs, just not EOS or SEGMENT_DONE. If, instead, the default context is used, then it works fine: GMainLoop *mainLoop = g_main_loop_new(NULL, FALSE); GMainContext *mainContext = g_main_loop_get_context(mainLoop); ... GSource *source = gst_bus_create_watch(bus); g_source_attach(source, mainContext); g_source_set_callback(source, (GSourceFunc)cb_bus_call, data, NULL); The callback will receive all of the other events as usual, but this time EOS and SEGMENT_DONE will also be delivered too. An extensive workaround is to write your own GSource, which is described here: http://article.gmane.org/gmane.comp.video.gstreamer.devel/21665 I currently use the above method with success, but probably GStreamer should be fixed too.
I think this is a known issue of sorts - we only wake up the default main context when a message is posted on the bus. It's a bit tricky to know which contexts to wake up, since the source doesn't know when it gets attached to a main context, or which, so we have to query and keep track of that actively ourselves. I'll have a look at this.
Created attachment 123365 [details] [review] possible fix, more or less untested Does this help at all?
I patched this on 10.21 which took hand-correction, and it doesn't work. I can try to get a CVS build set up to test, if that would make any difference. For your own testing purposes, it is very easy to make the two line mainContext initialization change to one of the example playback programs and see whether you still get EOS or not. Thanks for looking at this. When I examined the patch and saw the GMainContext in global space, plus the "FIXME"s, it is clear the existing code is quite wrong. :)
Okay, guess I'll have to go and actually test this with a non-default context then :) > An extensive workaround is to write your own GSource, which is described here: > http://article.gmane.org/gmane.comp.video.gstreamer.devel/21665 > > I currently use the above method with success, but probably GStreamer should be > fixed too. I think the reason this seems to work correctly is that the source_prepare function implicitly sets/keeps *timeout=0, which means 'I'm ready to be dispatched now'. You could probably achieve the same effect by adding an idle or timeout source to your main context and use the standard GstBusSource.
Why not simply start a thread that pops and dispatches messages? why does it have to be a mainloop?
(In reply to comment #5) > Why not simply start a thread that pops and dispatches messages? why does it > have to be a mainloop? Yes, maybe that's a better workaround than the one I found on gstreamer-devel. But I think the natural way to use GstBus is with a mainloop, right?
I've committed this now, along with a unit test: 2008-12-27 Tim-Philipp Müller <tim.muller at collabora co uk> * gst/gstbus.c: (gst_bus_dispose), (gst_bus_get_property), (gst_bus_wakeup_main_context), (gst_bus_set_main_context), (gst_bus_post), (gst_bus_source_prepare), (gst_bus_source_finalize), (gst_bus_create_watch): Make GstBusSource work with non-default main contexts (#562170). * tests/check/gst/gstbus.c: (message_func_eos), (message_func_app), (test_watch), (test_watch_with_custom_context), (gst_bus_suite): Add test case for GstBusSource with a non-default main context. > I patched this on 10.21 which took hand-correction, and it doesn't work. I > can try to get a CVS build set up to test, if that would make any difference. Are you sure your application was actually using the patched libgstreamer? Maybe you could attach a GST_DEBUG=GST_BUS:5 log? > But I think the natural way to use GstBus is with a mainloop, right? Using a main loop only makes sense if there are multiple event sources being watched (e.g. GStreamer bus, Gtk+/X events etc.). If you're only ever waiting for GstBus messages anyway, you may just as well use gst_bus_*pop* directly.
Created attachment 126601 [details] [review] Patch fixing GstBusSource inheritance and unit test which fails w/o this patch Rookie mistake: GstBusSource didn't inherit from GSource directly, which overwrites bussource->inited when GLib initialises what it thinks is the GSource member at the very beginning of the struct. This leads to non-default main contexts not working in some or all circumstances (no idea why it's not triggered with the existing unit test). This should most likely go into the release, even though I don't think it actually causes bad memory access of any kind. Patch also contains a unit test which demonstrates the bug (hangs until timeout because the custom context is never woken up when the eos message is posted on the bus, because it's not set, which is because bsrc->inited is already non-zero as described above).
confirmed patch works
Agree this should go in... please commit.
Committed, thanks! (And thanks for the test case Justin!) 2009-01-17 Tim-Philipp Müller <tim.muller at collabora co uk> * gst/gstbus.c: (gst_bus_set_main_context), (gst_bus_create_watch): Fix order of members in GstBusSource structure - the first member must be the parent structure ie. GSource. Should make bus sources attached to non-default main contexts work in all cases now (ie. primarily in cases where the callback has a non-NULL user data argument). Fixes #562170. * tests/check/gst/gstbus.c: (test_custom_main_context): Add unit test for the above, based on code by Justin Karneges <justin at affinix com>.