check/generic/states.c: Add a sleep to ensure elements have a chance to start their pad tasks before shutdown. Reduce...

Original commit message from CVS:
* check/generic/states.c: (GST_START_TEST), (states_suite):
Add a sleep to ensure elements have a chance to start their
pad tasks before shutdown. Reduces racy test results.

* gst/elements/gstfdsrc.c: (gst_fdsrc_init), (gst_fdsrc_create):
Time out the select every now and then to check for shutdown.

check/generic/states.c

@@ -42,6 +42,8 @@ GST_START_TEST (test_state_changes)
     gst_element_set_state (element, GST_STATE_READY);
     gst_element_set_state (element, GST_STATE_PAUSED);
     gst_element_set_state (element, GST_STATE_PLAYING);
+    /* Sleep to give any pad tasks time to start */
+    g_usleep (0.2 * G_USEC_PER_SEC);
     gst_element_set_state (element, GST_STATE_PAUSED);
     gst_element_set_state (element, GST_STATE_READY);
     gst_element_set_state (element, GST_STATE_NULL);
@@ -63,6 +65,10 @@ states_suite (void)
   Suite *s = suite_create ("states");
   TCase *tc_chain = tcase_create ("general");
 
+  /* Use a long timeout, as we test all elements and take
+   * at least 0.2 seconds each */
+  tcase_set_timeout (tc_chain, 120);
+
   suite_add_tcase (s, tc_chain);
   tcase_add_test (tc_chain, test_state_changes);
 

common

@@ -1 +1 @@
-Subproject commit 13022c3cb4558d201e2ddf3e65d2e36b16eedc4a
+Subproject commit cd4da6a319d9f92d28f7b8a3b412577e6de50b64

gst/elements/gstfdsrc.c

@@ -138,7 +138,8 @@ gst_fdsrc_class_init (GstFdSrcClass * klass)
 static void
 gst_fdsrc_init (GstFdSrc * fdsrc, GstFdSrcClass * klass)
 {
-  // TODO set live only if it's actually a live source
+  /* TODO set live only if it's actually a live source (check
+   * for seekable fd) */
   gst_base_src_set_live (GST_BASE_SRC (fdsrc), TRUE);
 
   fdsrc->fd = 0;
@@ -226,12 +227,15 @@ gst_fdsrc_get_property (GObject * object, guint prop_id, GValue * value,
   }
 }
 
+#define SELECT_TIMEOUT (GST_SECOND / 20)
+
 static GstFlowReturn
 gst_fdsrc_create (GstPushSrc * psrc, GstBuffer ** outbuf)
 {
   GstFdSrc *src;
   GstBuffer *buf;
   glong readbytes;
+  GstClockTime timeout;
 
 #ifndef HAVE_WIN32
   fd_set readfds;
@@ -241,21 +245,32 @@ gst_fdsrc_create (GstPushSrc * psrc, GstBuffer ** outbuf)
 
   src = GST_FDSRC (psrc);
 
-  /* create the buffer */
-  buf = gst_buffer_new_and_alloc (src->blocksize);
-
 #ifndef HAVE_WIN32
   FD_ZERO (&readfds);
   FD_SET (src->fd, &readfds);
 
   if (src->timeout != 0) {
-    GST_TIME_TO_TIMEVAL (src->timeout, t);
+    timeout = MIN (SELECT_TIMEOUT, src->timeout);
-  } else
+  } else {
-    tp = NULL;
+    timeout = SELECT_TIMEOUT;
+  }
 
   do {
+    GST_TIME_TO_TIMEVAL (timeout, t);
+    if (src->timeout != 0)
+      timeout -= MIN (timeout, SELECT_TIMEOUT);
+
     retval = select (src->fd + 1, &readfds, NULL, NULL, tp);
-  } while (retval == -1 && errno == EINTR);     /* retry if interrupted */
+
+    /* Check whether the element got shutdown before full timeout */
+    if (retval == 0) {
+      if (GST_PAD_IS_FLUSHING (GST_BASE_SRC_PAD (src))) {
+        GST_DEBUG_OBJECT (src, "Shutting down with no buffer.");
+        return GST_FLOW_WRONG_STATE;
+      }
+    }
+  } while ((retval == -1 && errno == EINTR) ||  /* retry if interrupted */
+      (retval == 0 && timeout > 0));    /* Retry on incomplete timeout */
 
   if (retval == -1) {
     GST_ELEMENT_ERROR (src, RESOURCE, READ, (NULL),
@@ -269,6 +284,9 @@ gst_fdsrc_create (GstPushSrc * psrc, GstBuffer ** outbuf)
   }
 #endif
 
+  /* create the buffer */
+  buf = gst_buffer_new_and_alloc (src->blocksize);
+
   do {
     readbytes = read (src->fd, GST_BUFFER_DATA (buf), src->blocksize);
   } while (readbytes == -1 && errno == EINTR);  /* retry if interrupted */
@@ -286,11 +304,13 @@ gst_fdsrc_create (GstPushSrc * psrc, GstBuffer ** outbuf)
     return GST_FLOW_OK;
   } else if (readbytes == 0) {
     GST_DEBUG_OBJECT (psrc, "Read 0 bytes. EOS.");
+    gst_buffer_unref (buf);
     return GST_FLOW_ERROR;
   } else {
     GST_ELEMENT_ERROR (src, RESOURCE, READ, (NULL),
         ("read on file descriptor: %s.", g_strerror (errno)));
     GST_DEBUG_OBJECT (psrc, "Error reading from fd");
+    gst_buffer_unref (buf);
     return GST_FLOW_ERROR;
   }
 }

