Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Commit

Permalink
Dtls server web rtc quick connection - fixed names collision (#40)
Browse files Browse the repository at this point in the history
## What is the current behavior you want to change?
This change is related to a previous PR in #37
More specifically this change covers a use case not covered in the previous PR. This is as follows:

 - When Kurento is set as DTLS server it may happen that the remote peer of the WebRTC connection reaches to ICE connected state before Kurento does. 
 - The problem appears when not only the remote peer gets ICE connected before but when the peer sends the DTLS Client Hello and it arrives before the WebRTCEndpoint in Kurento has reached the ICE connected state. 
 - If this happens, the DTLS Client hello is received in the nicesrc element from the Webrtctransportsrc and it is relayed to the dtlssrtpdec element. This in turns starts DTLS handshake and sends back a DTLS Server hello to the nicesink element. But, and this is the real problem, the nicesink finds the ICE connection not yet established and silently drops the DTLS server hello.
 - When the DTLS timeout is up (1 second this first time, but doubling each time), the endpoint will resend again the DTLS server hello packet, and hopefully this time it will find the ICE connection established and the DTLS server hello will be delivered to the remote peer. If not, it will try again on the next timeout. 
 - The problem is that each timeout introduces some additional delay in the WebRTC connection establishment. 
 - In many cases  that we have observed, the DTLS Client hello just arrives a few milliseconds before the ICE gets CONNECTED in WebRTCEndpoint. But the impact is that those few miliseconds will make the connection to delay around 1 second. 

This problem has a lesser impact than the previous PR, but in the end it is worth improving it.

## What is the new behavior provided by this change?
 - This change only takes effect when the WebRTCEndpoint is configured as DTLS server (waiter for DTLS handshake)
 - It just captures all packets coming from the nicesrc element to the dtlssrpdec and stores them until the ICE gets connected. 
 - A signal is connected so that when ICE connection changes state, and the new state is connected, it gets the temporary stored buffers and delivers them to the dtlssrtpdec element, thus ensuring the DTLS handshake process only takes place when the ICE connection is established.

According to our observations there is only one possible buffer stored that corresponds to the DTLS Client hello received from remote peer. Also, we noticed that if in the same call that notifies ICE connection we delivered the stored DTLS client hello, the answer (DTLS server hello) still finds the ICE connection not usable, so to avoid that, we just make that delivery asynchronously in a separate thread 10 ms later.

## How has this been tested?
IT has been tested with automatic tests that cannot yet be used in this Kurento version as they need GStreamer 1.18 to work.
We have also tested it in real environmentes capturing network traffic to see that the change causes the desired behaviour.
  • Loading branch information
slabajo authored Jul 11, 2022
1 parent 70a4f1e commit 75db50c
Show file tree
Hide file tree
Showing 10 changed files with 513 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/gst-plugins/webrtcendpoint/kmswebrtcbundleconnection.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ kms_webrtc_bundle_connection_add (KmsIRtpConnection * base_rtp_conn,
KmsWebRtcTransport *tr = priv->tr;

kms_webrtc_transport_sink_set_dtls_is_client (tr->sink, active);
kms_webrtc_transport_src_set_dtls_is_client (tr->src, active);

gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->src)));
gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->sink)));
Expand Down
1 change: 1 addition & 0 deletions src/gst-plugins/webrtcendpoint/kmswebrtcconnection.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ static void
add_tr (KmsWebRtcTransport * tr, GstBin * bin, gboolean is_client)
{
kms_webrtc_transport_sink_set_dtls_is_client(tr->sink, is_client);
kms_webrtc_transport_src_set_dtls_is_client (tr->src, is_client);

gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->src)));
gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->sink)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ kms_webrtc_rtcp_mux_connection_add (KmsIRtpConnection * base_rtp_conn,

/* srcs */
kms_webrtc_transport_sink_set_dtls_is_client(tr->sink, active);
kms_webrtc_transport_src_set_dtls_is_client (tr->src, active);

gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->src)));
gst_bin_add (bin, GST_ELEMENT (g_object_ref (tr->sink)));
Expand Down
1 change: 1 addition & 0 deletions src/gst-plugins/webrtcendpoint/kmswebrtcsctpconnection.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ kms_webrtc_sctp_connection_add (KmsIRtpConnection * base_conn, GstBin * bin,
KmsWebRtcTransport *tr = priv->tr;

kms_webrtc_transport_sink_set_dtls_is_client(tr->sink, active);
kms_webrtc_transport_src_set_dtls_is_client (tr->src, 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)));
Expand Down
2 changes: 1 addition & 1 deletion src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ compare_factory_names (const GValue *velement, GValue *factory_name_val)
}
//
// https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/10f72da5040b74678c8f81723971127ee8bee04f/subprojects/gstreamer/gst/gstbin.c#L4553-4574
GstIterator *
static GstIterator *
gst_bin_iterate_all_by_element_factory_name (GstBin *bin,
const gchar *factory_name)
{
Expand Down
109 changes: 106 additions & 3 deletions src/gst-plugins/webrtcendpoint/kmswebrtctransportsrc.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,85 @@ GST_DEBUG_CATEGORY_STATIC (GST_CAT_DEFAULT);
#define kms_webrtc_transport_src_parent_class parent_class
G_DEFINE_TYPE (KmsWebrtcTransportSrc, kms_webrtc_transport_src, GST_TYPE_BIN);

#define SRTPDEC_NAME "srtp-decoder"
#define SRTPDEC_FACTORY_NAME "srtpdec"


// {{{{ FIXME: This can be deleted when we start using GStreamer >=1.18 for Kurento.
// Code sourced from GStreamer/gstbin.c >=1.18
//
// https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/10f72da5040b74678c8f81723971127ee8bee04f/subprojects/gstreamer/gst/gstbin.c#L4526-4537
static gint
compare_factory_names (const GValue *velement, GValue *factory_name_val)
{
GstElement *element = g_value_get_object (velement);
GstElementFactory *factory = gst_element_get_factory (element);
const gchar *factory_name = g_value_get_string (factory_name_val);

if (factory == NULL)
return -1;

return g_strcmp0 (GST_OBJECT_NAME (factory), factory_name);
}

//
// https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/10f72da5040b74678c8f81723971127ee8bee04f/subprojects/gstreamer/gst/gstbin.c#L4553-4574
static GstIterator *
gst_bin_iterate_all_by_element_factory_name (GstBin *bin,
const gchar *factory_name)
{
GstIterator *children;
GstIterator *result;
GValue factory_name_val = G_VALUE_INIT;

g_return_val_if_fail (GST_IS_BIN (bin), NULL);
g_return_val_if_fail (factory_name && *factory_name, NULL);

g_value_init (&factory_name_val, G_TYPE_STRING);
g_value_set_string (&factory_name_val, factory_name);

children = gst_bin_iterate_recurse (bin);
result = gst_iterator_filter (children, (GCompareFunc)compare_factory_names,
&factory_name_val);

g_value_unset (&factory_name_val);

return result;
}
// }}}}

static GstElement *
kms_webrtc_transport_src_get_element_in_dtlssrtpdec (
KmsWebrtcTransportSrc *self,
const gchar *factory_name)
{
GstElement *element = NULL;
GValue velement = G_VALUE_INIT;

GstIterator *iter = gst_bin_iterate_all_by_element_factory_name (
GST_BIN (self->dtlssrtpdec), factory_name);

if (gst_iterator_next (iter, &velement) == GST_ITERATOR_OK) {
// Assume only one element of the given type. This is the case in dtlssrtpdec.
element = g_value_dup_object (&velement);
g_value_reset (&velement);
}

if (gst_iterator_next (iter, &velement) != GST_ITERATOR_DONE) {
GST_WARNING_OBJECT (self,
"BUG: Several elements '%s' found in dtlssrtpdec; code assumes only one",
factory_name);
}

if (element == NULL) {
GST_WARNING_OBJECT (self, "BUG: Element '%s' not found in dtlssrtpdec",
factory_name);
}

g_value_unset (&velement);
gst_iterator_free (iter);

return element;
}

static void
kms_webrtc_transport_src_init (KmsWebrtcTransportSrc * self)
Expand All @@ -45,12 +123,12 @@ kms_webrtc_transport_src_connect_elements (KmsWebrtcTransportSrc * self)
gst_bin_add_many (GST_BIN (self), self->src, self->dtlssrtpdec, NULL);
gst_element_link (self->src, self->dtlssrtpdec);

srtpdec = gst_bin_get_by_name (GST_BIN (self->dtlssrtpdec), SRTPDEC_NAME);
srtpdec = kms_webrtc_transport_src_get_element_in_dtlssrtpdec (self, SRTPDEC_FACTORY_NAME);
if (srtpdec != NULL) {
g_object_set (srtpdec, "replay-window-size", RTP_RTX_SIZE, NULL);
g_object_unref (srtpdec);
} else {
GST_WARNING ("Cannot get srtpdec with name %s", SRTPDEC_NAME);
GST_WARNING ("Cannot get SRTP DTLS decoder");
}
}

