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

diff --git a/configure b/configure
index f26d03cd68a81b4a9706a4a490a12c7ba0463fdc..256935fb0063dff81decff3a43ab0d66e04b7b2d 100755 (executable)
--- a/configure
+++ b/configure
@@ -9468,6 +9468,9 @@ $as_echo "#define HAVE_PJSIP_INV_ACCEPT_MULTIPLE_SDP_ANSWERS 1" >>confdefs.h
 $as_echo "#define HAVE_PJSIP_ENDPOINT_COMPACT_FORM 1" >>confdefs.h
 
 
+$as_echo "#define HAVE_PJPROJECT_ON_VALID_ICE_PAIR_CALLBACK 1" >>confdefs.h
+
+
 
 
 
@@ -16583,8 +16586,6 @@ main ()
     if (*(data + i) != *(data3 + i))
       return 14;
   close (fd);
-  free (data);
-  free (data3);
   return 0;
 }
 _ACEOF
@@ -25695,6 +25696,43 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
        CPPFLAGS="${saved_cppflags}"
     fi
 
+      { $as_echo "$as_me:${as_lineno-$LINENO}: checking for pjproject on_valid_pair callback" >&5
+$as_echo_n "checking for pjproject on_valid_pair callback... " >&6; }
+      cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <pjsip.h>
+            #include <pjsip_ua.h>
+            #include <pjnath.h>
+            void on_valid_pair(pj_ice_sess *ice) {}
+            void on_ice_complete(pj_ice_sess *ice, pj_status_t status) {}
+            void on_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) {}
+            pj_status_t on_tx_pkt(pj_ice_sess *ice, unsigned comp_id, unsigned transport_id, const void *pkt, pj_size_t size, const pj_sockaddr_t *dst_addr, unsigned dst_addr_len) {}
+int
+main ()
+{
+pj_ice_sess_cb ice_sess_cb = {
+               .on_valid_pair = on_valid_pair,
+               .on_ice_complete = on_ice_complete,
+               .on_rx_data = on_rx_data,
+               .on_tx_pkt = on_tx_pkt,
+            };
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+
+$as_echo "#define HAVE_PJPROJECT_ON_VALID_ICE_PAIR_CALLBACK 1" >>confdefs.h
+
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
       LIBS="${saved_libs}"
       CPPFLAGS="${saved_cppflags}"
 

diff --git a/configure.ac b/configure.ac
index ad8a679fb30127bb83211c2ec34f7a344d10a5ff..7c6f510b538cd374fac054e68371c51bc241de65 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -2405,6 +2405,26 @@ if test "$USE_PJPROJECT" != "no" ; then
       AST_C_COMPILE_CHECK([PJSIP_TLS_TRANSPORT_PROTO], [struct pjsip_tls_setting setting; int proto; proto = setting.proto;], [pjsip.h])
       AST_C_COMPILE_CHECK([PJSIP_INV_ACCEPT_MULTIPLE_SDP_ANSWERS], [pjsip_cfg()->endpt.accept_multiple_sdp_answers = 0;], [pjsip.h])
       AST_C_COMPILE_CHECK([PJSIP_ENDPOINT_COMPACT_FORM], [pjsip_cfg()->endpt.use_compact_form = PJ_TRUE;], [pjsip.h])
+      AC_MSG_CHECKING(for pjproject on_valid_pair callback)
+      AC_LINK_IFELSE(
+         [AC_LANG_PROGRAM(
+            [#include <pjsip.h>
+            #include <pjsip_ua.h>
+            #include <pjnath.h>
+            void on_valid_pair(pj_ice_sess *ice) {}
+            void on_ice_complete(pj_ice_sess *ice, pj_status_t status) {}
+            void on_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) {}
+            pj_status_t on_tx_pkt(pj_ice_sess *ice, unsigned comp_id, unsigned transport_id, const void *pkt, pj_size_t size, const pj_sockaddr_t *dst_addr, unsigned dst_addr_len) {}],
+            [pj_ice_sess_cb ice_sess_cb = {
+               .on_valid_pair = on_valid_pair,
+               .on_ice_complete = on_ice_complete,
+               .on_rx_data = on_rx_data,
+               .on_tx_pkt = on_tx_pkt,
+            };])],
+         AC_MSG_RESULT(yes)
+         AC_DEFINE(HAVE_PJPROJECT_ON_VALID_ICE_PAIR_CALLBACK, 1, [Define to 1 if on_valid_pair callback is present.]),
+         AC_MSG_RESULT(no)
+      )
       LIBS="${saved_libs}"
       CPPFLAGS="${saved_cppflags}"
 

