gst/videorate/gstvideorate.c: fix up docs fix a leak when no caps negotiated fix counting of input frames

Original commit message from CVS:

* gst/videorate/gstvideorate.c: (gst_video_rate_reset),
(gst_video_rate_swap_prev), (gst_video_rate_chain):
fix up docs
fix a leak when no caps negotiated
fix counting of input frames
* tests/check/elements/.cvsignore:
* tests/check/elements/videorate.c: (assert_videorate_stats),
(GST_START_TEST), (videorate_suite):
add tests for these

ChangeLog

@@ -1,3 +1,15 @@
+2006-04-28  Thomas Vander Stichele  <thomas at apestaart dot org>
+
+	* gst/videorate/gstvideorate.c: (gst_video_rate_reset),
+	(gst_video_rate_swap_prev), (gst_video_rate_chain):
+	  fix up docs
+	  fix a leak when no caps negotiated
+	  fix counting of input frames
+	* tests/check/elements/.cvsignore:
+	* tests/check/elements/videorate.c: (assert_videorate_stats),
+	(GST_START_TEST), (videorate_suite):
+	  add tests for these
+
 2006-04-28  Wim Taymans  <wim@fluendo.com>
 
 	* gst-libs/gst/audio/gstringbuffer.c:

docs/libs/tmpl/gsttuner.sgml

@@ -96,8 +96,6 @@ interface for elements that provide tuner operations
 
 @tuner: 
 @norm: 
-<!-- # Unused Parameters # -->
-@channel: 
 
 
 <!-- ##### FUNCTION gst_tuner_set_frequency ##### -->

gst/videorate/gstvideorate.c

@@ -28,29 +28,35 @@
  * used to interpolate frames (yet).
  * </para>
  * <para>
- * By default the element will simply negotiate the same framerate on its source and
+ * By default the element will simply negotiate the same framerate on its
- * sink pad and will adjust timestamps/insert/drop frames in case the input stream
+ * source and sink pad and will adjust timestamps/insert/drop frames in case
- * is not respecting that framerate.
+ * the input stream is not respecting that framerate.
  * </para>
  * <para>
- * A conversion to another framerate can be forced by using filtered caps on the source
+ * A conversion to another framerate can be forced by using filtered caps on
- * pad.
+ * the source pad.
  * </para>
  * <para>
  * The properties "in", "out", "duplicate" and "drop" can be read to obtain
- * information about respectively received frame, outputed frame, duplicated frames
+ * information about number of input frames, output frames, dropped frames
- * and dropped frames.
+ * (i.e. the number of unused input frames) and duplicated frames (i.e. the
- * When the "silent" property is set to FALSE, a GObject property notification will
+ *  number of times an input frame was duplicated, beside being used normally).
- * be emited whenever one of the "duplicate" or "drop" values changed. This can 
+ *
- * potentially cause performance degradation. Also note that property notification 
+ * An input stream that needs no adjustments will thus never have dropped or
- * will happen in the streaming thread so applications should be prepared for this.
+ * duplicated frames.
+ *
+ * When the "silent" property is set to FALSE, a GObject property notification
+ * will be emitted whenever one of the "duplicate" or "drop" values changes.
+ * This can potentially cause performance degradation.
+ * Note that property notification will happen from the streaming thread, so
+ * applications should be prepared for this.
  * </para>
  * <title>Example pipelines</title>
  * <para>
  * <programlisting>
  * gst-launch -v filesrc location=videotestsrc.ogg ! oggdemux ! theoradec ! videorate ! video/x-raw-yuv,framerate=15/1 ! xvimagesink
  * </programlisting>
- * Decode an Ogg/Theora and adjust the framerate to 15 fps. 
+ * Decode an Ogg/Theora file and adjust the framerate to 15 fps before playing.
  * To create the test Ogg/Theora file refer to the documentation of theoraenc.
  * </para>
  * </refsect2>
