From 7bf18ad258bfd81200197378dbedde125f813fad Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Mon, 8 Oct 2018 21:56:31 +1100 Subject: webrtc: start in the closed state This means that we will reject all operations before we've transitioned into READY. This also fixes the tests using the default GMainContext in the NULL state instead of the webrtcbin internal GMainContext and thread. Also removes a potential ordering race where on the element transitioning to READY, an operations could have been queued on two different threads and removing a guarentee on operation ordering. --- ext/webrtc/gstwebrtcbin.c | 3 + tests/check/elements/webrtcbin.c | 125 +++++++++++++++++++++++++++++++++++---- 2 files changed, 116 insertions(+), 12 deletions(-) diff --git a/ext/webrtc/gstwebrtcbin.c b/ext/webrtc/gstwebrtcbin.c index d7c1d479b..0bb1eb127 100644 --- a/ext/webrtc/gstwebrtcbin.c +++ b/ext/webrtc/gstwebrtcbin.c @@ -5146,4 +5146,7 @@ gst_webrtc_bin_init (GstWebRTCBin * webrtc) g_array_new (FALSE, TRUE, sizeof (IceCandidateItem *)); g_array_set_clear_func (webrtc->priv->pending_ice_candidates, (GDestroyNotify) _clear_ice_candidate_item); + + /* we start off closed until we move to READY */ + webrtc->priv->is_closed = TRUE; } diff --git a/tests/check/elements/webrtcbin.c b/tests/check/elements/webrtcbin.c index e6316687a..2f3420001 100644 --- a/tests/check/elements/webrtcbin.c +++ b/tests/check/elements/webrtcbin.c @@ -643,11 +643,17 @@ GST_START_TEST (test_sdp_no_media) /* check that a no stream connection creates 0 media sections */ + t->on_negotiation_needed = NULL; t->offer_data = GUINT_TO_POINTER (0); t->on_offer_created = _count_num_sdp_media; t->answer_data = GUINT_TO_POINTER (0); t->on_answer_created = _count_num_sdp_media; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + test_webrtc_create_offer (t, t->webrtc1); test_webrtc_wait_for_answer_error_eos (t); @@ -700,12 +706,18 @@ GST_START_TEST (test_audio) /* check that a single stream connection creates the associated number * of media sections */ + t->on_negotiation_needed = NULL; t->offer_data = GUINT_TO_POINTER (1); t->on_offer_created = _count_num_sdp_media; t->answer_data = GUINT_TO_POINTER (1); t->on_answer_created = _count_num_sdp_media; t->on_ice_candidate = NULL; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + test_webrtc_create_offer (t, t->webrtc1); test_webrtc_wait_for_answer_error_eos (t); @@ -742,12 +754,18 @@ GST_START_TEST (test_audio_video) /* check that a dual stream connection creates the associated number * of media sections */ + t->on_negotiation_needed = NULL; t->offer_data = GUINT_TO_POINTER (2); t->on_offer_created = _count_num_sdp_media; t->answer_data = GUINT_TO_POINTER (2); t->on_answer_created = _count_num_sdp_media; t->on_ice_candidate = NULL; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + test_webrtc_create_offer (t, t->webrtc1); test_webrtc_wait_for_answer_error_eos (t); @@ -842,12 +860,18 @@ GST_START_TEST (test_media_direction) add_fake_audio_src_harness (h, 96); t->harnesses = g_list_prepend (t->harnesses, h); + t->on_negotiation_needed = NULL; t->offer_data = &offer; t->on_offer_created = validate_sdp; t->answer_data = &answer; t->on_answer_created = validate_sdp; t->on_ice_candidate = NULL; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + test_webrtc_create_offer (t, t->webrtc1); test_webrtc_wait_for_answer_error_eos (t); @@ -896,6 +920,7 @@ GST_START_TEST (test_payload_types) GstWebRTCRTPTransceiver *trans; GArray *transceivers; + t->on_negotiation_needed = NULL; t->offer_data = &offer; t->on_offer_created = validate_sdp; t->on_ice_candidate = NULL; @@ -909,6 +934,11 @@ GST_START_TEST (test_payload_types) NULL); g_array_unref (transceivers); + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + test_webrtc_create_offer (t, t->webrtc1); test_webrtc_wait_for_answer_error_eos (t); @@ -954,12 +984,18 @@ GST_START_TEST (test_media_setup) /* check the default dtls setup negotiation values */ + t->on_negotiation_needed = NULL; t->offer_data = &offer; t->on_offer_created = validate_sdp; t->answer_data = &answer; t->on_answer_created = validate_sdp; t->on_ice_candidate = NULL; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + test_webrtc_create_offer (t, t->webrtc1); test_webrtc_wait_for_answer_error_eos (t); @@ -1276,9 +1312,15 @@ GST_START_TEST (test_session_stats) /* test that the stats generated without any streams are sane */ + t->on_negotiation_needed = NULL; t->on_offer_created = NULL; t->on_answer_created = NULL; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + test_webrtc_create_offer (t, t->webrtc1); test_webrtc_wait_for_answer_error_eos (t); @@ -1350,6 +1392,7 @@ GST_START_TEST (test_add_recvonly_transceiver) /* add a transceiver that will only receive an opus stream and check that * the created offer is marked as recvonly */ + t->on_negotiation_needed = NULL; t->on_pad_added = _pad_added_fakesink; t->on_negotiation_needed = NULL; t->offer_data = &offer; @@ -1372,6 +1415,11 @@ GST_START_TEST (test_add_recvonly_transceiver) add_fake_audio_src_harness (h, 96); t->harnesses = g_list_prepend (t->harnesses, h); + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + test_webrtc_create_offer (t, t->webrtc1); test_webrtc_wait_for_answer_error_eos (t); @@ -1397,6 +1445,7 @@ GST_START_TEST (test_recvonly_sendonly) /* add a transceiver that will only receive an opus stream and check that * the created offer is marked as recvonly */ + t->on_negotiation_needed = NULL; t->on_pad_added = _pad_added_fakesink; t->on_negotiation_needed = NULL; t->offer_data = &offer; @@ -1430,6 +1479,11 @@ GST_START_TEST (test_recvonly_sendonly) add_fake_audio_src_harness (h, 96); t->harnesses = g_list_prepend (t->harnesses, h); + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + test_webrtc_create_offer (t, t->webrtc1); test_webrtc_wait_for_answer_error_eos (t); @@ -1505,6 +1559,11 @@ GST_START_TEST (test_data_channel_create) t->on_answer_created = validate_sdp; t->on_ice_candidate = NULL; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + g_signal_emit_by_name (t->webrtc1, "create-data-channel", "label", NULL, &channel); g_assert_nonnull (channel); @@ -1560,6 +1619,11 @@ GST_START_TEST (test_data_channel_remote_notify) t->on_ice_candidate = NULL; t->on_data_channel = have_data_channel; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + g_signal_emit_by_name (t->webrtc1, "create-data-channel", "label", NULL, &channel); g_assert_nonnull (channel); @@ -1567,8 +1631,10 @@ GST_START_TEST (test_data_channel_remote_notify) g_signal_connect (channel, "on-error", G_CALLBACK (on_channel_error_not_reached), NULL); - gst_element_set_state (t->webrtc1, GST_STATE_PLAYING); - gst_element_set_state (t->webrtc2, GST_STATE_PLAYING); + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); test_webrtc_create_offer (t, t->webrtc1); @@ -1628,6 +1694,11 @@ GST_START_TEST (test_data_channel_transfer_string) t->on_ice_candidate = NULL; t->on_data_channel = have_data_channel_transfer_string; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + g_signal_emit_by_name (t->webrtc1, "create-data-channel", "label", NULL, &channel); g_assert_nonnull (channel); @@ -1635,8 +1706,10 @@ GST_START_TEST (test_data_channel_transfer_string) g_signal_connect (channel, "on-error", G_CALLBACK (on_channel_error_not_reached), NULL); - gst_element_set_state (t->webrtc1, GST_STATE_PLAYING); - gst_element_set_state (t->webrtc2, GST_STATE_PLAYING); + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); test_webrtc_create_offer (t, t->webrtc1); @@ -1703,6 +1776,11 @@ GST_START_TEST (test_data_channel_transfer_data) t->on_ice_candidate = NULL; t->on_data_channel = have_data_channel_transfer_data; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + g_signal_emit_by_name (t->webrtc1, "create-data-channel", "label", NULL, &channel); g_assert_nonnull (channel); @@ -1710,8 +1788,10 @@ GST_START_TEST (test_data_channel_transfer_data) g_signal_connect (channel, "on-error", G_CALLBACK (on_channel_error_not_reached), NULL); - gst_element_set_state (t->webrtc1, GST_STATE_PLAYING); - gst_element_set_state (t->webrtc2, GST_STATE_PLAYING); + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); test_webrtc_create_offer (t, t->webrtc1); @@ -1754,6 +1834,11 @@ GST_START_TEST (test_data_channel_create_after_negotiate) t->on_ice_candidate = NULL; t->on_data_channel = have_data_channel_create_data_channel; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + g_signal_emit_by_name (t->webrtc1, "create-data-channel", "prev-label", NULL, &channel); g_assert_nonnull (channel); @@ -1761,8 +1846,10 @@ GST_START_TEST (test_data_channel_create_after_negotiate) g_signal_connect (channel, "on-error", G_CALLBACK (on_channel_error_not_reached), NULL); - gst_element_set_state (t->webrtc1, GST_STATE_PLAYING); - gst_element_set_state (t->webrtc2, GST_STATE_PLAYING); + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); test_webrtc_create_offer (t, t->webrtc1); @@ -1808,6 +1895,11 @@ GST_START_TEST (test_data_channel_low_threshold) t->on_ice_candidate = NULL; t->on_data_channel = have_data_channel_check_low_threshold_emitted; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + g_signal_emit_by_name (t->webrtc1, "create-data-channel", "label", NULL, &channel); g_assert_nonnull (channel); @@ -1815,8 +1907,10 @@ GST_START_TEST (test_data_channel_low_threshold) g_signal_connect (channel, "on-error", G_CALLBACK (on_channel_error_not_reached), NULL); - gst_element_set_state (t->webrtc1, GST_STATE_PLAYING); - gst_element_set_state (t->webrtc2, GST_STATE_PLAYING); + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); test_webrtc_create_offer (t, t->webrtc1); @@ -1874,13 +1968,20 @@ GST_START_TEST (test_data_channel_max_message_size) t->on_ice_candidate = NULL; t->on_data_channel = have_data_channel_transfer_large_data; + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_READY) == GST_STATE_CHANGE_FAILURE); + g_signal_emit_by_name (t->webrtc1, "create-data-channel", "label", NULL, &channel); g_assert_nonnull (channel); t->data_channel_data = channel; - gst_element_set_state (t->webrtc1, GST_STATE_PLAYING); - gst_element_set_state (t->webrtc2, GST_STATE_PLAYING); + fail_if (gst_element_set_state (t->webrtc1, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, + GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE); test_webrtc_create_offer (t, t->webrtc1); -- cgit v1.2.3