diff --git a/include/asterisk/autoconfig.h.in b/include/asterisk/autoconfig.h.in
index 5cf7a10d508ca770e07a5ae99fbfbb52336a113d..6816c5b39d860335394c1f9da3c1c370718c6366 100644 (file)
--- a/include/asterisk/autoconfig.h.in
+++ b/include/asterisk/autoconfig.h.in
@@ -606,6 +606,9 @@
 /* Define if your system has PJPROJECT_BUNDLED */
 #undef HAVE_PJPROJECT_BUNDLED
 
+/* Define to 1 if on_valid_pair callback is present. */
+#undef HAVE_PJPROJECT_ON_VALID_ICE_PAIR_CALLBACK
+
 /* Define to 1 if PJPROJECT has the pjsip_auth_clt_deinit support feature. */
 #undef HAVE_PJSIP_AUTH_CLT_DEINIT
 

diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 8154c2cac4a037c2d71ef917cf8f346f29fb1406..e5bb94e717c0ffed65035c71a9e124caae252d50 100644 (file)
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -390,6 +390,7 @@ struct ast_rtp {
        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)
@@ -897,6 +898,8 @@ static int ice_reset_session(struct ast_rtp_instance *instance)
                }
        }
 
+       rtp->ice_media_started = 0;
+
        return res;
 }
 
@@ -2149,13 +2152,14 @@ static void dtls_perform_handshake(struct ast_rtp_instance *instance, struct dtl
 #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 (status == PJ_SUCCESS) {
                struct ast_sockaddr remote_address;
 
@@ -2174,6 +2178,12 @@ static void ast_rtp_on_ice_complete(pj_ice_sess *ice, pj_status_t status)
        }
 
 #if defined(HAVE_OPENSSL) && (OPENSSL_VERSION_NUMBER >= 0x10001000L) && !defined(OPENSSL_NO_SRTP)
+       /* If we've already started media, no need to do all of this again */
+       if (rtp->ice_media_started) {
+               ao2_unlock(instance);
+               return;
+       }
+
        dtls_perform_handshake(instance, &rtp->dtls, 0);
 
        if (rtp->rtcp && rtp->rtcp->type == AST_RTP_INSTANCE_RTCP_STANDARD) {
@@ -2181,6 +2191,8 @@ static void ast_rtp_on_ice_complete(pj_ice_sess *ice, pj_status_t status)
        }
 #endif
 
+       rtp->ice_media_started = 1;
+
        if (!strictrtp) {
                ao2_unlock(instance);
                return;
@@ -2191,6 +2203,20 @@ static void ast_rtp_on_ice_complete(pj_ice_sess *ice, pj_status_t status)
        ao2_unlock(instance);
 }
 
+#ifdef HAVE_PJPROJECT_ON_VALID_ICE_PAIR_CALLBACK
+/* 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 +2273,9 @@ static pj_status_t ast_rtp_on_ice_tx_pkt(pj_ice_sess *ice, unsigned comp_id, uns
 
 /* ICE Session interface declaration */
 static pj_ice_sess_cb ast_rtp_ice_sess_cb = {
+#ifdef HAVE_PJPROJECT_ON_VALID_ICE_PAIR_CALLBACK
+       .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,

diff --git a/third-party/pjproject/configure.m4 b/third-party/pjproject/configure.m4
index 9175a1177eb56a06b8737c6fc1065a56e18a29aa..32cf56dd546c1252f5609a1967551dbfd8e9b4d5 100644 (file)
--- a/third-party/pjproject/configure.m4
+++ b/third-party/pjproject/configure.m4
@@ -106,6 +106,7 @@ AC_DEFUN([_PJPROJECT_CONFIGURE],
        AC_DEFINE([HAVE_PJSIP_TSX_LAYER_FIND_TSX2], 1, [Define if your system has pjsip_tsx_layer_find_tsx2 declared.])
        AC_DEFINE([HAVE_PJSIP_INV_ACCEPT_MULTIPLE_SDP_ANSWERS], 1, [Define if your system has HAVE_PJSIP_INV_ACCEPT_MULTIPLE_SDP_ANSWERS declared.])
        AC_DEFINE([HAVE_PJSIP_ENDPOINT_COMPACT_FORM], 1, [Define if your system has HAVE_PJSIP_ENDPOINT_COMPACT_FORM declared.])
+       AC_DEFINE([HAVE_PJPROJECT_ON_VALID_ICE_PAIR_CALLBACK], 1, [Define if your system has the on_valid_pair pjnath callback.])
 
        AC_SUBST([PJPROJECT_BUNDLED])
        AC_SUBST([PJPROJECT_DIR])

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 (file)
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@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
+