gst/playback/: Don't error out if there is no fakesink in the READY to NULL state change, since when decodebin is re-...

Original commit message from CVS:
* gst/playback/gstdecodebin.c: (add_fakesink),
(gst_decode_bin_change_state):
* gst/playback/gstdecodebin2.c: (add_fakesink),
(gst_decode_bin_change_state):
Don't error out if there is no fakesink in the READY to NULL state
change, since when decodebin is re-used, we're only adding the
fakesink element in READY to PAUSED.
* tests/check/elements/decodebin.c:
(new_decoded_pad_plug_fakesink_cb), (GST_START_TEST),
(decodebin_suite):
Minimal unit test to make sure we can use the same decodebin
instance twice (at least with audiotestsrc input).

ChangeLog

@@ -1,3 +1,19 @@
+2007-02-09  Tim-Philipp Müller  <tim at centricular dot net>
+
+	* gst/playback/gstdecodebin.c: (add_fakesink),
+	(gst_decode_bin_change_state):
+	* gst/playback/gstdecodebin2.c: (add_fakesink),
+	(gst_decode_bin_change_state):
+	  Don't error out if there is no fakesink in the READY to NULL state
+	  change, since when decodebin is re-used, we're only adding the
+	  fakesink element in READY to PAUSED.
+
+	* tests/check/elements/decodebin.c:
+	(new_decoded_pad_plug_fakesink_cb), (GST_START_TEST),
+	(decodebin_suite):
+	  Minimal unit test to make sure we can use the same decodebin
+	  instance twice (at least with audiotestsrc input).
+
 2007-02-09  Tim-Philipp Müller  <tim at centricular dot net>
 
 	* ext/alsa/gstalsa.c: (gst_alsa_find_device_name):

gst/playback/gstdecodebin.c

@@ -131,7 +131,7 @@ static void gst_decode_bin_finalize (GObject * object);
 static GstStateChangeReturn gst_decode_bin_change_state (GstElement * element,
     GstStateChange transition);
 
-static void add_fakesink (GstDecodeBin * decode_bin);
+static gboolean add_fakesink (GstDecodeBin * decode_bin);
 static void remove_fakesink (GstDecodeBin * decode_bin);
 
 static void dynamic_free (GstDynamic * dyn);
@@ -566,11 +566,11 @@ free_pad_probes (GstDecodeBin * decode_bin)
   decode_bin->probes = NULL;
 }
 
-static void
+static gboolean
 add_fakesink (GstDecodeBin * decode_bin)
 {
   if (decode_bin->fakesink != NULL)
-    return;
+    return TRUE;
 
   g_mutex_lock (decode_bin->cb_mutex);
 
@@ -588,14 +588,14 @@ add_fakesink (GstDecodeBin * decode_bin)
     decode_bin->fakesink = NULL;
   }
   g_mutex_unlock (decode_bin->cb_mutex);
-  return;
+  return TRUE;
 
   /* ERRORS */
 no_fakesink:
   {
     g_warning ("can't find fakesink element, decodebin will not work");
     g_mutex_unlock (decode_bin->cb_mutex);
-    return;
+    return FALSE;
   }
 }
 
@@ -1667,19 +1667,16 @@ gst_decode_bin_change_state (GstElement * element, GstStateChange transition)
       decode_bin->numpads = 0;
       decode_bin->numwaiting = 0;
       decode_bin->dynamics = NULL;
-      /* catch fatal errors that may have occured in the init function */
-      if (decode_bin->typefind == NULL || decode_bin->fakesink == NULL) {
-        GST_ELEMENT_ERROR (decode_bin, CORE, MISSING_PLUGIN, (NULL), (NULL));
-        return GST_STATE_CHANGE_FAILURE;
-      }
+      if (decode_bin->typefind == NULL)
+        goto missing_typefind;
       break;
     case GST_STATE_CHANGE_READY_TO_PAUSED:
       GST_OBJECT_LOCK (decode_bin);
       decode_bin->shutting_down = FALSE;
       GST_OBJECT_UNLOCK (decode_bin);
 
-      add_fakesink (decode_bin);
-      break;
+      if (!add_fakesink (decode_bin))
+        goto missing_fakesink;
       break;
     case GST_STATE_CHANGE_PAUSED_TO_PLAYING:
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
@@ -1711,6 +1708,21 @@ gst_decode_bin_change_state (GstElement * element, GstStateChange transition)
   }
 
   return ret;
