[Asterisk-code-review] RTP/ICE: Send on first valid pair. (asterisk[13])
Benjamin Keith Ford
asteriskteam at digium.com
Wed Feb 5 11:47:34 CST 2020
Benjamin Keith Ford has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/13731 )
Change subject: RTP/ICE: Send on first valid pair.
......................................................................
RTP/ICE: Send on first valid pair.
When handling ICE negotiations, it's possible that there can be a delay
between STUN binding requests which in turn will cause a delay in ICE
completion, preventing media from flowing. It should be possible to send
media when there is at least one valid pair, preventing this scenario
from occurring.
A change was added to PJPROJECT that adds an optional callback
(on_valid_pair) that will be called when the first valid pair is found
during ICE negotiation. Asterisk uses this to start the DTLS handshake,
allowing media to flow. It will only be called once, either on the first
valid pair, or when ICE negotiation is complete.
ASTERISK-28716
Change-Id: Ia7b68c34f06d2a1d91c5ed51627b66fd0363d867
---
M res/res_rtp_asterisk.c
A third-party/pjproject/patches/0040-ICE-Add-callback-for-finding-valid-pair.patch
2 files changed, 120 insertions(+), 2 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/31/13731/1
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 8154c2c..164a4ac 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -390,6 +390,7 @@
struct ao2_container *ice_proposed_remote_candidates; /*!< Incoming remote ICE candidates for new session */
struct ast_sockaddr ice_original_rtp_addr; /*!< rtp address that ICE started on first session */
unsigned int ice_num_components; /*!< The number of ICE components */
+ unsigned int ice_media_started:1; /*!< ICE media has started, either on a valid pair or on ICE completion */
#endif
#if defined(HAVE_OPENSSL) && (OPENSSL_VERSION_NUMBER >= 0x10001000L) && !defined(OPENSSL_NO_SRTP)
@@ -2149,13 +2150,22 @@
#ifdef HAVE_PJPROJECT
static void rtp_learning_start(struct ast_rtp *rtp);
-/* PJPROJECT ICE callback */
-static void ast_rtp_on_ice_complete(pj_ice_sess *ice, pj_status_t status)
+/* Handles start of media during ICE negotiation or completion */
+static void ast_rtp_ice_start_media(pj_ice_sess *ice, pj_status_t status)
{
struct ast_rtp_instance *instance = ice->user_data;
struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
ao2_lock(instance);
+
+ /* If we've already started media, no need to do all of this again */
+ if (rtp->ice_media_started) {
+ ao2_unlock(instance);
+ return;
+ }
+
+ rtp->ice_media_started = 1;
+
if (status == PJ_SUCCESS) {
struct ast_sockaddr remote_address;
@@ -2191,6 +2201,20 @@
ao2_unlock(instance);
}
+#ifdef HAVE_PJPROJECT_BUNDLED
+/* PJPROJECT ICE optional callback */
+static void ast_rtp_on_valid_pair(pj_ice_sess *ice)
+{
+ ast_rtp_ice_start_media(ice, PJ_SUCCESS);
+}
+#endif
+
+/* PJPROJECT ICE callback */
+static void ast_rtp_on_ice_complete(pj_ice_sess *ice, pj_status_t status)
+{
+ ast_rtp_ice_start_media(ice, status);
+}
+
/* PJPROJECT ICE callback */
static void ast_rtp_on_ice_rx_data(pj_ice_sess *ice, unsigned comp_id, unsigned transport_id, void *pkt, pj_size_t size, const pj_sockaddr_t *src_addr, unsigned src_addr_len)
{
@@ -2247,6 +2271,9 @@
/* ICE Session interface declaration */
static pj_ice_sess_cb ast_rtp_ice_sess_cb = {
+#ifdef HAVE_PJPROJECT_BUNDLED
+ .on_valid_pair = ast_rtp_on_valid_pair,
+#endif
.on_ice_complete = ast_rtp_on_ice_complete,
.on_rx_data = ast_rtp_on_ice_rx_data,
.on_tx_pkt = ast_rtp_on_ice_tx_pkt,
@@ -3245,6 +3272,13 @@
rtp->ssrc = ast_random();
rtp->seqno = ast_random() & 0x7fff;
rtp->strict_rtp_state = (strictrtp ? STRICT_RTP_CLOSED : STRICT_RTP_OPEN);
+#ifdef HAVE_PJPROJECT
+ /* Keep track of whether or not media has started during ICE negotiation.
+ * Usually media will start to flow when ICE negotiation is complete. However,
+ * media can begin to flow once a valid pair has been found.
+ */
+ rtp->ice_media_started = 0;
+#endif
/* Create a new socket for us to listen on and use */
if ((rtp->s =
diff --git a/third-party/pjproject/patches/0040-ICE-Add-callback-for-finding-valid-pair.patch b/third-party/pjproject/patches/0040-ICE-Add-callback-for-finding-valid-pair.patch
new file mode 100644
index 0000000..062e75e
--- /dev/null
+++ b/third-party/pjproject/patches/0040-ICE-Add-callback-for-finding-valid-pair.patch
@@ -0,0 +1,84 @@
+From 8b8199180766e3eab6014feaa64ccaedcdc12816 Mon Sep 17 00:00:00 2001
+From: Ben Ford <bford at digium.com>
+Date: Mon, 23 Dec 2019 11:11:13 -0600
+Subject: [PATCH] ICE: Add callback for finding valid pair.
+
+It's possible to start sending as soon as one valid pair is found during
+ICE negotiation. The reason we would want to do this is because it is
+possible for a delay to occur at the start of a call for up to 3 seconds
+until ICE negotiation has actually completed. More information can be
+found here:
+https://bugs.chromium.org/p/chromium/issues/detail?id=1024096
+
+This patch adds a callback once a valid pair is found that applications
+can use to start sending to avoid this scenario. Since only one valid
+pair is needed to start media, we only trigger the callback once.
+---
+ pjnath/include/pjnath/ice_session.h | 9 +++++++++
+ pjnath/src/pjnath/ice_session.c | 16 ++++++++++++++++
+ 2 files changed, 25 insertions(+)
+
+diff --git a/pjnath/include/pjnath/ice_session.h b/pjnath/include/pjnath/ice_session.h
+index 15f0d04..8971220 100644
+--- a/pjnath/include/pjnath/ice_session.h
++++ b/pjnath/include/pjnath/ice_session.h
+@@ -468,6 +468,14 @@ typedef struct pj_ice_sess_cb
+ {
+ /**
+ * An optional callback that will be called by the ICE session when
++ * a valid pair has been found during ICE negotiation.
++ *
++ * @param ice The ICE session.
++ */
++ void (*on_valid_pair)(pj_ice_sess *ice);
++
++ /**
++ * An optional callback that will be called by the ICE session when
+ * ICE negotiation has completed, successfully or with failure.
+ *
+ * @param ice The ICE session.
+@@ -625,6 +633,7 @@ struct pj_ice_sess
+ pj_bool_t is_nominating; /**< Nominating stage */
+ pj_bool_t is_complete; /**< Complete? */
+ pj_bool_t is_destroying; /**< Destroy is called */
++ pj_bool_t valid_pair_found; /**< First pair found */
+ pj_status_t ice_status; /**< Error status. */
+ pj_timer_entry timer; /**< ICE timer. */
+ pj_ice_sess_cb cb; /**< Callback. */
+diff --git a/pjnath/src/pjnath/ice_session.c b/pjnath/src/pjnath/ice_session.c
+index c51dba7..ed4138a 100644
+--- a/pjnath/src/pjnath/ice_session.c
++++ b/pjnath/src/pjnath/ice_session.c
+@@ -418,6 +418,8 @@ PJ_DEF(pj_status_t) pj_ice_sess_create(pj_stun_config *stun_cfg,
+
+ pj_list_init(&ice->early_check);
+
++ ice->valid_pair_found = PJ_FALSE;
++
+ /* Done */
+ *p_ice = ice;
+
+@@ -1348,6 +1350,20 @@ static pj_bool_t on_check_complete(pj_ice_sess *ice,
+ GET_CHECK_ID(&ice->clist, check),
+ (check->nominated ? " and nominated" : "")));
+
++ {
++ /* On the first valid pair, we call the callback, if present */
++ if (ice->valid_pair_found == PJ_FALSE) {
++ void (*on_valid_pair)(pj_ice_sess *ice);
++
++ ice->valid_pair_found = PJ_TRUE;
++ on_valid_pair = ice->cb.on_valid_pair;
++
++ if (on_valid_pair) {
++ (*on_valid_pair)(ice);
++ }
++ }
++ }
++
+ }
+
+ /* 8.2. Updating States
+--
+2.7.4
+
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13731
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Ia7b68c34f06d2a1d91c5ed51627b66fd0363d867
Gerrit-Change-Number: 13731
Gerrit-PatchSet: 1
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200205/a4f1687b/attachment-0001.html>
More information about the asterisk-code-review
mailing list