gst/videorate/gstvideorate.c: Don't leak incoming buffer if gst_pad_push() returns...

Original commit message from CVS:
Patch by: Dan Williams <dcbw redhat com>
* gst/videorate/gstvideorate.c: (gst_video_rate_chain):
Don't leak incoming buffer if gst_pad_push() returns a
non-OK flow. Fixes #432755.
* tests/check/elements/videorate.c: (GST_START_TEST),
(videorate_suite):
Unit test for the above by Yours Truly.

diff --git a/ChangeLog b/ChangeLog
index 9823b36..cc12532 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2007-04-24  Tim-Philipp Müller  <tim at centricular dot net>
+
+       Patch by: Dan Williams <dcbw redhat com>
+
+       * gst/videorate/gstvideorate.c: (gst_video_rate_chain):
+         Don't leak incoming buffer if gst_pad_push() returns a
+         non-OK flow. Fixes #432755.
+        
+       * tests/check/elements/videorate.c: (GST_START_TEST),
+       (videorate_suite):
+         Unit test for the above by Yours Truly.
+
 2007-04-23  Stefan Kost  <ensonic@users.sf.net>
 
        * gst/adder/gstadder.c: (gst_adder_setcaps), (gst_adder_src_event),

diff --git a/gst/videorate/gstvideorate.c b/gst/videorate/gstvideorate.c
index 17e5b33..d71f051 100644 (file)
--- a/gst/videorate/gstvideorate.c
+++ b/gst/videorate/gstvideorate.c
@@ -560,7 +560,7 @@ gst_video_rate_chain (GstPad * pad, GstBuffer * buffer)
   GstFlowReturn res = GST_FLOW_OK;
   GstClockTime intime, in_ts;
 
-  videorate = GST_VIDEO_RATE (gst_pad_get_parent (pad));
+  videorate = GST_VIDEO_RATE (GST_PAD_PARENT (pad));
 
   /* make sure the denominators are not 0 */
   if (videorate->from_rate_denominator == 0 ||
@@ -636,8 +636,10 @@ gst_video_rate_chain (GstPad * pad, GstBuffer * buffer)
         count++;
 
         /* on error the _flush function posted a warning already */
-        if ((res = gst_video_rate_flush_prev (videorate)) != GST_FLOW_OK)
+        if ((res = gst_video_rate_flush_prev (videorate)) != GST_FLOW_OK) {
+          gst_buffer_unref (buffer);
           goto done;
+        }
       }
       /* continue while the first one was the best */
     }
@@ -671,7 +673,6 @@ gst_video_rate_chain (GstPad * pad, GstBuffer * buffer)
     gst_video_rate_swap_prev (videorate, buffer, intime);
   }
 done:
-  gst_object_unref (videorate);
   return res;
 
   /* ERRORS */

diff --git a/tests/check/elements/videorate.c b/tests/check/elements/videorate.c
index aedc619..974244f 100644 (file)
--- a/tests/check/elements/videorate.c
+++ b/tests/check/elements/videorate.c
@@ -609,7 +609,56 @@ GST_START_TEST (test_changing_size)
 }
 
 GST_END_TEST;
-Suite *
+
+GST_START_TEST (test_non_ok_flow)
+{
+  GstElement *videorate;
+  GstClockTime ts;
+  GstBuffer *buf;
+  GstCaps *caps;
+
+  videorate = setup_videorate ();
+  fail_unless (gst_element_set_state (videorate,
+          GST_STATE_PLAYING) == GST_STATE_CHANGE_SUCCESS,
+      "could not set to playing");
+
+  buf = gst_buffer_new_and_alloc (4);
+  memset (GST_BUFFER_DATA (buf), 0, 4);
+  caps = gst_caps_from_string (VIDEO_CAPS_STRING);
+  gst_buffer_set_caps (buf, caps);
+  gst_caps_unref (caps);
+  ASSERT_BUFFER_REFCOUNT (buf, "inbuffer", 1);
+
+  /* push a few 'normal' buffers */
+  for (ts = 0; ts < 100 * GST_SECOND; ts += GST_SECOND / 33) {
+    GstBuffer *inbuf;
+
+    inbuf = gst_buffer_copy (buf);
+    GST_BUFFER_TIMESTAMP (inbuf) = ts;
+
+    fail_unless_equals_int (gst_pad_push (mysrcpad, inbuf), GST_FLOW_OK);
+  }
+
+  /* we should have buffers according to the output framerate of 25/1 */
+  fail_unless_equals_int (g_list_length (buffers), 100 * 25);
+
+  /* now deactivate pad so we get a WRONG_STATE flow return */
+  gst_pad_set_active (mysinkpad, FALSE);
+
+  /* push buffer on deactivated pad */
+  fail_unless (gst_buffer_is_metadata_writable (buf));
+  GST_BUFFER_TIMESTAMP (buf) = ts;
+
+  /* pushing gives away our reference */
+  fail_unless_equals_int (gst_pad_push (mysrcpad, buf), GST_FLOW_WRONG_STATE);
+
+  /* cleanup */
+  cleanup_videorate (videorate);
+}
+
+GST_END_TEST;
+
+static Suite *
 videorate_suite (void)
 {
   Suite *s = suite_create ("videorate");
@@ -622,23 +671,9 @@ videorate_suite (void)
   tcase_add_test (tc_chain, test_wrong_order);
   tcase_add_test (tc_chain, test_no_framerate);
   tcase_add_test (tc_chain, test_changing_size);
+  tcase_add_test (tc_chain, test_non_ok_flow);
 
   return s;
 }
 
-int
-main (int argc, char **argv)
-{
-  int nf;
-
-  Suite *s = videorate_suite ();
-  SRunner *sr = srunner_create (s);
-
-  gst_check_init (&argc, &argv);
-
-  srunner_run_all (sr, CK_NORMAL);
-  nf = srunner_ntests_failed (sr);
-  srunner_free (sr);
-
-  return nf;
-}
+GST_CHECK_MAIN (videorate)