[Asterisk-code-review] res pjsip session: Add ability to follow media forking duri... (asterisk[13])

George Joseph asteriskteam at digium.com
Mon Jun 18 21:39:22 CDT 2018


George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/9200


Change subject: res_pjsip_session:  Add ability to follow media forking during an INVITE
......................................................................

res_pjsip_session:  Add ability to follow media forking during an INVITE

pjproject by default currently will follow media forked during an INVITE
on outbound calls if the To tag is different on a subsequent response as
that on an earlier response.  We handle this correctly.  There have
been reported cases where the To tag is the same but we still need to
follow the forked media.  The pjproject patch in this commit adds the
capability to sip_inv and also adds the capability to control it at
runtime.  The original "different tag" behavior was always controllable
at runtime but we never did anythign with it and left it to default to
TRUE.

So, along with the pjproject patch, this commit adds options to both the
system and endpoint objects to control the two behaviors, and a small
logic change to session_inv_on_media_update in res_pjsip_session to
control the behavior at the endpoint level.

The default behavior for "different tags" remains the same at TRUE and
the default for "same tag" is FALSE.

Change-Id: I64d071942b79adb2f0a4e13137389b19404fe3d6
Reported-by: Ross Beer
---
M CHANGES
M configs/samples/pjsip.conf.sample
A contrib/ast-db-manage/config/versions/0be05c3a8225_add_early_media_options.py
M include/asterisk/res_pjsip.h
M res/res_pjsip.c
M res/res_pjsip/config_system.c
M res/res_pjsip/pjsip_configuration.c
M res/res_pjsip_session.c
A third-party/pjproject/patches/0100-sip_inv-Add-ability-to-accept-updated-media-on-same-.patch
9 files changed, 313 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/00/9200/1

diff --git a/CHANGES b/CHANGES
index a02cd8a..c830566 100644
--- a/CHANGES
+++ b/CHANGES
@@ -34,6 +34,21 @@
     detection as the ConfbridgeTalking event, so bridges must be configured
     with "talk_detection_events=yes" for this flag to have meaning.
 
+res_pjsip
+------------------
+  * Two new options have been added to the system and endpoint objects to
+    control whether, on outbound calls, Asterisk will follow media forked
+    during the initial INVITE transaction.  For instance, the media port in
+    a 200 OK's SDP is different than that in a previous 183 Session Progress.
+    follow_early_media_forked sets whether Asterisk will follow when the
+    To tag on the subsequent response is different that on the the previous
+    response.  The default behavior of "yes" is unchanged.
+    follow_early_media_forked_same_tag sets whether Asterisk will follow when
+    the To tag on the subsequent response is the same as that on the previous
+    response. The default behavior of "no" is unchanged.
+    These options have to be enabled system-wide in the system config section
+    as well as on individual endpoints that require the functionality.
+
 ------------------------------------------------------------------------------
 --- Functionality changes from Asterisk 13.20.0 to Asterisk 13.21.0 ----------
 ------------------------------------------------------------------------------
diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample
index e172908..2c39dd1 100644
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -790,6 +790,26 @@
                         ; of MWI status changes.  If not set, incoming MWI
                         ; NOTIFYs are ignored.
 
+;follow_early_media_fork = ; Follow SDP forked media when To tag is different.
+                           ; On outgoing calls, if the UAS responds with
+                           ; different SDP attributes on subsequent 18X or 2XX
+                           ; responses (such as a port update) AND the To tag
+                           ; on the subsequent response is different that the
+                           ; previous one, follow it.
+                           ; This option must also be enabled in the system
+                           ; section for it to take effect here.
+                           ; (default: yes)
+;follow_early_media_fork_same_tag =
+                           ; Follow SDP forked media when To tag is the same.
+                           ; On outgoing calls, if the UAS responds with
+                           ; different SDP attributes on subsequent 18X or 2XX
+                           ; responses (such as a port update) AND the To tag on
+                           ; the subsequent response is the same as that the
+                           ; previous one, follow it.
+                           ; This option must also be enabled in the system
+                           ; section for it to take effect here.
+                           ; (default: no)
+
 ;==========================AUTH SECTION OPTIONS=========================
 ;[auth]
 ;  SYNOPSIS: Authentication type
@@ -935,6 +955,25 @@
                         ; Disabling this option has been known to cause interoperability
                         ; issues, so disable at your own risk.
                         ; (default: "yes")