+/* ERRORS */
+missing_typefind:
+  {
+    gst_element_post_message (element,
+        gst_missing_element_message_new (element, "typefind"));
+    GST_ELEMENT_ERROR (element, CORE, MISSING_PLUGIN, (NULL), ("no typefind!"));
+    return GST_STATE_CHANGE_FAILURE;
+  }
+missing_fakesink:
+  {
+    gst_element_post_message (element,
+        gst_missing_element_message_new (element, "fakesink"));
+    GST_ELEMENT_ERROR (element, CORE, MISSING_PLUGIN, (NULL), ("no fakesink!"));
+    return GST_STATE_CHANGE_FAILURE;
+  }
 }
 
 static gboolean

gst/playback/gstdecodebin2.c

@@ -122,7 +122,7 @@ GST_ELEMENT_DETAILS ("Decoder Bin",
     "Edward Hervey <edward@fluendo.com>");
 
 
-static void add_fakesink (GstDecodeBin * decode_bin);
+static gboolean add_fakesink (GstDecodeBin * decode_bin);
 static void remove_fakesink (GstDecodeBin * decode_bin);
 
 static void type_found (GstElement * typefind, guint probability,
@@ -1800,13 +1800,13 @@ gst_decode_pad_new (GstDecodeGroup * group, GstPad * pad, gboolean block)
  * ::get_state .
  */
 
-static void
+static gboolean
 add_fakesink (GstDecodeBin * decode_bin)
 {
   GST_DEBUG_OBJECT (decode_bin, "Adding the fakesink");
 
   if (decode_bin->fakesink)
-    return;
+    return TRUE;
 
   decode_bin->fakesink =
       gst_element_factory_make ("fakesink", "async-fakesink");
@@ -1820,20 +1820,20 @@ add_fakesink (GstDecodeBin * decode_bin)
   if (!gst_bin_add (GST_BIN (decode_bin), decode_bin->fakesink))
     goto could_not_add;
 
-  return;
+  return TRUE;
 
   /* ERRORS */
 no_fakesink:
   {
     g_warning ("can't find fakesink element, decodebin will not work");
-    return;
+    return FALSE;
   }
 could_not_add:
   {
     g_warning ("Could not add fakesink to decodebin, decodebin will not work");
     gst_object_unref (decode_bin->fakesink);
     decode_bin->fakesink = NULL;
-    return;
+    return FALSE;
   }
 }
 
@@ -1888,14 +1888,12 @@ gst_decode_bin_change_state (GstElement * element, GstStateChange transition)
 
   switch (transition) {
     case GST_STATE_CHANGE_NULL_TO_READY:
-      /* catch fatal errors that may have occured in the init function */
-      if (dbin->typefind == NULL || dbin->fakesink == NULL) {
-        GST_ELEMENT_ERROR (dbin, CORE, MISSING_PLUGIN, (NULL), (NULL));
-        return GST_STATE_CHANGE_FAILURE;
-      }
+      if (dbin->typefind == NULL)
+        goto missing_typefind;
       break;
     case GST_STATE_CHANGE_READY_TO_PAUSED:{
-      add_fakesink (dbin);
+      if (!add_fakesink (dbin))
+        goto missing_fakesink;
       break;
     }
     default:
@@ -1907,6 +1905,22 @@ gst_decode_bin_change_state (GstElement * element, GstStateChange transition)
   /* FIXME : put some cleanup functions here.. if needed */
 
   return ret;
+
+/* ERRORS */
+missing_typefind:
+  {
+    gst_element_post_message (element,
+        gst_missing_element_message_new (element, "typefind"));
+    GST_ELEMENT_ERROR (dbin, CORE, MISSING_PLUGIN, (NULL), ("no typefind!"));
+    return GST_STATE_CHANGE_FAILURE;
+  }
+missing_fakesink:
+  {
+    gst_element_post_message (element,
+        gst_missing_element_message_new (element, "fakesink"));
+    GST_ELEMENT_ERROR (dbin, CORE, MISSING_PLUGIN, (NULL), ("no fakesink!"));
+    return GST_STATE_CHANGE_FAILURE;
+  }
 }
 
 static gboolean

tests/check/elements/decodebin.c

@@ -102,6 +102,86 @@ GST_START_TEST (test_text_plain_streams)
 
 GST_END_TEST;
 
+static void
+new_decoded_pad_plug_fakesink_cb (GstElement * decodebin, GstPad * srcpad,
+    gboolean last, GstElement * pipeline)
+{
+  GstElement *sink;
+  GstPad *sinkpad;
+
+  sink = gst_element_factory_make ("fakesink", "sink");
+  fail_unless (sink != NULL, "Failed to create fakesink element");
+
+  gst_bin_add (GST_BIN (pipeline), sink);
+
+  sinkpad = gst_element_get_static_pad (sink, "sink");
+  fail_unless_equals_int (gst_pad_link (srcpad, sinkpad), GST_PAD_LINK_OK);
+  gst_object_unref (sinkpad);
+
+  gst_element_set_state (sink, GST_STATE_PLAYING);
+}
+
+GST_START_TEST (test_reuse_without_decoders)
+{
+  GstElement *pipe, *src, *decodebin, *sink;
+
+  pipe = gst_pipeline_new (NULL);
+  fail_unless (pipe != NULL, "failed to create pipeline");
+
+  src = gst_element_factory_make ("audiotestsrc", "src");
+  fail_unless (src != NULL, "Failed to create audiotestsrc element");
+
+  decodebin = gst_element_factory_make ("decodebin", "decodebin");
+  fail_unless (decodebin != NULL, "Failed to create decodebin element");
+
+  g_signal_connect (decodebin, "new-decoded-pad",
+      G_CALLBACK (new_decoded_pad_plug_fakesink_cb), pipe);
+
+  fail_unless (gst_bin_add (GST_BIN (pipe), src));
+  fail_unless (gst_bin_add (GST_BIN (pipe), decodebin));
+  fail_unless (gst_element_link (src, decodebin), "can't link src<->decodebin");
+
+  fail_unless_equals_int (gst_element_set_state (pipe, GST_STATE_READY),
+      GST_STATE_CHANGE_SUCCESS);
+  /* it's push-based, so should be async */
+  fail_unless_equals_int (gst_element_set_state (pipe, GST_STATE_PAUSED),
+      GST_STATE_CHANGE_ASYNC);
+
+  /* wait for state change to complete */
+  fail_unless_equals_int (gst_element_get_state (pipe, NULL, NULL, -1),
+      GST_STATE_CHANGE_SUCCESS);
+
+  /* there shouldn't be any errors */
+  fail_if (gst_bus_poll (GST_ELEMENT_BUS (pipe), GST_MESSAGE_ERROR, 0) != NULL);
+
+  /* reset */
+  gst_element_set_state (pipe, GST_STATE_NULL);
+
+  sink = gst_bin_get_by_name (GST_BIN (pipe), "sink");
+  gst_bin_remove (GST_BIN (pipe), sink);
+  gst_object_unref (sink);
+
+  GST_LOG ("second try");
+
+  fail_unless_equals_int (gst_element_set_state (pipe, GST_STATE_READY),
+      GST_STATE_CHANGE_SUCCESS);
+  /* it's push-based, so should be async */
+  fail_unless_equals_int (gst_element_set_state (pipe, GST_STATE_PAUSED),
+      GST_STATE_CHANGE_ASYNC);
+
+  /* wait for state change to complete */
+  fail_unless_equals_int (gst_element_get_state (pipe, NULL, NULL, -1),
+      GST_STATE_CHANGE_SUCCESS);
+
+  /* there shouldn't be any errors */
+  fail_if (gst_bus_poll (GST_ELEMENT_BUS (pipe), GST_MESSAGE_ERROR, 0) != NULL);
+
+  gst_element_set_state (pipe, GST_STATE_NULL);
+  gst_object_unref (pipe);
+}
+
+GST_END_TEST;
+
 static Suite *
 decodebin_suite (void)
 {
@@ -109,8 +189,8 @@ decodebin_suite (void)
   TCase *tc_chain = tcase_create ("general");
 
   suite_add_tcase (s, tc_chain);
-
   tcase_add_test (tc_chain, test_text_plain_streams);
+  tcase_add_test (tc_chain, test_reuse_without_decoders);
 
   return s;
 }