Expand All @@ -77,12 +155,27 @@ kms_webrtc_transport_src_configure (KmsWebrtcTransportSrc * self,
klass->configure (self, agent, stream_id, component_id);
}

void
kms_webrtc_transport_src_set_dtls_is_client_default (KmsWebrtcTransportSrc * self,
gboolean is_client)
{
// We just need to cache the DTLS client value for later use
self->dtls_client = is_client;

if (is_client) {
GST_DEBUG_OBJECT(self, "Set as DTLS client (handshake initiator)");
} else {
GST_DEBUG_OBJECT(self, "Set as DTLS server (wait for handshake)");
}
}

static void
kms_webrtc_transport_src_class_init (KmsWebrtcTransportSrcClass * klass)
{
GstElementClass *gstelement_class = GST_ELEMENT_CLASS (klass);

klass->configure = kms_webrtc_transport_src_configure_default;
klass->set_dtls_is_client = kms_webrtc_transport_src_set_dtls_is_client_default;

GST_DEBUG_CATEGORY_INIT (GST_CAT_DEFAULT, GST_DEFAULT_NAME, 0,
GST_DEFAULT_NAME);
Expand All @@ -103,3 +196,13 @@ kms_webrtc_transport_src_new ()

return KMS_WEBRTC_TRANSPORT_SRC (obj);
}

