From 338fbffbe25d477940de7f19f87f015081e5b32c Mon Sep 17 00:00:00 2001 From: Saul Pablo Labajo Izquierdo Date: Wed, 20 Apr 2022 20:19:11 +0200 Subject: [PATCH 1/7] Fix for quick DTLS connection --- .../webrtcendpoint/kmswebrtctransportsink.c | 51 ++++++++++++++++++- .../webrtcendpoint/kmswebrtctransportsink.h | 7 +++ .../kmswebrtctransportsinknice.c | 42 ++++++++++++++- 3 files changed, 98 insertions(+), 2 deletions(-) diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c index 87edf5c4e..116b546c9 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c @@ -31,15 +31,24 @@ G_DEFINE_TYPE (KmsWebrtcTransportSink, kms_webrtc_transport_sink, GST_TYPE_BIN); #define FUNNEL_NAME "funnel" #define SRTPENC_NAME "srtp-encoder" +#define DTLS_ENCODER_NAME "dtls-encoder" static void kms_webrtc_transport_sink_init (KmsWebrtcTransportSink * self) { + GstElement *dtls_encoder; + self->dtlssrtpenc = gst_element_factory_make ("dtlssrtpenc", NULL); + dtls_encoder = gst_bin_get_by_name (GST_BIN(self->dtlssrtpenc), DTLS_ENCODER_NAME); + if (dtls_encoder != NULL) { + gst_element_set_locked_state (dtls_encoder, TRUE); + } else { + GST_WARNING ("Cannot get DTLS encoder with name %s", DTLS_ENCODER_NAME); + } } void -kms_webrtc_transport_sink_connect_elements (KmsWebrtcTransportSink * self) +kms_webrtc_transport_sink_connect_elements (KmsWebrtcTransportSink *self) { GstElement *funnel, *srtpenc; @@ -62,6 +71,7 @@ kms_webrtc_transport_sink_connect_elements (KmsWebrtcTransportSink * self) } else { GST_WARNING ("Cannot get srtpenc with name %s", SRTPENC_NAME); } + } void @@ -77,6 +87,18 @@ kms_webrtc_transport_sink_configure_default (KmsWebrtcTransportSink * self, } } +void +kms_webrtc_transport_sink_set_dtls_is_client_default (KmsWebrtcTransportSink * self, + gboolean is_client) +{ + g_object_set (G_OBJECT (self->dtlssrtpenc), "is-client", is_client, NULL); + if (is_client) { + GST_DEBUG_OBJECT(self, "Set DTLS client"); + } else { + GST_DEBUG_OBJECT(self, "Set DTLS server"); + } +} + void kms_webrtc_transport_sink_configure (KmsWebrtcTransportSink * self, KmsIceBaseAgent * agent, const char *stream_id, guint component_id) @@ -87,12 +109,23 @@ kms_webrtc_transport_sink_configure (KmsWebrtcTransportSink * self, klass->configure (self, agent, stream_id, component_id); } +void +kms_webrtc_transport_sink_set_dtls_is_client (KmsWebrtcTransportSink * self, + gboolean is_client) +{ + KmsWebrtcTransportSinkClass *klass = + KMS_WEBRTC_TRANSPORT_SINK_CLASS (G_OBJECT_GET_CLASS (self)); + + klass->set_dtls_is_client (self, is_client); +} + static void kms_webrtc_transport_sink_class_init (KmsWebrtcTransportSinkClass * klass) { GstElementClass *gstelement_class = GST_ELEMENT_CLASS (klass); klass->configure = kms_webrtc_transport_sink_configure_default; + klass->set_dtls_is_client = kms_webrtc_transport_sink_set_dtls_is_client_default; GST_DEBUG_CATEGORY_INIT (GST_CAT_DEFAULT, GST_DEFAULT_NAME, 0, GST_DEFAULT_NAME); @@ -104,6 +137,22 @@ kms_webrtc_transport_sink_class_init (KmsWebrtcTransportSinkClass * klass) "Miguel París Díaz "); } +void +kms_webrtc_transport_sink_start_dtls (KmsWebrtcTransportSink * self) +{ + GstElement *dtls_encoder; + + dtls_encoder = gst_bin_get_by_name (GST_BIN(self->dtlssrtpenc), DTLS_ENCODER_NAME); + if (dtls_encoder != NULL) { + gst_element_set_locked_state (dtls_encoder, FALSE); + gst_element_sync_state_with_parent (dtls_encoder); + GST_DEBUG_OBJECT(self, "Starting DTLS"); + } else { + GST_WARNING ("Cannot get DTLS encoder with name %s", DTLS_ENCODER_NAME); + } +} + + KmsWebrtcTransportSink * kms_webrtc_transport_sink_new () { diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.h b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.h index 711ecee41..27d538cb5 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.h +++ b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.h @@ -55,6 +55,9 @@ struct _KmsWebrtcTransportSinkClass KmsIceBaseAgent *agent, const char *stream_id, guint component_id); + + void (*set_dtls_is_client) (KmsWebrtcTransportSink * self, + gboolean is_client); }; GType kms_webrtc_transport_sink_get_type (void); @@ -65,5 +68,9 @@ void kms_webrtc_transport_sink_configure (KmsWebrtcTransportSink * self, KmsIceBaseAgent *agent, const char *stream_id, guint component_id); +void kms_webrtc_transport_sink_set_dtls_is_client (KmsWebrtcTransportSink * self, + gboolean is_client); +void kms_webrtc_transport_sink_start_dtls (KmsWebrtcTransportSink * self); + G_END_DECLS #endif /* __KMS_WEBRTC_TRANSPORT_SINK_H__ */ diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsinknice.c b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsinknice.c index 594f6b860..a01f7549f 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsinknice.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsinknice.c @@ -36,12 +36,31 @@ static void kms_webrtc_transport_sink_nice_init (KmsWebrtcTransportSinkNice * self) { KmsWebrtcTransportSink *parent = KMS_WEBRTC_TRANSPORT_SINK (self); - + parent->sink = gst_element_factory_make ("nicesink", NULL); kms_webrtc_transport_sink_connect_elements (parent); } +static void +kms_webrtc_transport_sink_nice_component_state_changed (KmsIceBaseAgent * agent, + char *stream_id, guint component_id, IceState state, + KmsWebrtcTransportSink * self) +{ + gboolean is_client; + + GST_LOG_OBJECT (self, + "[IceComponentStateChanged] state: %s, stream_id: %s, component_id: %u", + kms_ice_base_agent_state_to_string (state), stream_id, component_id); + + g_object_get (G_OBJECT (self->dtlssrtpenc), "is-client", + &is_client, NULL); + + if ((state == ICE_STATE_CONNECTED) && is_client) { + kms_webrtc_transport_sink_start_dtls (self); + } +} + void kms_webrtc_transport_sink_nice_configure (KmsWebrtcTransportSink * self, KmsIceBaseAgent * agent, const char *stream_id, guint component_id) @@ -53,6 +72,26 @@ kms_webrtc_transport_sink_nice_configure (KmsWebrtcTransportSink * self, "agent", kms_ice_nice_agent_get_agent (nice_agent), "stream", id, "component", component_id, "sync", FALSE, "async", FALSE, NULL); + + g_signal_connect (nice_agent, "on-ice-component-state-changed", + G_CALLBACK (kms_webrtc_transport_sink_nice_component_state_changed), self); +} + +void +kms_webrtc_transport_sink_nice_set_dtls_is_client (KmsWebrtcTransportSink * self, + gboolean is_client) +{ + KmsWebrtcTransportSinkNiceClass *klass = + KMS_WEBRTC_TRANSPORT_SINK_NICE_CLASS (G_OBJECT_GET_CLASS (self)); + KmsWebrtcTransportSinkClass *parent_klass = + KMS_WEBRTC_TRANSPORT_SINK_CLASS (g_type_class_peek_parent(klass)); + + parent_klass->set_dtls_is_client (self, is_client); + + if (!is_client) { + kms_webrtc_transport_sink_start_dtls (self); + } + } static void @@ -64,6 +103,7 @@ kms_webrtc_transport_sink_nice_class_init (KmsWebrtcTransportSinkNiceClass * base_class = KMS_WEBRTC_TRANSPORT_SINK_CLASS (klass); base_class->configure = kms_webrtc_transport_sink_nice_configure; + base_class->set_dtls_is_client = kms_webrtc_transport_sink_nice_set_dtls_is_client; GST_DEBUG_CATEGORY_INIT (GST_CAT_DEFAULT, GST_DEFAULT_NAME, 0, GST_DEFAULT_NAME); From 7e416faecffc5f32440bff8057fa87115f751d80 Mon Sep 17 00:00:00 2001 From: Saul Pablo Labajo Izquierdo Date: Wed, 20 Apr 2022 20:29:33 +0200 Subject: [PATCH 2/7] setting is-client as virtual method --- src/gst-plugins/webrtcendpoint/kmswebrtcbundleconnection.c | 2 +- src/gst-plugins/webrtcendpoint/kmswebrtcconnection.c | 2 +- src/gst-plugins/webrtcendpoint/kmswebrtcrtcpmuxconnection.c | 2 +- src/gst-plugins/webrtcendpoint/kmswebrtcsctpconnection.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtcbundleconnection.c b/src/gst-plugins/webrtcendpoint/kmswebrtcbundleconnection.c index 34ae34750..ae6a570d0 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtcbundleconnection.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtcbundleconnection.c @@ -92,7 +92,7 @@ kms_webrtc_bundle_connection_add (KmsIRtpConnection * base_rtp_conn, KmsWebRtcBundleConnectionPrivate *priv = self->priv; KmsWebRtcTransport *tr = priv->tr; - g_object_set (G_OBJECT (tr->sink->dtlssrtpenc), "is-client", active, NULL); + kms_webrtc_transport_sink_set_dtls_is_client (tr->sink, active); gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->src))); gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->sink))); diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtcconnection.c b/src/gst-plugins/webrtcendpoint/kmswebrtcconnection.c index 52a74f331..aaee0cdd4 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtcconnection.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtcconnection.c @@ -79,7 +79,7 @@ kms_webrtc_connection_get_certificate_pem (KmsWebRtcBaseConnection * base_conn) static void add_tr (KmsWebRtcTransport * tr, GstBin * bin, gboolean is_client) { - g_object_set (G_OBJECT (tr->sink->dtlssrtpenc), "is-client", is_client, NULL); + kms_webrtc_transport_sink_set_dtls_is_client(tr->sink, is_client); gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->src))); gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->sink))); diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtcrtcpmuxconnection.c b/src/gst-plugins/webrtcendpoint/kmswebrtcrtcpmuxconnection.c index d24bf5766..b2165ce2f 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtcrtcpmuxconnection.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtcrtcpmuxconnection.c @@ -87,7 +87,7 @@ kms_webrtc_rtcp_mux_connection_add (KmsIRtpConnection * base_rtp_conn, KmsWebRtcTransport *tr = priv->tr; /* srcs */ - g_object_set (G_OBJECT (tr->sink->dtlssrtpenc), "is-client", active, NULL); + kms_webrtc_transport_sink_set_dtls_is_client(tr->sink, active); gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->src))); gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->sink))); diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtcsctpconnection.c b/src/gst-plugins/webrtcendpoint/kmswebrtcsctpconnection.c index 1f7d4406d..1e47390b0 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtcsctpconnection.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtcsctpconnection.c @@ -79,7 +79,7 @@ kms_webrtc_sctp_connection_add (KmsIRtpConnection * base_conn, GstBin * bin, KmsWebRtcSctpConnectionPrivate *priv = self->priv; KmsWebRtcTransport *tr = priv->tr; - g_object_set (G_OBJECT (tr->sink->dtlssrtpenc), "is-client", active, NULL); + kms_webrtc_transport_sink_set_dtls_is_client(tr->sink, active); gst_bin_add (bin, GST_ELEMENT (g_object_ref (self->priv->tr->src))); gst_bin_add (bin, GST_ELEMENT (g_object_ref (self->priv->tr->sink))); From 24cb74d1e8dd897983b9a619ab757f7472573fcb Mon Sep 17 00:00:00 2001 From: Saul Pablo Labajo Izquierdo Date: Wed, 20 Apr 2022 20:30:00 +0200 Subject: [PATCH 3/7] Avoided race condition freeing resources --- tests/check/element/webrtcendpoint.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/check/element/webrtcendpoint.c b/tests/check/element/webrtcendpoint.c index 2e23adb1d..95a012804 100644 --- a/tests/check/element/webrtcendpoint.c +++ b/tests/check/element/webrtcendpoint.c @@ -1744,6 +1744,7 @@ test_data_channels (gboolean bundle) GST_WARNING ("Finishing test"); + g_usleep (500000); gst_element_set_state (pipeline, GST_STATE_NULL); gst_bus_remove_signal_watch (bus); g_object_unref (bus); From 2dc9d1e6bf4cca7407921b118bbd1d250b8b15be Mon Sep 17 00:00:00 2001 From: Saul Pablo Labajo Izquierdo Date: Thu, 21 Apr 2022 10:49:24 +0200 Subject: [PATCH 4/7] Added quick DTLS connection test --- tests/server/webRtcEndpoint.cpp | 137 +++++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 2 deletions(-) diff --git a/tests/server/webRtcEndpoint.cpp b/tests/server/webRtcEndpoint.cpp index 70681fe34..075b5bfd7 100644 --- a/tests/server/webRtcEndpoint.cpp +++ b/tests/server/webRtcEndpoint.cpp @@ -32,6 +32,8 @@ #define NUMBER_OF_RECONNECTIONS 5 +#include + using namespace kurento; using namespace boost::unit_test; @@ -180,6 +182,7 @@ ice_state_changes (bool useIpv6) std::condition_variable cv; std::mutex mtx; std::unique_lock lck (mtx); + bool active = true; std::shared_ptr webRtcEpOfferer = createWebrtc(); std::shared_ptr webRtcEpAnswerer = createWebrtc(); @@ -188,11 +191,15 @@ ice_state_changes (bool useIpv6) webRtcEpAnswerer->setName ("answerer"); webRtcEpOfferer->signalOnIceCandidate.connect ([&] (OnIceCandidate event) { - exchange_candidate (event, webRtcEpAnswerer, useIpv6); + if (active) { + exchange_candidate (event, webRtcEpAnswerer, useIpv6); + } }); webRtcEpAnswerer->signalOnIceCandidate.connect ([&] (OnIceCandidate event) { - exchange_candidate (event, webRtcEpOfferer, useIpv6); + if (active) { + exchange_candidate (event, webRtcEpOfferer, useIpv6); + } }); webRtcEpOfferer->signalOnIceComponentStateChanged.connect ([&] ( @@ -213,6 +220,7 @@ ice_state_changes (bool useIpv6) }) ) { BOOST_ERROR ("Timeout waiting for ICE state change"); } + active = false; if (!ice_state_changed) { BOOST_ERROR ("ICE state not chagned"); @@ -234,6 +242,126 @@ ice_state_changes_ipv6 () ice_state_changes (true); } +// This test depends on Gstreamer 1.17+ to be installed and on the +// feature of DTLS connection state and event on property changes as implemented on +// https://github.com/naevatec/kms-elements/tree/dtls-connection-state +// +// That feature will be PR'd when KMS reaches at least GStreamer 1.17 +/****************************************************** +static void +dtls_quick_connection_test (bool useIpv6) +{ + DtlsConnectionState offerer_dtls_connection_state = DtlsConnectionState::FAILED; + DtlsConnectionState answerer_dtls_connection_state = DtlsConnectionState::FAILED; + std::condition_variable cv; + std::mutex mtx; + std::unique_lock lck (mtx); + uint64_t ice_gathering_started = 0; + uint64_t dtls_connection_started_offerer = 0; + uint64_t dtls_connection_started_answerer = 0; + uint64_t dtls_connection_connecting_offerer = 0; + uint64_t dtls_connection_connecting_answerer = 0; + uint64_t dtls_connection_connected_offerer = 0; + uint64_t dtls_connection_connected_answerer = 0; + + std::shared_ptr webRtcEpOfferer = createWebrtc(); + std::shared_ptr webRtcEpAnswerer = createWebrtc(); + + webRtcEpOfferer->setName ("offerer"); + webRtcEpAnswerer->setName ("answerer"); + + webRtcEpOfferer->signalOnIceCandidate.connect ([&] (OnIceCandidate event) { + exchange_candidate (event, webRtcEpAnswerer, useIpv6); + }); + + webRtcEpAnswerer->signalOnIceCandidate.connect ([&] (OnIceCandidate event) { + exchange_candidate (event, webRtcEpOfferer, useIpv6); + }); + + webRtcEpOfferer->signalDtlsConnectionStateChange.connect ([&] ( + DtlsConnectionStateChange event) { + offerer_dtls_connection_state = *(event.getState()); + BOOST_TEST_MESSAGE("Offerer DTLS connection state: " + offerer_dtls_connection_state.getString() + " component: " + event.getComponentId() + " stream " + event.getStreamId()); + // Using current KMS offerer is passive one so it gets to the NEW state immediately + if (offerer_dtls_connection_state.getValue() == DtlsConnectionState::NEW) { + dtls_connection_started_offerer = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); + } else if (offerer_dtls_connection_state.getValue() == DtlsConnectionState::CONNECTED) { + dtls_connection_connected_offerer = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); + cv.notify_one(); + } else if (offerer_dtls_connection_state.getValue() == DtlsConnectionState::CONNECTING) { + dtls_connection_connecting_offerer = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); + } + }); + + + webRtcEpAnswerer->signalDtlsConnectionStateChange.connect ([&] ( + DtlsConnectionStateChange event) { + answerer_dtls_connection_state = *(event.getState()); + BOOST_TEST_MESSAGE("Answerer DTLS connection state: " + answerer_dtls_connection_state.getString() + " component: " + event.getComponentId() + " stream " + event.getStreamId()); + // Using current KMS answerer is active so its is-client is true and should only be reached after the ICE connection gets to + // CONNECTED state with the feature we are testing. + if (answerer_dtls_connection_state.getValue() == DtlsConnectionState::NEW) { + dtls_connection_started_answerer = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); + } else if (answerer_dtls_connection_state.getValue() == DtlsConnectionState::CONNECTED) { + dtls_connection_connected_answerer = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); + cv.notify_one(); + } else if (answerer_dtls_connection_state.getValue() == DtlsConnectionState::CONNECTING) { + dtls_connection_connecting_answerer = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); + } + }); + + + std::string offer = webRtcEpOfferer->generateOffer (); + std::string answer = webRtcEpAnswerer->processOffer (offer); + webRtcEpOfferer->processAnswer (answer); + + + // Just to check the feature we wait for some seconds, if feature is not working, DTLS Hello should be triggered immediately and + // as it is dropped (no valid candidate pair) exponential backoff will take place + // This delay just makes the ICE connection to take longer + // Although testing with delays is always a not so good idea, due to the dependencies it get about the conditions of the testing host + // in this case we are using it just to enhance the difference between the feature working (DTLS NEW state only reached after ICE connection + // reaching CONNECTED state for answerer), and not working (NEW state reached immediately after processOffer is done on answerer) + std::this_thread::sleep_for(std::chrono::milliseconds(1500)); + ice_gathering_started = std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count(); + + webRtcEpOfferer->gatherCandidates (); + webRtcEpAnswerer->gatherCandidates (); + + if (!cv.wait_for (lck, std::chrono::seconds (4*TIMEOUT), [&] () { + return ((offerer_dtls_connection_state.getValue() == DtlsConnectionState::CONNECTED) && (answerer_dtls_connection_state.getValue() == DtlsConnectionState::CONNECTED)); + }) ) { + BOOST_ERROR ("Timeout waiting for ICE state change"); + } + + // Correct outcome is answerer DTLS NEW state reached after ICE gathering is started + // Incorrect outcome is answerer DTLS NEW state reached before ICE gathering is started + // Either case offerer DTLS NEW state should be reached before ICE gathering is started + if (dtls_connection_started_answerer <= ice_gathering_started) { + BOOST_ERROR ("DTLS quick connection is not working"); + } + if (dtls_connection_started_offerer > ice_gathering_started) { + BOOST_ERROR ("This should not happen"); + } + + releaseWebRtc (webRtcEpOfferer); + releaseWebRtc (webRtcEpAnswerer); +} + + +static void +dtls_quick_connection_test_ipv6 () +{ + dtls_quick_connection_test (true); +} + +static void +dtls_quick_connection_test_ipv4 () +{ + dtls_quick_connection_test (false); +} +******************************/ + static void stun_turn_properties () { @@ -714,6 +842,11 @@ init_unit_test_suite ( int , char *[] ) test->add (BOOST_TEST_CASE ( &stun_turn_properties ), 0, /* timeout */ 15); test->add (BOOST_TEST_CASE ( &media_state_changes_ipv4 ), 0, /* timeout */ 15); test->add (BOOST_TEST_CASE ( &media_state_changes_ipv6 ), 0, /* timeout */ 15); + + /* These tests depend on GStreamer 1.17+ and feature on https://github.com/naevatec/kms-elements/tree/dtls-connection-state*/ + //test->add (BOOST_TEST_CASE (&dtls_quick_connection_test_ipv4), 0, /* timeout */ 15); + //test->add (BOOST_TEST_CASE (&dtls_quick_connection_test_ipv6), 0, /* timeout */ 15); + test->add (BOOST_TEST_CASE ( &connection_state_changes_ipv4 ), 0, /* timeout */ 15); test->add (BOOST_TEST_CASE ( &connection_state_changes_ipv6 ), From c53382271bb0fe81d6b185371976f0c1ed833061 Mon Sep 17 00:00:00 2001 From: Saul Pablo Labajo Izquierdo Date: Thu, 21 Apr 2022 11:39:04 +0200 Subject: [PATCH 5/7] fixes --- .../webrtcendpoint/kmswebrtctransportsink.c | 54 +++++++++++++++---- .../kmswebrtctransportsinknice.c | 2 +- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c index 116b546c9..828efc0d0 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c @@ -29,9 +29,40 @@ GST_DEBUG_CATEGORY_STATIC (GST_CAT_DEFAULT); #define kms_webrtc_transport_sink_parent_class parent_class G_DEFINE_TYPE (KmsWebrtcTransportSink, kms_webrtc_transport_sink, GST_TYPE_BIN); -#define FUNNEL_NAME "funnel" -#define SRTPENC_NAME "srtp-encoder" -#define DTLS_ENCODER_NAME "dtls-encoder" +#define FUNNEL_FACTORY_NAME "funnel" +#define SRTPENC_FACTORY_NAME "srtpenc" +#define DTLS_ENCODER_FACTORY_NAME "dtlsenc" + + +static GType +get_type_from_factory_name (const gchar *factory_name) +{ + GstElementFactory *factory; + GType type = 0; + + factory = gst_element_factory_find (factory_name) + + if (factory != NULL) { + type = gst_element_factory_get_element_type (factory_name); + + g_object_unref(factory); + } + return type; +} + +static GElement* +kms_webrtc_transport_sink_get_element_in_dtlssrtpenc (KmsWebrtcTransportSink *self, const gchar *factory_name) +{ + GType type; + + type = get_type_from_factory_name (factory_name); + if (type != 0) { + return gst_bin_get_by_interface (GST_BIN(self->dtlssrtpenc), type); + } else { + GST_WARNING_OBJECT (self, "Factory %s not installed", factory_name); + return NULL; + } +} static void kms_webrtc_transport_sink_init (KmsWebrtcTransportSink * self) @@ -39,11 +70,12 @@ kms_webrtc_transport_sink_init (KmsWebrtcTransportSink * self) GstElement *dtls_encoder; self->dtlssrtpenc = gst_element_factory_make ("dtlssrtpenc", NULL); - dtls_encoder = gst_bin_get_by_name (GST_BIN(self->dtlssrtpenc), DTLS_ENCODER_NAME); + dtls_encoder = kms_webrtc_transport_sink_get_element_in_dtlssrtpenc (self, DTLS_ENCODER_FACTORY_NAME); if (dtls_encoder != NULL) { gst_element_set_locked_state (dtls_encoder, TRUE); + g_object_unref (dtls_encoder); } else { - GST_WARNING ("Cannot get DTLS encoder with name %s", DTLS_ENCODER_NAME); + GST_WARNING_OBJECT (self, "Cannot get DTLS encoder"); } } @@ -55,7 +87,7 @@ kms_webrtc_transport_sink_connect_elements (KmsWebrtcTransportSink *self) gst_bin_add_many (GST_BIN (self), self->dtlssrtpenc, self->sink, NULL); gst_element_link (self->dtlssrtpenc, self->sink); - funnel = gst_bin_get_by_name (GST_BIN (self->dtlssrtpenc), FUNNEL_NAME); + funnel = kms_webrtc_transport_sink_get_element_in_dtlssrtpenc (self, FUNNEL_FACTORY_NAME); if (funnel != NULL) { g_object_set (funnel, "forward-sticky-events-mode", 0 /* never */ , NULL); g_object_unref (funnel); @@ -63,7 +95,7 @@ kms_webrtc_transport_sink_connect_elements (KmsWebrtcTransportSink *self) GST_WARNING ("Cannot get funnel with name %s", FUNNEL_NAME); } - srtpenc = gst_bin_get_by_name (GST_BIN (self->dtlssrtpenc), SRTPENC_NAME); + srtpenc = kms_webrtc_transport_sink_get_element_in_dtlssrtpenc (self, SRTPENC_FACTORY_NAME); if (srtpenc != NULL) { g_object_set (srtpenc, "allow-repeat-tx", TRUE, "replay-window-size", RTP_RTX_SIZE, NULL); @@ -93,9 +125,9 @@ kms_webrtc_transport_sink_set_dtls_is_client_default (KmsWebrtcTransportSink * s { g_object_set (G_OBJECT (self->dtlssrtpenc), "is-client", is_client, NULL); if (is_client) { - GST_DEBUG_OBJECT(self, "Set DTLS client"); + GST_DEBUG_OBJECT(self, "Set as DTLS client (handshake initiator)"); } else { - GST_DEBUG_OBJECT(self, "Set DTLS server"); + GST_DEBUG_OBJECT(self, "Set as DTLS server (wait for handshake)"); } } @@ -142,11 +174,13 @@ kms_webrtc_transport_sink_start_dtls (KmsWebrtcTransportSink * self) { GstElement *dtls_encoder; - dtls_encoder = gst_bin_get_by_name (GST_BIN(self->dtlssrtpenc), DTLS_ENCODER_NAME); + dtls_encoder = kms_webrtc_transport_sink_get_element_in_dtlssrtpenc (self, DTLS_ENCODER_FACTORY_NAME); if (dtls_encoder != NULL) { gst_element_set_locked_state (dtls_encoder, FALSE); gst_element_sync_state_with_parent (dtls_encoder); GST_DEBUG_OBJECT(self, "Starting DTLS"); + + g_object_unref (dtls_encoder); } else { GST_WARNING ("Cannot get DTLS encoder with name %s", DTLS_ENCODER_NAME); } diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsinknice.c b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsinknice.c index a01f7549f..8f2117823 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsinknice.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsinknice.c @@ -36,7 +36,7 @@ static void kms_webrtc_transport_sink_nice_init (KmsWebrtcTransportSinkNice * self) { KmsWebrtcTransportSink *parent = KMS_WEBRTC_TRANSPORT_SINK (self); - + parent->sink = gst_element_factory_make ("nicesink", NULL); kms_webrtc_transport_sink_connect_elements (parent); From c09665d2d37a178b4055ad547a3893e5d0d10def Mon Sep 17 00:00:00 2001 From: Saul Pablo Labajo Izquierdo Date: Thu, 21 Apr 2022 13:10:34 +0200 Subject: [PATCH 6/7] requested fixes completed (cherry picked from commit d3ed7b78b32aaa099ec34d0f7b7afff98b485877) --- .../webrtcendpoint/kmswebrtctransportsink.c | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c index 828efc0d0..3d8fcb41a 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c @@ -34,34 +34,36 @@ G_DEFINE_TYPE (KmsWebrtcTransportSink, kms_webrtc_transport_sink, GST_TYPE_BIN); #define DTLS_ENCODER_FACTORY_NAME "dtlsenc" -static GType -get_type_from_factory_name (const gchar *factory_name) + +static GstElement* +kms_webrtc_transport_sink_get_element_in_dtlssrtpenc (KmsWebrtcTransportSink *self, const gchar *factory_name) { + GstIterator *iterator; + GValue item = G_VALUE_INIT; + GstElement *element; GstElementFactory *factory; - GType type = 0; - - factory = gst_element_factory_find (factory_name) - if (factory != NULL) { - type = gst_element_factory_get_element_type (factory_name); + factory = gst_element_factory_find (factory_name); - g_object_unref(factory); + if (factory == NULL) { + GST_WARNING_OBJECT(self, "Factory %s not installed", factory_name); + return NULL; } - return type; -} -static GElement* -kms_webrtc_transport_sink_get_element_in_dtlssrtpenc (KmsWebrtcTransportSink *self, const gchar *factory_name) -{ - GType type; - - type = get_type_from_factory_name (factory_name); - if (type != 0) { - return gst_bin_get_by_interface (GST_BIN(self->dtlssrtpenc), type); - } else { - GST_WARNING_OBJECT (self, "Factory %s not installed", factory_name); - return NULL; + // Until KMS is updated to GStreamer 1.18 and method https://gstreamer.freedesktop.org/documentation/gstreamer/gstbin.html#gst_bin_iterate_all_by_element_factory_name + // is available, this will do + iterator = gst_bin_iterate_elements (GST_BIN(self->dtlssrtpenc)); + while (gst_iterator_next (iterator, &item) == GST_ITERATOR_OK) { + element = (GstElement *) g_value_get_object (&item); + if (factory == gst_element_get_factory (element)) { + break; + } else { + element = NULL; + } } + gst_iterator_free (iterator); + g_object_unref (factory); + return element; } static void @@ -92,7 +94,7 @@ kms_webrtc_transport_sink_connect_elements (KmsWebrtcTransportSink *self) g_object_set (funnel, "forward-sticky-events-mode", 0 /* never */ , NULL); g_object_unref (funnel); } else { - GST_WARNING ("Cannot get funnel with name %s", FUNNEL_NAME); + GST_WARNING ("Cannot get funnel with factory %s", FUNNEL_FACTORY_NAME); } srtpenc = kms_webrtc_transport_sink_get_element_in_dtlssrtpenc (self, SRTPENC_FACTORY_NAME); @@ -101,7 +103,7 @@ kms_webrtc_transport_sink_connect_elements (KmsWebrtcTransportSink *self) RTP_RTX_SIZE, NULL); g_object_unref (srtpenc); } else { - GST_WARNING ("Cannot get srtpenc with name %s", SRTPENC_NAME); + GST_WARNING ("Cannot get srtpenc with factory %s", SRTPENC_FACTORY_NAME); } } @@ -182,7 +184,7 @@ kms_webrtc_transport_sink_start_dtls (KmsWebrtcTransportSink * self) g_object_unref (dtls_encoder); } else { - GST_WARNING ("Cannot get DTLS encoder with name %s", DTLS_ENCODER_NAME); + GST_WARNING_OBJECT ("Cannot get DTLS encoder with factory %s", DTLS_ENCODER_FACTORY_NAME); } } From a45c33acb6e853bade8db77b7fcb95aaa30006ad Mon Sep 17 00:00:00 2001 From: Saul Pablo Labajo Izquierdo Date: Thu, 28 Apr 2022 12:22:50 +0200 Subject: [PATCH 7/7] Better memory handling (cherry picked from commit a0a036dd42bd79e04969f2eb1f511689412c398b) --- src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c index 3d8fcb41a..31382d5b7 100644 --- a/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c +++ b/src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c @@ -63,6 +63,11 @@ kms_webrtc_transport_sink_get_element_in_dtlssrtpenc (KmsWebrtcTransportSink *se } gst_iterator_free (iterator); g_object_unref (factory); + + if (element != NULL) { + element = g_value_dup_object (&item); + g_value_unset (&item); + } return element; }