@@ -358,7 +364,7 @@ no_transform:
 static void
 gst_video_rate_reset (GstVideoRate * videorate)
 {
-  GST_DEBUG ("resetting data");
+  GST_DEBUG ("resetting internal variables");
 
   videorate->in = 0;
   videorate->out = 0;
@@ -450,6 +456,7 @@ static void
 gst_video_rate_swap_prev (GstVideoRate * videorate, GstBuffer * buffer,
     gint64 time)
 {
+  GST_LOG_OBJECT (videorate, "swap_prev: storing buffer %p in prev", buffer);
   if (videorate->prevbuf)
     gst_buffer_unref (videorate->prevbuf);
   videorate->prevbuf = buffer;
@@ -530,7 +537,7 @@ gst_video_rate_chain (GstPad * pad, GstBuffer * buffer)
 
   videorate = GST_VIDEO_RATE (gst_pad_get_parent (pad));
 
-  /* make sure the denominators have to be != 0 */
+  /* make sure the denominators are not 0 */
   if (videorate->from_rate_denominator == 0 ||
       videorate->to_rate_denominator == 0)
     goto not_negotiated;
@@ -543,9 +550,10 @@ gst_video_rate_chain (GstPad * pad, GstBuffer * buffer)
   intime = gst_segment_to_running_time (&videorate->segment,
       GST_FORMAT_TIME, in_ts);
 
-  /* pull in 2 buffers */
+  /* we need to have two buffers to compare */
   if (videorate->prevbuf == NULL) {
     gst_video_rate_swap_prev (videorate, buffer, intime);
+    videorate->in++;
     videorate->next_ts = 0;
   } else {
     GstClockTime prevtime;
@@ -639,6 +647,7 @@ done:
 not_negotiated:
   {
     GST_WARNING_OBJECT (videorate, "no framerate negotiated");
+    gst_buffer_unref (buffer);
     res = GST_FLOW_NOT_NEGOTIATED;
     goto done;
   }

tests/check/elements/.gitignore

@@ -2,6 +2,7 @@
 audioconvert
 audioresample
 audiotestsrc
+videorate
 videotestsrc
 volume
 vorbisdec

tests/check/elements/videorate.c

@@ -146,6 +146,114 @@ GST_START_TEST (test_one)
 
 GST_END_TEST;
 
+GST_START_TEST (test_more)
+{
+  GstElement *videorate;
+  GstBuffer *first, *second, *third, *outbuffer;
+  GList *l;
+  GstCaps *caps;
+
+  videorate = setup_videorate ();
+  fail_unless (gst_element_set_state (videorate,
+          GST_STATE_PLAYING) == GST_STATE_CHANGE_SUCCESS,
+      "could not set to playing");
+  assert_videorate_stats (videorate, "creation", 0, 0, 0, 0);
+
+  /* first buffer */
+  first = gst_buffer_new_and_alloc (4);
+  GST_BUFFER_TIMESTAMP (first) = 0;
+  memset (GST_BUFFER_DATA (first), 1, 4);
+  caps = gst_caps_from_string (VIDEO_CAPS_STRING);
+  gst_buffer_set_caps (first, caps);
+  gst_caps_unref (caps);
+  ASSERT_BUFFER_REFCOUNT (first, "first", 1);
+  gst_buffer_ref (first);
+
+  /* pushing gives away my reference ... */
+  fail_unless (gst_pad_push (mysrcpad, first) == GST_FLOW_OK);
+  /* ... and it is now stuck inside videorate */
+  ASSERT_BUFFER_REFCOUNT (first, "first", 2);
+  fail_unless_equals_int (g_list_length (buffers), 0);
+  assert_videorate_stats (videorate, "first buffer", 1, 0, 0, 0);
+
+  /* second buffer; inbetween second and third output frame's timestamp */
+  second = gst_buffer_new_and_alloc (4);
+  GST_BUFFER_TIMESTAMP (second) = GST_SECOND * 3 / 50;
+  memset (GST_BUFFER_DATA (second), 2, 4);
+  caps = gst_caps_from_string (VIDEO_CAPS_STRING);
+  gst_buffer_set_caps (second, caps);
+  gst_caps_unref (caps);
+  ASSERT_BUFFER_REFCOUNT (second, "second", 1);
+  gst_buffer_ref (second);
+
+  /* pushing gives away one of my references ... */
+  fail_unless (gst_pad_push (mysrcpad, second) == GST_FLOW_OK);
+  /* ... and it is now stuck inside videorate */
+  ASSERT_BUFFER_REFCOUNT (second, "second", 2);
+
+  /* ... and the first one is pushed out, with timestamp 0 */
+  fail_unless_equals_int (g_list_length (buffers), 1);
+  assert_videorate_stats (videorate, "second buffer", 2, 1, 0, 0);
+  ASSERT_BUFFER_REFCOUNT (first, "first", 2);
+
+  outbuffer = buffers->data;
+  fail_unless_equals_uint64 (GST_BUFFER_TIMESTAMP (outbuffer), 0);
+
+  /* third buffer */
+  third = gst_buffer_new_and_alloc (4);
+  GST_BUFFER_TIMESTAMP (third) = GST_SECOND * 12 / 50;
+  memset (GST_BUFFER_DATA (third), 3, 4);
+  caps = gst_caps_from_string (VIDEO_CAPS_STRING);
+  gst_buffer_set_caps (third, caps);
+  gst_caps_unref (caps);
+  ASSERT_BUFFER_REFCOUNT (third, "third", 1);
+  gst_buffer_ref (third);
+
+  /* pushing gives away my reference ... */
+  fail_unless (gst_pad_push (mysrcpad, third) == GST_FLOW_OK);
+  /* ... and it is now stuck inside videorate */
+  ASSERT_BUFFER_REFCOUNT (third, "third", 2);
+
+  /* submitting the third buffer has triggered flushing of three more frames */
+  assert_videorate_stats (videorate, "third buffer", 3, 4, 0, 2);
+
+  /* check timestamp and source correctness */
+  l = buffers;
+  fail_unless_equals_uint64 (GST_BUFFER_TIMESTAMP (l->data), 0);
+  fail_unless_equals_int (GST_BUFFER_DATA (l->data)[0], 1);
+  l = g_list_next (l);
+  fail_unless_equals_uint64 (GST_BUFFER_TIMESTAMP (l->data), GST_SECOND / 25);
+  fail_unless_equals_int (GST_BUFFER_DATA (l->data)[0], 2);
+  l = g_list_next (l);
+  fail_unless_equals_uint64 (GST_BUFFER_TIMESTAMP (l->data),
+      GST_SECOND * 2 / 25);
+  fail_unless_equals_int (GST_BUFFER_DATA (l->data)[0], 2);
+  l = g_list_next (l);
+  fail_unless_equals_uint64 (GST_BUFFER_TIMESTAMP (l->data),
+      GST_SECOND * 3 / 25);
+  fail_unless_equals_int (GST_BUFFER_DATA (l->data)[0], 2);
+
+  fail_unless_equals_int (g_list_length (buffers), 4);
+  /* one held by us, three held by each output frame taken from the second */
+  ASSERT_BUFFER_REFCOUNT (second, "second", 4);
+
+  /* now send EOS */
+  fail_unless (gst_pad_push_event (mysrcpad, gst_event_new_eos ()));
+
+  /* submitting eos should flush out two more frames for tick 8 and 10 */
+  /* FIXME: right now it only flushes out one, so out is 5 instead of 6 ! */
+  assert_videorate_stats (videorate, "eos", 3, 5, 0, 2);
+  fail_unless_equals_int (g_list_length (buffers), 5);
+
+  /* cleanup */
+  gst_buffer_unref (first);
+  gst_buffer_unref (second);
+  gst_buffer_unref (third);
+  cleanup_videorate (videorate);
+}
+
+GST_END_TEST;
+
 /* frames at 1, 0, 2 -> second one should be ignored */
 GST_START_TEST (test_wrong_order_from_zero)
 {
@@ -176,8 +284,7 @@ GST_START_TEST (test_wrong_order_from_zero)
   /* ... and it is now stuck inside videorate */
   ASSERT_BUFFER_REFCOUNT (first, "first", 2);
   fail_unless_equals_int (g_list_length (buffers), 0);
-  /* FIXME: in is not counted properly, should be 1 */
+  assert_videorate_stats (videorate, "first", 1, 0, 0, 0);
-  assert_videorate_stats (videorate, "first", 0, 0, 0, 0);
 
   /* second buffer */
   second = gst_buffer_new_and_alloc (4);
@@ -191,13 +298,12 @@ GST_START_TEST (test_wrong_order_from_zero)
 
   /* pushing gives away my reference ... */
   fail_unless (gst_pad_push (mysrcpad, second) == GST_FLOW_OK);
-  /* ... and it got dropped because it was before the first */
+  /* ... and it is now dropped because it is too old */
   ASSERT_BUFFER_REFCOUNT (second, "second", 1);
   fail_unless_equals_int (g_list_length (buffers), 0);
 
-  /* ... and the first one is now dropped */
+  /* ... and the first one is still there */
-  /* FIXME: still a bug with in, needs to be 2 */
+  assert_videorate_stats (videorate, "second", 2, 0, 1, 0);
-  assert_videorate_stats (videorate, "second", 1, 0, 1, 0);
   ASSERT_BUFFER_REFCOUNT (first, "first", 2);
 
   /* third buffer */
@@ -221,8 +327,7 @@ GST_START_TEST (test_wrong_order_from_zero)
   ASSERT_BUFFER_REFCOUNT (first, "first", 39);
   ASSERT_BUFFER_REFCOUNT (second, "second", 1);
   ASSERT_BUFFER_REFCOUNT (third, "third", 2);
-  /* FIXME: you guessed it ... */
+  assert_videorate_stats (videorate, "third", 3, 38, 1, 37);
-  assert_videorate_stats (videorate, "third", 2, 38, 1, 37);
 
   /* verify last buffer */
   outbuffer = g_list_last (buffers)->data;
@@ -269,8 +374,7 @@ GST_START_TEST (test_wrong_order)
   /* ... and it is now stuck inside videorate */
   ASSERT_BUFFER_REFCOUNT (first, "first", 2);
   fail_unless_equals_int (g_list_length (buffers), 0);
-  /* FIXME: in is not counted properly, should be 1 */
+  assert_videorate_stats (videorate, "first", 1, 0, 0, 0);
-  assert_videorate_stats (videorate, "first", 0, 0, 0, 0);
 
   /* second buffer */
   second = gst_buffer_new_and_alloc (4);
@@ -288,8 +392,7 @@ GST_START_TEST (test_wrong_order)
   ASSERT_BUFFER_REFCOUNT (second, "second", 2);
   /* and it created 13 output buffers as copies of the first frame */
   fail_unless_equals_int (g_list_length (buffers), 13);
-  /* FIXME: guess */
+  assert_videorate_stats (videorate, "second", 2, 13, 0, 12);
-  assert_videorate_stats (videorate, "second", 1, 13, 0, 12);
   ASSERT_BUFFER_REFCOUNT (first, "first", 14);
 
   /* third buffer */
@@ -312,8 +415,7 @@ GST_START_TEST (test_wrong_order)
   ASSERT_BUFFER_REFCOUNT (first, "first", 14);
   ASSERT_BUFFER_REFCOUNT (second, "second", 26);
   /* three frames submitted; two of them output as is, and 36 duplicated */
-  /* FIXME: guess */
+  assert_videorate_stats (videorate, "third", 3, 38, 0, 36);
-  assert_videorate_stats (videorate, "third", 2, 38, 0, 36);
 
   /* fourth buffer */
   fourth = gst_buffer_new_and_alloc (4);
@@ -327,14 +429,13 @@ GST_START_TEST (test_wrong_order)
 
   /* pushing gives away my reference ... */
   fail_unless (gst_pad_push (mysrcpad, fourth) == GST_FLOW_OK);
-  /* ... and it is dropped from videorate because 0 < 2 */
+  /* ... and it is dropped */
   ASSERT_BUFFER_REFCOUNT (fourth, "fourth", 1);
 
   fail_unless_equals_int (g_list_length (buffers), 38);
-  ASSERT_BUFFER_REFCOUNT (first, "first", 14);  /* 13 frames pushed out */
+  ASSERT_BUFFER_REFCOUNT (first, "first", 14);
   ASSERT_BUFFER_REFCOUNT (second, "second", 26);
-  /* FIXME: guess */
+  assert_videorate_stats (videorate, "fourth", 4, 38, 1, 36);
-  assert_videorate_stats (videorate, "fourth", 3, 38, 1, 36);
 
   /* verify last buffer */
   outbuffer = g_list_last (buffers)->data;
@@ -342,6 +443,7 @@ GST_START_TEST (test_wrong_order)
   fail_unless_equals_uint64 (GST_BUFFER_TIMESTAMP (outbuffer),
       GST_SECOND * 37 / 25);
 
+
   /* cleanup */
   gst_buffer_unref (first);
   gst_buffer_unref (second);
@@ -352,6 +454,42 @@ GST_START_TEST (test_wrong_order)
 
 GST_END_TEST;
 
+
+/* if no framerate is negotiated, we should not be able to push a buffer */
+GST_START_TEST (test_no_framerate)
+{
+  GstElement *videorate;
+  GstBuffer *inbuffer, *outbuffer;
+  GstCaps *caps;
+
+  videorate = setup_videorate ();
+  fail_unless (gst_element_set_state (videorate,
+          GST_STATE_PLAYING) == GST_STATE_CHANGE_SUCCESS,
+      "could not set to playing");
+
+  inbuffer = gst_buffer_new_and_alloc (4);
+  memset (GST_BUFFER_DATA (inbuffer), 0, 4);
+  caps = gst_caps_from_string (VIDEO_CAPS_NO_FRAMERATE_STRING);
+  gst_buffer_set_caps (inbuffer, caps);
+  gst_caps_unref (caps);
+  ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
+
+  /* take a ref so we can later check refcount */
+  gst_buffer_ref (inbuffer);
+
+  /* no framerate is negotiated so pushing should fail */
+  fail_if (gst_pad_push (mysrcpad, inbuffer) == GST_FLOW_OK);
+  ASSERT_BUFFER_REFCOUNT (inbuffer, "inbuffer", 1);
+  gst_buffer_unref (inbuffer);
+  fail_unless_equals_int (g_list_length (buffers), 0);
+
+  /* cleanup */
+  cleanup_videorate (videorate);
+}
+
+GST_END_TEST;
+
+
 Suite *
 videorate_suite (void)
 {
@@ -359,8 +497,11 @@ videorate_suite (void)
   TCase *tc_chain = tcase_create ("general");
 
   suite_add_tcase (s, tc_chain);
+  tcase_add_test (tc_chain, test_one);
+  tcase_add_test (tc_chain, test_more);
   tcase_add_test (tc_chain, test_wrong_order_from_zero);
   tcase_add_test (tc_chain, test_wrong_order);
+  tcase_add_test (tc_chain, test_no_framerate);
 
   return s;
 }