void
kms_webrtc_transport_src_set_dtls_is_client (KmsWebrtcTransportSrc * self,
gboolean is_client)
{
KmsWebrtcTransportSrcClass *klass =
KMS_WEBRTC_TRANSPORT_SRC_CLASS (G_OBJECT_GET_CLASS (self));

klass->set_dtls_is_client (self, is_client);
}
9 changes: 9 additions & 0 deletions src/gst-plugins/webrtcendpoint/kmswebrtctransportsrc.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ struct _KmsWebrtcTransportSrc
GstElement *src;
GstElement *dtlssrtpdec;
gulong src_probe;

gboolean dtls_client;
};

struct _KmsWebrtcTransportSrcClass
Expand All @@ -56,6 +58,10 @@ struct _KmsWebrtcTransportSrcClass
KmsIceBaseAgent *agent,
const char *stream_id,
guint component_id);

void (*set_dtls_is_client) (KmsWebrtcTransportSrc * self,
gboolean is_client);

};

GType kms_webrtc_transport_src_get_type (void);
Expand All @@ -67,5 +73,8 @@ void kms_webrtc_transport_src_configure (KmsWebrtcTransportSrc * self,
const char *stream_id,
guint component_id);

void kms_webrtc_transport_src_set_dtls_is_client (KmsWebrtcTransportSrc *src,
gboolean is_client);

G_END_DECLS
#endif /* __KMS_WEBRTC_TRANSPORT_SRC_H__ */
Loading

0 comments on commit 75db50c

Please sign in to comment.