gst/playback/gstplaybin2.c: Get the object data correct so that we can remove our channels correctly.

Original commit message from CVS:
* gst/playback/gstplaybin2.c: (pad_added_cb), (pad_removed_cb),
(no_more_pads_cb):
Get the object data correct so that we can remove our channels
correctly.
* gst/playback/gstplaysink.c: (gen_video_chain), (gen_audio_chain),
(gen_vis_chain), (gst_play_sink_reconfigure),
(gst_play_sink_request_pad):
Add option to disable async behaviour in the sinks when possible. This
makes it possible to avoid an audio queue when dealing with
visualisations.
Add option to add a queue for the audio path.
* tests/examples/seek/seek.c: (clear_streams), (update_streams),
(main):
Disable the vis checkbox to match the defaults of playbin2.
Only get the stream info when we need to.

ChangeLog

@@ -1,3 +1,23 @@
+2008-02-18  Wim Taymans  <wim.taymans@collabora.co.uk>
+
+	* gst/playback/gstplaybin2.c: (pad_added_cb), (pad_removed_cb),
+	(no_more_pads_cb):
+	Get the object data correct so that we can remove our channels
+	correctly.
+
+	* gst/playback/gstplaysink.c: (gen_video_chain), (gen_audio_chain),
+	(gen_vis_chain), (gst_play_sink_reconfigure),
+	(gst_play_sink_request_pad):
+	Add option to disable async behaviour in the sinks when possible. This
+	makes it possible to avoid an audio queue when dealing with
+	visualisations.
+	Add option to add a queue for the audio path.
+
+	* tests/examples/seek/seek.c: (clear_streams), (update_streams),
+	(main):
+	Disable the vis checkbox to match the defaults of playbin2.
+	Only get the stream info when we need to.
+
 2008-02-17  Sebastian Dröge  <slomo@circular-chaos.org>
 
 	* ext/gio/gstgiobasesink.c: (gst_gio_base_sink_stop),

gst/playback/gstplaybin2.c

@@ -1285,6 +1285,7 @@ pad_added_cb (GstElement * decodebin, GstPad * pad, GstSourceGroup * group)
     g_object_set_data (G_OBJECT (sinkpad), "playbin2.select", select);
 
     /* store the pad in the array */
+    GST_DEBUG_OBJECT (playbin, "pad %p added to array", sinkpad);
     g_ptr_array_add (select->channels, sinkpad);
 
     res = gst_pad_link (pad, sinkpad);
@@ -1336,16 +1337,16 @@ pad_removed_cb (GstElement * decodebin, GstPad * pad, GstSourceGroup * group)
   GST_DEBUG_OBJECT (playbin,
       "pad %s:%s removed from group %p", GST_DEBUG_PAD_NAME (pad), group);
 
-  if ((select = g_object_get_data (G_OBJECT (pad), "playbin2.select"))) {
-    /* remove the pad from the array */
-    g_ptr_array_remove (select->channels, pad);
-    GST_DEBUG_OBJECT (playbin, "pad removed from array");
-  }
-
   /* get the selector sinkpad */
   if (!(peer = g_object_get_data (G_OBJECT (pad), "playbin2.sinkpad")))
     goto not_linked;
 
+  if ((select = g_object_get_data (G_OBJECT (peer), "playbin2.select"))) {
+    /* remove the pad from the array */
+    g_ptr_array_remove (select->channels, peer);
+    GST_DEBUG_OBJECT (playbin, "pad %p removed from array", peer);
+  }
+
   /* unlink the pad now (can fail, the pad is unlinked before it's removed) */
   gst_pad_unlink (pad, peer);
 

gst/playback/gstplaysink.c

@@ -59,10 +59,11 @@ typedef struct
 typedef struct
 {
   GstPlayChain chain;
-  GstElement *conv;
   GstElement *queue;
+  GstElement *conv;
   GstElement *scale;
   GstElement *sink;
+  gboolean async;
 } GstPlayVideoChain;
 
 typedef struct
@@ -717,14 +718,14 @@ activate_chain (GstPlayChain * chain, gboolean activate)
 /* make the element (bin) that contains the elements needed to perform
  * video display. 
  *
- *  +------------------------------------------------+
+ *  +------------------------------------------------------------+
- *  | vbin                                           |
+ *  | vbin                                                       |
- *  |      +----------+   +----------+   +---------+ |
+ *  |      +-------+   +----------+   +----------+   +---------+ |
- *  |      |colorspace|   |videoscale|   |videosink| |
+ *  |      | queue |   |colorspace|   |videoscale|   |videosink| |
- *  |   +-sink       src-sink       src-sink       | |
+ *  |   +-sink    src-sink       src-sink       src-sink       | |
- *  |   |  +----------+   +----------+   +---------+ |
+ *  |   |  +-------+   +----------+   +----------+   +---------+ |
- * sink-+                                            |
+ * sink-+                                                        |
- *  +------------------------------------------------+
+ *  +------------------------------------------------------------+
  *           
  */
 static GstPlayChain *