+;follow_early_media_fork = ; Follow SDP forked media when To tag is different.
+                           ; On outgoing calls, if the UAS responds with
+                           ; different SDP attributes on subsequent 18X or 2XX
+                           ; responses (such as a port update) AND the To tag
+                           ; on the subsequent response is different that the
+                           ; previous one, follow it.
+                           ; This option must also be enabled on endpoints that
+                           ; require this functionality.
+                           ; (default: yes)
+;follow_early_media_fork_same_tag =
+                           ; Follow SDP forked media when To tag is the same.
+                           ; On outgoing calls, if the UAS responds with
+                           ; different SDP attributes on subsequent 18X or 2XX
+                           ; responses (such as a port update) AND the To tag on
+                           ; the subsequent response is the same as that the
+                           ; previous one, follow it.
+                           ; This option must also be enabled on endpoints that
+                           ; require this functionality.
+                           ; (default: no)
 ;type=  ; Must be of type system (default: "")
 
 ;==========================GLOBAL SECTION OPTIONS=========================
diff --git a/contrib/ast-db-manage/config/versions/0be05c3a8225_add_early_media_options.py b/contrib/ast-db-manage/config/versions/0be05c3a8225_add_early_media_options.py
new file mode 100644
index 0000000..a2b1fef
--- /dev/null
+++ b/contrib/ast-db-manage/config/versions/0be05c3a8225_add_early_media_options.py
@@ -0,0 +1,31 @@
+"""Add early media options
+
+Revision ID: 0be05c3a8225
+Revises: d3e4284f8707
+Create Date: 2018-06-18 17:26:16.737692
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '0be05c3a8225'
+down_revision = 'd3e4284f8707'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects.postgresql import ENUM
+
+YESNO_NAME = 'yesno_values'
+YESNO_VALUES = ['yes', 'no']
+
+def upgrade():
+    yesno_values = ENUM(*YESNO_VALUES, name=YESNO_NAME, create_type=False)
+
+    op.add_column('ps_systems', sa.Column('follow_early_media_fork', yesno_values))
+    op.add_column('ps_systems', sa.Column('follow_early_media_fork_same_tag', yesno_values))
+
+def downgrade():
+    if op.get_context().bind.dialect.name == 'mssql':
+        op.drop_constraint('ck_ps_systems_follow_early_media_fork_yesno_values','ps_systems')
+        op.drop_constraint('ck_ps_systems_follow_early_media_fork_any_tag_yesno_values','ps_systems')
+    op.drop_column('ps_systems', 'follow_early_media_fork')
+    op.drop_column('ps_systems', 'follow_early_media_fork_same_tag')
diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 7177ceb..7dc790f 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -637,6 +637,10 @@
 	unsigned int timeout;
 	/*! Number of seconds before terminating channel due to lack of RTP (when on hold) */
 	unsigned int timeout_hold;
+	/*! Follow forked media with a different To tag */
+	unsigned int follow_early_media_fork;
+	/*! Follow forked media with the same To tag */
+	unsigned int follow_early_media_fork_same_tag;
 };
 
 /*!
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index b5f8cbd..d0cb5d5 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -1027,6 +1027,34 @@
 						changes.  If not set, incoming MWI NOTIFYs are ignored.
 					</para></description>
 				</configOption>
+				<configOption name="follow_early_media_fork" default="no">
+					<synopsis>Follow SDP forked media when To tag is different</synopsis>
+					<description><para>
+						On outgoing calls, if the UAS responds with different SDP attributes
+						on subsequent 18X or 2XX responses (such as a port update) AND the
+						To tag on the subsequent response is different that the previous one,
+						follow it.
+						</para>
+						<para>
+							This option must also be enabled in the <literal>system</literal>
+							section for it to take effect here.
+						</para>
+					</description>
+				</configOption>
+				<configOption name="follow_early_media_fork_same_tag" default="no">
+					<synopsis>Follow SDP forked media when To tag is the same</synopsis>
+					<description><para>
+						On outgoing calls, if the UAS responds with different SDP attributes
+						on subsequent 18X or 2XX responses (such as a port update) AND the
+						To tag on the subsequent response is the same as that the previous one,
+						follow it.
+						</para>
+						<para>
+							This option must also be enabled in the <literal>system</literal>
+							section for it to take effect here.
+						</para>
+					</description>
+				</configOption>
 			</configObject>
 			<configObject name="auth">
 				<synopsis>Authentication type</synopsis>
@@ -1594,6 +1622,34 @@
 						request is too large.  See RFC 3261 section 18.1.1.
 					</para></description>
 				</configOption>
+				<configOption name="follow_early_media_fork" default="no">
+					<synopsis>Follow SDP forked media when To tag is different</synopsis>
+					<description><para>
+						On outgoing calls, if the UAS responds with different SDP attributes
+						on subsequent 18X or 2XX responses (such as a port update) AND the
+						To tag on the subsequent response is different that the previous one,
+						follow it.
+						</para>
+						<para>
+							This option must also be enabled on endpoints that require
+							this functionality.
+						</para>
+					</description>
+				</configOption>
+				<configOption name="follow_early_media_fork_same_tag" default="no">
+					<synopsis>Follow SDP forked media when To tag is the same</synopsis>
+					<description><para>
+						On outgoing calls, if the UAS responds with different SDP attributes
+						on subsequent 18X or 2XX responses (such as a port update) AND the
+						To tag on the subsequent response is the same as that the previous one,
+						follow it.
+						</para>
+						<para>
+							This option must also be enabled on endpoints that require
+							this functionality.
+						</para>
+					</description>
+				</configOption>
 				<configOption name="type">
 					<synopsis>Must be of type 'system'.</synopsis>
 				</configOption>
diff --git a/res/res_pjsip/config_system.c b/res/res_pjsip/config_system.c
index ed2b5d2..1e4a730 100644
--- a/res/res_pjsip/config_system.c
+++ b/res/res_pjsip/config_system.c
@@ -52,6 +52,13 @@
 	} threadpool;
 	/*! Nonzero to disable switching from UDP to TCP transport */
 	unsigned int disable_tcp_switch;
+	/*!
+	 * Although early media is enabled in pjproject by default, it's only
+	 * enabled when the To tags are different. These options allow turning
+	 * onm or off the feature for different tags and same tags.
+	 */
+	unsigned int follow_early_media_fork;
+	unsigned int follow_early_media_fork_same_tag;
 };
 
 static struct ast_threadpool_options sip_threadpool_options = {
@@ -95,6 +102,9 @@
 
 	pjsip_cfg()->tsx.t1 = system->timert1;
 	pjsip_cfg()->tsx.td = system->timerb;
+
+	pjsip_cfg()->endpt.follow_early_media_fork = system->follow_early_media_fork;
+	pjsip_cfg()->endpt.follow_early_media_fork_same_tag = system->follow_early_media_fork_same_tag;
 
 	if (system->compactheaders) {
 		extern pj_bool_t pjsip_use_compact_form;
@@ -184,6 +194,10 @@
 			OPT_UINT_T, 0, FLDSET(struct system_config, threadpool.max_size));
 	ast_sorcery_object_field_register(system_sorcery, "system", "disable_tcp_switch", "yes",
 			OPT_BOOL_T, 1, FLDSET(struct system_config, disable_tcp_switch));
+	ast_sorcery_object_field_register(system_sorcery, "system", "follow_early_media_fork", "no",
+			OPT_BOOL_T, 1, FLDSET(struct system_config, follow_early_media_fork));
+	ast_sorcery_object_field_register(system_sorcery, "system", "follow_early_media_fork_same_tag", "no",
+			OPT_BOOL_T, 1, FLDSET(struct system_config, follow_early_media_fork_same_tag));
 
 	ast_sorcery_load(system_sorcery);
 
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 9426e92..e8c9d0d 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -1854,6 +1854,8 @@
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "refer_blind_progress", "yes", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, refer_blind_progress));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "notify_early_inuse_ringing", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, notify_early_inuse_ringing));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "incoming_mwi_mailbox", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, incoming_mwi_mailbox));
+	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "follow_early_media_fork", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, media.rtp.follow_early_media_fork));
+	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "follow_early_media_fork_same_tag", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, media.rtp.follow_early_media_fork_same_tag));
 
 	if (ast_sip_initialize_sorcery_transport()) {
 		ast_log(LOG_ERROR, "Failed to register SIP transport support with sorcery\n");
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 26e5c39..590125a 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -3065,6 +3065,26 @@
 		return;
 	}
 
+	if (session->endpoint && (inv->following_fork || inv->following_fork_same_tag)) {
+		int bail = 0;
+		if ((inv->following_fork && session->endpoint->media.rtp.follow_early_media_fork)) {
+			ast_debug(3, "Following early media fork with different To tags\n");
+		} else {
+			ast_debug(3, "Not following early media fork with different To tags\n");
+			bail = 1;
+		}
+		if ((inv->following_fork_same_tag && session->endpoint->media.rtp.follow_early_media_fork_same_tag)) {
+			ast_debug(3, "Following early media fork with same To tag\n");
+			bail = 0;
+		} else {
+			ast_debug(3, "Not following early media fork with same To tag\n");
+			bail = 1;
+		}
+		if (bail) {
+			return;
+		}
+	}
+
 	if ((status != PJ_SUCCESS) || (pjmedia_sdp_neg_get_active_local(inv->neg, &local) != PJ_SUCCESS) ||
 		(pjmedia_sdp_neg_get_active_remote(inv->neg, &remote) != PJ_SUCCESS)) {
 		ast_channel_hangupcause_set(session->channel, AST_CAUSE_BEARERCAPABILITY_NOTAVAIL);
diff --git a/third-party/pjproject/patches/0100-sip_inv-Add-ability-to-accept-updated-media-on-same-.patch b/third-party/pjproject/patches/0100-sip_inv-Add-ability-to-accept-updated-media-on-same-.patch
new file mode 100644
index 0000000..8a75751
--- /dev/null
+++ b/third-party/pjproject/patches/0100-sip_inv-Add-ability-to-accept-updated-media-on-same-.patch
@@ -0,0 +1,132 @@
+From c57874baacd2b41cd42cd93e784cfb96ac9a6012 Mon Sep 17 00:00:00 2001
+From: George Joseph <gjoseph at digium.com>
+Date: Mon, 18 Jun 2018 20:16:54 -0600
+Subject: [PATCH] sip_inv:  Add ability to accept updated media on same To tag
+
+Currently, setting pjsip_cfg()->endpt.follow_early_media_fork allows
+sip_inv to process media updates when the To tag is different.  There
+are some cases where media updates need to be processed when the tags
+are the same.  Since removing the requirement for different tags would
+change default behavior, a new option "follow_early_media_fork_same_tag"
+has been added along with a new pjsip_inv_session flag
+"following_fork_same_tag" to indicate under which condition we're
+following.
+---
+ pjsip/include/pjsip-ua/sip_inv.h |  3 +++
+ pjsip/include/pjsip/sip_config.h | 23 +++++++++++++++++++++++
+ pjsip/src/pjsip-ua/sip_inv.c     | 15 ++++++++-------
+ pjsip/src/pjsip/sip_config.c     |  3 ++-
+ 4 files changed, 36 insertions(+), 8 deletions(-)
+
+diff --git a/pjsip/include/pjsip-ua/sip_inv.h b/pjsip/include/pjsip-ua/sip_inv.h
+index 1bb7b8adc..1509831d8 100644
+--- a/pjsip/include/pjsip-ua/sip_inv.h
++++ b/pjsip/include/pjsip-ua/sip_inv.h
+@@ -442,6 +442,9 @@ struct pjsip_inv_session
+     pj_bool_t		 following_fork;	    /**< Internal, following
+ 							 forked media?	    */
+     pj_atomic_t		*ref_cnt;		    /**< Reference counter. */
++    pj_bool_t            following_fork_same_tag;    /**< Internal, following
++                                                          forked media same
++                                                          tag?              */
+ };
+ 
+ 
+diff --git a/pjsip/include/pjsip/sip_config.h b/pjsip/include/pjsip/sip_config.h
+index 04871c98a..777d73279 100644
+--- a/pjsip/include/pjsip/sip_config.h
++++ b/pjsip/include/pjsip/sip_config.h
+@@ -170,6 +170,14 @@ typedef struct pjsip_cfg_t
+ 	 */
+ 	pj_bool_t use_compact_form;
+ 
++        /**
++         * Enable call media session to always be updated to the latest
++         * received early media SDP when receiving forked early media
++         * (multiple 183 responses with SAME To tag).
++         *
++         * Default is PJSIP_FOLLOW_EARLY_MEDIA_FORK_SAME_TAG.
++         */
++        pj_bool_t follow_early_media_fork_same_tag;
+     } endpt;
+ 
+     /** Transaction layer settings. */
+@@ -415,6 +423,21 @@ PJ_INLINE(pjsip_cfg_t*) pjsip_cfg(void)
+ #endif
+ 
+ 
++/**
++ * Specify whether the call media session should be updated to the latest
++ * received early media SDP when receiving forked early media (multiple 183
++ * responses with the SAME To tag).
++ *
++ * This option can also be controlled at run-time by the
++ * \a follow_early_media_fork_same_tag setting in pjsip_cfg_t.
++ *
++ * Default is PJ_FALSE.
++ */
++#ifndef PJSIP_FOLLOW_EARLY_MEDIA_FORK_SAME_TAG
++#   define PJSIP_FOLLOW_EARLY_MEDIA_FORK_SAME_TAG        PJ_FALSE
++#endif
++
++
+ /**
+  * Specify whether "alias" param should be added to the Via header
+  * in any outgoing request with connection oriented transport.
+diff --git a/pjsip/src/pjsip-ua/sip_inv.c b/pjsip/src/pjsip-ua/sip_inv.c
+index 0173cb4cb..332147a62 100644
+--- a/pjsip/src/pjsip-ua/sip_inv.c
++++ b/pjsip/src/pjsip-ua/sip_inv.c
+@@ -2000,6 +2000,7 @@ static pj_status_t inv_check_sdp_in_incoming_msg( pjsip_inv_session *inv,
+ 
+     /* Initialize info that we are following forked media */
+     inv->following_fork = PJ_FALSE;
++    inv->following_fork_same_tag = PJ_FALSE;
+ 
+     /* MUST NOT do multiple SDP offer/answer in a single transaction,
+      * EXCEPT if:
+@@ -2025,12 +2026,11 @@ static pj_status_t inv_check_sdp_in_incoming_msg( pjsip_inv_session *inv,
+ 	 * tag.
+          * See ticket #1644 and #1764 for forked early media case.
+ 	 */
+-	if (tsx->role == PJSIP_ROLE_UAC &&
+-	    (st_code/100 == 2 ||
+-	     (st_code/10 == 18 /* st_code == 18x */
+-              && pjsip_cfg()->endpt.follow_early_media_fork)) &&
+-	    tsx_inv_data->done_early &&
+-	    pj_stricmp(&tsx_inv_data->done_tag, &res_tag))
++	if (tsx->role == PJSIP_ROLE_UAC && tsx_inv_data->done_early &&
++	    (st_code/100 == 2 || st_code/10 == 18) && /* st_code == 18x */
++              pjsip_cfg()->endpt.follow_early_media_fork &&
++            (pj_stricmp(&tsx_inv_data->done_tag, &res_tag) ||
++                pjsip_cfg()->endpt.follow_early_media_fork_same_tag))
+ 	{
+ 	    const pjmedia_sdp_session *reoffer_sdp = NULL;
+ 
+@@ -2054,7 +2054,8 @@ static pj_status_t inv_check_sdp_in_incoming_msg( pjsip_inv_session *inv,
+ 		return status;
+ 	    }
+ 
+-	    inv->following_fork = PJ_TRUE;
++	    inv->following_fork = !!pj_stricmp(&tsx_inv_data->done_tag, &res_tag);
++	    inv->following_fork_same_tag = !pj_stricmp(&tsx_inv_data->done_tag, &res_tag);
+ 
+ 	} else {
+ 
+diff --git a/pjsip/src/pjsip/sip_config.c b/pjsip/src/pjsip/sip_config.c
+index fdeec2f6b..e29c341c5 100644
+--- a/pjsip/src/pjsip/sip_config.c
++++ b/pjsip/src/pjsip/sip_config.c
+@@ -35,7 +35,8 @@ pjsip_cfg_t pjsip_sip_cfg_var =
+        PJSIP_REQ_HAS_VIA_ALIAS,
+        PJSIP_RESOLVE_HOSTNAME_TO_GET_INTERFACE,
+        0,
+-       PJSIP_ENCODE_SHORT_HNAME
++       PJSIP_ENCODE_SHORT_HNAME,
++       PJSIP_FOLLOW_EARLY_MEDIA_FORK_SAME_TAG
+     },
+ 
+     /* Transaction settings */
+-- 
+2.14.4
+

-- 
To view, visit https://gerrit.asterisk.org/9200
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: I64d071942b79adb2f0a4e13137389b19404fe3d6
Gerrit-Change-Number: 9200
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180618/ea91c4db/attachment-0001.html>


More information about the asterisk-code-review mailing list