From: Havard Graff Date: Tue, 7 Jul 2015 09:53:07 +0000 (+0200) Subject: identity: refactor and add tests using GstHarness X-Git-Tag: 1.6.1~128 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ee63702d619c92185a33d67e66351f8777998c81;p=platform%2Fupstream%2Fgstreamer.git identity: refactor and add tests using GstHarness Writing a test for unscheduling the gst_clock_id_wait inside the identity element, found an invalid read, caused by removing the clock-id when calling _unschedule instead of letting the code calling _wait remove the clock-id after being unscheduled. https://bugzilla.gnome.org/show_bug.cgi?id=752055 --- diff --git a/plugins/elements/gstidentity.c b/plugins/elements/gstidentity.c index 177bfa0..0214596 100644 --- a/plugins/elements/gstidentity.c +++ b/plugins/elements/gstidentity.c @@ -401,8 +401,6 @@ gst_identity_sink_event (GstBaseTransform * trans, GstEvent * event) if (identity->clock_id) { GST_DEBUG_OBJECT (identity, "unlock clock wait"); gst_clock_id_unschedule (identity->clock_id); - gst_clock_id_unref (identity->clock_id); - identity->clock_id = NULL; } GST_OBJECT_UNLOCK (identity); } @@ -886,8 +884,6 @@ gst_identity_change_state (GstElement * element, GstStateChange transition) if (identity->clock_id) { GST_DEBUG_OBJECT (identity, "unlock clock wait"); gst_clock_id_unschedule (identity->clock_id); - gst_clock_id_unref (identity->clock_id); - identity->clock_id = NULL; } identity->blocked = FALSE; g_cond_broadcast (&identity->blocked_cond); diff --git a/tests/check/elements/identity.c b/tests/check/elements/identity.c index 2beb987..fd59b33 100644 --- a/tests/check/elements/identity.c +++ b/tests/check/elements/identity.c @@ -3,6 +3,7 @@ * unit test for identity * * Copyright (C) <2005> Thomas Vander Stichele + * Copyright (C) <2015> Havard Graff * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public @@ -20,100 +21,159 @@ * Boston, MA 02110-1301, USA. */ -#include - #include +#include -static gboolean have_eos = FALSE; +GST_START_TEST (test_one_buffer) +{ + GstHarness *h = gst_harness_new ("identity"); + GstBuffer *buffer_in; + GstBuffer *buffer_out; -/* For ease of programming we use globals to keep refs for our floating - * src and sink pads we create; otherwise we always have to do get_pad, - * get_peer, and then remove references in every test function */ -static GstPad *mysrcpad, *mysinkpad; + gst_harness_set_src_caps_str (h, "mycaps"); + buffer_in = gst_buffer_new_and_alloc (4); + ASSERT_BUFFER_REFCOUNT (buffer_in, "buffer", 1); -static GstStaticPadTemplate sinktemplate = GST_STATIC_PAD_TEMPLATE ("sink", - GST_PAD_SINK, - GST_PAD_ALWAYS, - GST_STATIC_CAPS_ANY); -static GstStaticPadTemplate srctemplate = GST_STATIC_PAD_TEMPLATE ("src", - GST_PAD_SRC, - GST_PAD_ALWAYS, - GST_STATIC_CAPS_ANY); + gst_buffer_fill (buffer_in, 0, "data", 4); -static gboolean -event_func (GstPad * pad, GstObject * parent, GstEvent * event) -{ - if (GST_EVENT_TYPE (event) == GST_EVENT_EOS) { - have_eos = TRUE; - } + /* pushing gives away my reference ... */ + fail_unless_equals_int (GST_FLOW_OK, gst_harness_push (h, buffer_in)); + + /* ... but it should end up being collected on GstHarness queue */ + fail_unless_equals_int (1, gst_harness_buffers_in_queue (h)); + buffer_out = gst_harness_pull (h); - gst_event_unref (event); - return TRUE; + fail_unless (buffer_in == buffer_out); + ASSERT_BUFFER_REFCOUNT (buffer_out, "buffer", 1); + + /* cleanup */ + gst_buffer_unref (buffer_out); + gst_harness_teardown (h); } +GST_END_TEST; -static GstElement * -setup_identity (void) +static void +handoff_func (GstElement * identity, GstBuffer * buf, GstBuffer ** ret) { - GstElement *identity; + (void)identity; + *ret = buf; +} + +GST_START_TEST (test_signal_handoffs) +{ + GstHarness *h = gst_harness_new ("identity"); + GstBuffer *buffer_in; + GstBuffer *buffer_signaled = NULL; + gst_harness_set_src_caps_str (h, "mycaps"); + + /* connect to the handoff signal */ + g_signal_connect (h->element, "handoff", + G_CALLBACK (handoff_func), &buffer_signaled); + + /* first, turn off signal-handoffs */ + g_object_set (h->element, "signal-handoffs", FALSE, NULL); - GST_DEBUG ("setup_identity"); + /* then push a buffer */ + buffer_in = gst_buffer_new_and_alloc (4); + fail_unless_equals_int (GST_FLOW_OK, gst_harness_push (h, buffer_in)); - identity = gst_check_setup_element ("identity"); - mysrcpad = gst_check_setup_src_pad (identity, &srctemplate); - mysinkpad = gst_check_setup_sink_pad (identity, &sinktemplate); - gst_pad_set_event_function (mysinkpad, event_func); - gst_pad_set_active (mysrcpad, TRUE); - gst_pad_set_active (mysinkpad, TRUE); + /* verify that we got no buffer signaled */ + fail_unless (buffer_signaled == NULL); - return identity; + /* now turn on signal-handoffs */ + g_object_set (h->element, "signal-handoffs", TRUE, NULL); + + /* then push another buffer */ + buffer_in = gst_buffer_new_and_alloc (4); + fail_unless_equals_int (GST_FLOW_OK, gst_harness_push (h, buffer_in)); + + /* verify the buffer signaled is equal to the one pushed in */ + fail_unless (buffer_signaled == buffer_in); + ASSERT_BUFFER_REFCOUNT (buffer_signaled, "buffer", 1); + + /* cleanup */ + gst_harness_teardown (h); } +GST_END_TEST; -static void -cleanup_identity (GstElement * identity) +GST_START_TEST (test_sync_on_timestamp) { - GST_DEBUG ("cleanup_identity"); + /* the reason to use the queue in front of the identity element + is to effectively make gst_harness_push asynchronous, not locking + up the test, waiting for gst_clock_id_wait */ + GstHarness * h = gst_harness_new_parse ("queue ! identity sync=1"); + GstBuffer *buf; + GstClock *clock; + GstClockTime timestamp = 123456789; + + /* use testclock */ + gst_harness_use_testclock (h); + gst_harness_set_src_caps_str (h, "mycaps"); + + /* make a buffer and set the timestamp */ + buf = gst_buffer_new (); + GST_BUFFER_PTS (buf) = timestamp; + + /* push the buffer, and verify it does *not* make it through */ + gst_harness_push (h, buf); + fail_unless_equals_int (0, gst_harness_buffers_in_queue (h)); + + /* verify the identity element has registered exactly one GstClockID */ + fail_unless (gst_harness_wait_for_clock_id_waits (h, 1, 42)); + + /* crank the clock and pull the buffer */ + gst_harness_crank_single_clock_wait (h); + buf = gst_harness_pull (h); + + /* verify that the buffer has the right timestamp, and that the time on + the clock is equal to the timestamp */ + fail_unless_equals_int64 (timestamp, GST_BUFFER_PTS (buf)); + clock = gst_element_get_clock (h->element); + fail_unless_equals_int64 (timestamp, gst_clock_get_time (clock)); - gst_pad_set_active (mysrcpad, FALSE); - gst_pad_set_active (mysinkpad, FALSE); - gst_check_teardown_src_pad (identity); - gst_check_teardown_sink_pad (identity); - gst_check_teardown_element (identity); + /* cleanup */ + gst_object_unref (clock); + gst_buffer_unref (buf); + gst_harness_teardown (h); } +GST_END_TEST; -GST_START_TEST (test_one_buffer) +GST_START_TEST (test_stopping_element_unschedules_sync) { - GstElement *identity; - GstBuffer *buffer; - GstSegment segment; + /* the reason to use the queue in front of the identity element + is to effectively make gst_harness_push asynchronous, not locking + up the test, waiting for gst_clock_id_wait */ + GstHarness * h = gst_harness_new_parse ("queue ! identity sync=1"); + GstBuffer *buf; + GstClockTime timestamp = 123456789; - identity = setup_identity (); - fail_unless (gst_element_set_state (identity, - GST_STATE_PLAYING) == GST_STATE_CHANGE_SUCCESS, - "could not set to playing"); + /* use testclock */ + gst_harness_use_testclock (h); + gst_harness_set_src_caps_str (h, "mycaps"); - gst_segment_init (&segment, GST_FORMAT_BYTES); - gst_pad_push_event (mysrcpad, gst_event_new_stream_start ("test")); - gst_pad_push_event (mysrcpad, gst_event_new_segment (&segment)); + /* make a buffer and set the timestamp */ + buf = gst_buffer_new (); + GST_BUFFER_PTS (buf) = timestamp; - buffer = gst_buffer_new_and_alloc (4); - ASSERT_BUFFER_REFCOUNT (buffer, "buffer", 1); + /* push the buffer, and verify it does *not* make it through */ + gst_harness_push (h, buf); + fail_unless_equals_int (0, gst_harness_buffers_in_queue (h)); - gst_buffer_fill (buffer, 0, "data", 4); + /* verify the identity element has registered exactly one GstClockID */ + fail_unless (gst_harness_wait_for_clock_id_waits (h, 1, 42)); - /* pushing gives away my reference ... */ - fail_unless (gst_pad_push (mysrcpad, buffer) == GST_FLOW_OK, - "Failed pushing buffer to identity"); + /* setting identity to READY should unschedule the sync */ + gst_element_set_state (h->element, GST_STATE_READY); - /* ... but it should end up being collected on the global buffer list */ - fail_unless (g_list_length (buffers) == 1); - fail_unless ((GstBuffer *) (g_list_first (buffers)->data) == buffer); - ASSERT_BUFFER_REFCOUNT (buffer, "buffer", 1); + /* verify the identity element no longer waits on the clock */ + fail_unless (gst_harness_wait_for_clock_id_waits (h, 0, 42)); - /* cleanup */ - cleanup_identity (identity); -} + /* and that the waiting buffer was dropped */ + fail_unless_equals_int (0, gst_harness_buffers_received (h)); + gst_harness_teardown (h); +} GST_END_TEST; static Suite * @@ -124,6 +184,10 @@ identity_suite (void) suite_add_tcase (s, tc_chain); tcase_add_test (tc_chain, test_one_buffer); + tcase_add_test (tc_chain, test_signal_handoffs); + tcase_add_test (tc_chain, test_sync_on_timestamp); + tcase_add_test (tc_chain, test_stopping_element_unschedules_sync); + return s; }