plugins/elements/gstfdsrc.c

@@ -138,7 +138,8 @@ gst_fdsrc_class_init (GstFdSrcClass * klass)
 static void
 gst_fdsrc_init (GstFdSrc * fdsrc, GstFdSrcClass * klass)
 {
-  // TODO set live only if it's actually a live source
+  /* TODO set live only if it's actually a live source (check
+   * for seekable fd) */
   gst_base_src_set_live (GST_BASE_SRC (fdsrc), TRUE);
 
   fdsrc->fd = 0;
@@ -226,12 +227,15 @@ gst_fdsrc_get_property (GObject * object, guint prop_id, GValue * value,
   }
 }
 
+#define SELECT_TIMEOUT (GST_SECOND / 20)
+
 static GstFlowReturn
 gst_fdsrc_create (GstPushSrc * psrc, GstBuffer ** outbuf)
 {
   GstFdSrc *src;
   GstBuffer *buf;
   glong readbytes;
+  GstClockTime timeout;
 
 #ifndef HAVE_WIN32
   fd_set readfds;
@@ -241,21 +245,32 @@ gst_fdsrc_create (GstPushSrc * psrc, GstBuffer ** outbuf)
 
   src = GST_FDSRC (psrc);
 
-  /* create the buffer */
-  buf = gst_buffer_new_and_alloc (src->blocksize);
-
 #ifndef HAVE_WIN32
   FD_ZERO (&readfds);
   FD_SET (src->fd, &readfds);
 
   if (src->timeout != 0) {
-    GST_TIME_TO_TIMEVAL (src->timeout, t);
+    timeout = MIN (SELECT_TIMEOUT, src->timeout);
-  } else
+  } else {
-    tp = NULL;
+    timeout = SELECT_TIMEOUT;
+  }
 
   do {
+    GST_TIME_TO_TIMEVAL (timeout, t);
+    if (src->timeout != 0)
+      timeout -= MIN (timeout, SELECT_TIMEOUT);
+
     retval = select (src->fd + 1, &readfds, NULL, NULL, tp);
-  } while (retval == -1 && errno == EINTR);     /* retry if interrupted */
+
+    /* Check whether the element got shutdown before full timeout */
+    if (retval == 0) {
+      if (GST_PAD_IS_FLUSHING (GST_BASE_SRC_PAD (src))) {
+        GST_DEBUG_OBJECT (src, "Shutting down with no buffer.");
+        return GST_FLOW_WRONG_STATE;
+      }
+    }
+  } while ((retval == -1 && errno == EINTR) ||  /* retry if interrupted */
+      (retval == 0 && timeout > 0));    /* Retry on incomplete timeout */
 
   if (retval == -1) {
     GST_ELEMENT_ERROR (src, RESOURCE, READ, (NULL),
@@ -269,6 +284,9 @@ gst_fdsrc_create (GstPushSrc * psrc, GstBuffer ** outbuf)
   }
 #endif
 
+  /* create the buffer */
+  buf = gst_buffer_new_and_alloc (src->blocksize);
+
   do {
     readbytes = read (src->fd, GST_BUFFER_DATA (buf), src->blocksize);
   } while (readbytes == -1 && errno == EINTR);  /* retry if interrupted */
@@ -286,11 +304,13 @@ gst_fdsrc_create (GstPushSrc * psrc, GstBuffer ** outbuf)
     return GST_FLOW_OK;
   } else if (readbytes == 0) {
     GST_DEBUG_OBJECT (psrc, "Read 0 bytes. EOS.");
+    gst_buffer_unref (buf);
     return GST_FLOW_ERROR;
   } else {
     GST_ELEMENT_ERROR (src, RESOURCE, READ, (NULL),
         ("read on file descriptor: %s.", g_strerror (errno)));
     GST_DEBUG_OBJECT (psrc, "Error reading from fd");
+    gst_buffer_unref (buf);
     return GST_FLOW_ERROR;
   }
 }

tests/check/generic/states.c

@@ -42,6 +42,8 @@ GST_START_TEST (test_state_changes)
     gst_element_set_state (element, GST_STATE_READY);
     gst_element_set_state (element, GST_STATE_PAUSED);
     gst_element_set_state (element, GST_STATE_PLAYING);
+    /* Sleep to give any pad tasks time to start */
+    g_usleep (0.2 * G_USEC_PER_SEC);
     gst_element_set_state (element, GST_STATE_PAUSED);
     gst_element_set_state (element, GST_STATE_READY);
     gst_element_set_state (element, GST_STATE_NULL);
@@ -63,6 +65,10 @@ states_suite (void)
   Suite *s = suite_create ("states");
   TCase *tc_chain = tcase_create ("general");
 
+  /* Use a long timeout, as we test all elements and take
+   * at least 0.2 seconds each */
+  tcase_set_timeout (tc_chain, 120);
+
   suite_add_tcase (s, tc_chain);
   tcase_add_test (tc_chain, test_state_changes);