@@ -748,8 +749,13 @@ gen_video_chain (GstPlaySink * playsink, gboolean raw, gboolean async)
       goto no_sinks;
   }
 
-  /* FIXME, proxy property in sinks */
+  /* if we can disable async behaviour of the sink, we can avoid adding a
-  /* g_object_set (chain->sink, "async", async, NULL); */
+   * queue for the audio chain. */
+  if (g_object_class_find_property (G_OBJECT_GET_CLASS (chain->sink), "async")) {
+    g_object_set (chain->sink, "async", async, NULL);
+    chain->async = async;
+  } else
+    chain->async = TRUE;
 
   /* create a bin to hold objects, as we create them we add them to this bin so
    * that when something goes wrong we only need to unref the bin */
@@ -927,17 +933,17 @@ no_overlay:
  * We add a tee as the first element so that we can link the visualisation chain
  * to it when requested.
  *
- *  +-----------------------------------------------------------------------+
+ *  +-------------------------------------------------------------+
- *  | abin                                                                  |
+ *  | abin                                                        |
- *  |      +-----+   +---------+   +----------+   +---------+   +---------+ |
+ *  |      +---------+   +----------+   +---------+   +---------+ |
- *  |      | tee |   |audioconv|   |audioscale|   | volume  |   |audiosink| |
+ *  |      |audioconv|   |audioscale|   | volume  |   |audiosink| |
- *  |   +-sink  src-sink      src-sink       src-sink      src-sink       | |
+ *  |   +-srck      src-sink       src-sink      src-sink       | |
- *  |   |  +-----+   +---------+   +----------+   +---------+   +---------+ |
+ *  |   |  +---------+   +----------+   +---------+   +---------+ |
- * sink-+                                                                   |
+ * sink-+                                                         |
- *  +-----------------------------------------------------------------------+
+ *  +-------------------------------------------------------------+
  */
 static GstPlayChain *
