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