Created attachment 88485 [details] [review]
exemplary patch for gnobots

When I init XThreads, the error goes away. Does GStreamer access X even when only playing sound ?

I don't know if or why GStreamer accesses X when playing sounds. It could be because gnome-games uses Playbin from Gstreamer, which could output both audio and video.

But do you think think that we should commit this?  Could there be any portability-problems with the ifdef's that you've added?

I think we should ask the gstreamer guys for advice :)

> I don't know if or why GStreamer accesses X when playing sounds. It could be
> because gnome-games uses Playbin from Gstreamer, which could output both audio
> and video.

GStreamer should not access any X resources as long as the files you're playing only contain audio. Nevertheless, you might want to do something like this

  GstElement *fakevideosink;

  fakevideosink = gst_element_factory_make ("fakesink", "fakevideosink");
  g_object_set (playbin, "video-sink", fakevideosink, NULL);

to make sure it never ever uses a real video sink (e.g. via the autovideosink element which it will try to use if you don't specify a video sink explicitly). If GStreamer really does do X stuff for an audio file, that would be a bug. I'd be surprised if that was the problem though.

Unrelated to this bug, you might also want to set the audio-sink property to a gconfaudiosink element so the user's preferences are honoured if they differ from the default.

What might be related, however, is the way you run main loops from the sound thread functions. I'm not entirely sure how this works across several thread boundaries, but it looks like trouble to me :)  You create a main loop that iterates the default main context (NULL) and then g_main_loop_run(loop) that in your thread function. This is the same main context that Gtk+ uses for the main GUI thread, so you might end up dispatching gtk stuff from your playbin thread if I'm not mistaken (which I well might be).

As far as I can tell, you don't really need the main loop and the bus watch at all. If you're doing playback in a separate thread in a blocking way, you could just do something like this and get rid of the main loop/ bus watch stuff:

  if (!playbin_in_use) {
    gboolean done = FALSE;
    GstBus *bus;

    playbin_in_use = TRUE;  // maybe do atomically or with locking

    bus = gst_pipeline_get_bus (GST_PIPELINE (playbin));

    .. set uri ...
    .. set playbin to PLAYING state ..
    .. (and check return value for STATE_CHANE_FAILURE!) ..

    do {
      GstMessage *msg;

      /* wait for message on the bus */
      msg = gst_bus_timed_pop (bus, GST_CLOCK_TIME_NONE);

      switch (GST_MESSAGE_TYPE (msg)) {
        case GST_MESSAGE_EOS:
        case GST_MESSAGE_ERROR:
          done = TRUE;
          break;
        default:
          break;
      }
      gst_message_unref (msg);
    }
    while (!done);

    ... set playbin to NULL state ...
    ... cleanup ...
    playbin_in_use = FALSE;
  }


Hope this helps.

Thanks for the help, Tim-Phillip. Christian, could you please decrypt the above comment into a patch?  :-)

Thanks for the help, both of you. I have committed a fix, based on the comments from Tim-Philipp.