-gen_audio_chain (GstPlaySink * playsink, gboolean raw)
+gen_audio_chain (GstPlaySink * playsink, gboolean raw, gboolean queue)
 {
   GstPlayAudioChain *chain;
   GstBin *bin;
@@ -963,10 +969,15 @@ gen_audio_chain (GstPlaySink * playsink, gboolean raw)
   gst_object_sink (bin);
   gst_bin_add (bin, chain->sink);
 
-  if (raw) {
+  if (queue) {
+    /* we have to add a queue when we need to decouple for the video sink in
+     * visualisations */
+    GST_DEBUG_OBJECT (playsink, "adding audio queue");
     chain->queue = gst_element_factory_make ("queue", "aqueue");
     gst_bin_add (bin, chain->queue);
+  }
 
+  if (raw) {
     chain->conv = gst_element_factory_make ("audioconvert", "aconv");
     if (chain->conv == NULL)
       goto no_audioconvert;
@@ -977,9 +988,7 @@ gen_audio_chain (GstPlaySink * playsink, gboolean raw)
       goto no_audioresample;
     gst_bin_add (bin, chain->resample);
 
-    /* FIXME, remove queue */
+    res = gst_element_link_pads (chain->conv, "src", chain->resample, "sink");
-    res = gst_element_link_pads (chain->queue, "src", chain->conv, "sink");
-    res &= gst_element_link_pads (chain->conv, "src", chain->resample, "sink");
 
     /* FIXME check if the sink has the volume property */
 
@@ -1004,9 +1013,19 @@ gen_audio_chain (GstPlaySink * playsink, gboolean raw)
     if (!res)
       goto link_failed;
 
-    pad = gst_element_get_pad (chain->queue, "sink");
+    if (queue) {
+      res = gst_element_link_pads (chain->queue, "src", chain->conv, "sink");
+      pad = gst_element_get_pad (chain->queue, "sink");
+    } else {
+      pad = gst_element_get_pad (chain->conv, "sink");
+    }
   } else {
-    pad = gst_element_get_pad (chain->sink, "sink");
+    if (queue) {
+      res = gst_element_link_pads (chain->queue, "src", chain->sink, "sink");
+      pad = gst_element_get_pad (chain->queue, "sink");
+    } else {
+      pad = gst_element_get_pad (chain->sink, "sink");
+    }
   }
   chain->chain.sinkpad = gst_ghost_pad_new ("sink", pad);
   gst_object_unref (pad);
@@ -1086,6 +1105,8 @@ gen_vis_chain (GstPlaySink * playsink)
   gst_object_ref (bin);
   gst_object_sink (bin);
 
+  /* we're queuing raw audio here, we can remove this queue when we can disable
+   * async behaviour in the video sink. */
   chain->queue = gst_element_factory_make ("queue", "visqueue");
   gst_bin_add (bin, chain->queue);
 
@@ -1227,10 +1248,17 @@ gst_play_sink_reconfigure (GstPlaySink * playsink)
   if (need_video) {
     GST_DEBUG_OBJECT (playsink, "adding video, raw %d",
         playsink->video_pad_raw);
-    if (!playsink->videochain)
+    if (!playsink->videochain) {
-      playsink->videochain =
+      gboolean raw, async;
-          gen_video_chain (playsink, need_vis ? TRUE : playsink->video_pad_raw,
+
-          !need_vis);
+      /* we need a raw sink when we do vis or when we have a raw pad */
+      raw = need_vis ? TRUE : playsink->video_pad_raw;
+      /* we try to set the sink async=FALSE when we need vis, this way we can
+       * avoid a queue in the audio chain. */
+      async = !need_vis;
+
+      playsink->videochain = gen_video_chain (playsink, raw, async);
+    }
     add_chain (playsink->videochain, TRUE);
     activate_chain (playsink->videochain, TRUE);
     if (!need_vis)
@@ -1247,9 +1275,25 @@ gst_play_sink_reconfigure (GstPlaySink * playsink)
 
   if (need_audio) {
     GST_DEBUG_OBJECT (playsink, "adding audio");
-    if (!playsink->audiochain)
+    if (!playsink->audiochain) {
-      playsink->audiochain =
+      gboolean raw, queue;
-          gen_audio_chain (playsink, playsink->audio_pad_raw);
+
+      /* get a raw sink if we are asked for a raw pad */
+      raw = playsink->audio_pad_raw;
+      if (need_vis) {
+        /* If we are dealing with visualisations, we need to add a queue to
+         * decouple the audio from the video part. We only have to do this when
+         * the video part is async=true */
+        queue = ((GstPlayVideoChain *) playsink->videochain)->async;
+        GST_DEBUG_OBJECT (playsink, "need audio queue for vis");
+      } else {
+        /* no vis, we can avoid a queue */
+        GST_DEBUG_OBJECT (playsink, "don't need audio queue");
+        queue = FALSE;
+      }
+
+      playsink->audiochain = gen_audio_chain (playsink, raw, queue);
+    }
     add_chain (playsink->audiochain, TRUE);
     gst_pad_link (playsink->audio_tee_asrc, playsink->audiochain->sinkpad);
     activate_chain (playsink->audiochain, TRUE);
@@ -1334,10 +1378,10 @@ gst_play_sink_request_pad (GstPlaySink * playsink, GstPlaySinkType type)
         playsink->audio_tee_sink =
             gst_element_get_pad (playsink->audio_tee, "sink");
         /* get two request pads */
-        playsink->audio_tee_asrc =
-            gst_element_get_request_pad (playsink->audio_tee, "src%d");
         playsink->audio_tee_vissrc =
             gst_element_get_request_pad (playsink->audio_tee, "src%d");
+        playsink->audio_tee_asrc =
+            gst_element_get_request_pad (playsink->audio_tee, "src%d");
         gst_bin_add (GST_BIN_CAST (playsink), playsink->audio_tee);
         gst_element_set_state (playsink->audio_tee, GST_STATE_PAUSED);
       }

tests/examples/seek/seek.c

@@ -86,6 +86,7 @@ static guint seek_timeout_id = 0;
 static gulong changed_id;
 
 static gint n_video = 0, n_audio = 0, n_text = 0;
+static gboolean need_streams = TRUE;
 static GtkWidget *video_combo, *audio_combo, *text_combo;
 static GtkWidget *vis_checkbox, *video_checkbox, *audio_checkbox;
 static GtkWidget *text_checkbox, *mute_checkbox, *volume_spinbutton;
@@ -1532,6 +1533,8 @@ clear_streams (GstElement * pipeline)
   gtk_widget_set_sensitive (video_combo, FALSE);
   gtk_widget_set_sensitive (audio_combo, FALSE);
   gtk_widget_set_sensitive (text_combo, FALSE);
+
+  need_streams = TRUE;
 }
 
 static void
@@ -1539,7 +1542,7 @@ update_streams (GstPipeline * pipeline)
 {
   gint i;
 
-  if (pipeline_type == 16) {
+  if (pipeline_type == 16 && need_streams) {
     GstTagList *tags;
     gchar *name;
     gint active_idx;
@@ -1586,6 +1589,8 @@ update_streams (GstPipeline * pipeline)
     }
     gtk_widget_set_sensitive (text_combo, n_text > 0);
     gtk_combo_box_set_active (GTK_COMBO_BOX (text_combo), active_idx);
+
+    need_streams = FALSE;
   }
 }
 
@@ -1890,7 +1895,7 @@ main (int argc, char **argv)
     audio_checkbox = gtk_check_button_new_with_label ("Audio");
     text_checkbox = gtk_check_button_new_with_label ("Text");
     mute_checkbox = gtk_check_button_new_with_label ("Mute");
-    volume_spinbutton = gtk_spin_button_new_with_range (0, 2.0, 0.1);
+    volume_spinbutton = gtk_spin_button_new_with_range (0, 5.0, 0.1);
     gtk_spin_button_set_value (GTK_SPIN_BUTTON (volume_spinbutton), 1.0);
     gtk_box_pack_start (GTK_BOX (boxes), vis_checkbox, TRUE, TRUE, 2);
     gtk_box_pack_start (GTK_BOX (boxes), audio_checkbox, TRUE, TRUE, 2);