[Asterisk-code-review] chan pjsip: Fix ability to send UPDATE on COLP (asterisk[14])

George Joseph asteriskteam at digium.com
Wed Jul 5 14:11:48 CDT 2017


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/5931 )

Change subject: chan_pjsip:  Fix ability to send UPDATE on COLP
......................................................................

chan_pjsip:  Fix ability to send UPDATE on COLP

When connected_line_method is "invite", we're supposed to determine
if the client can support UPDATE and if it can, send UPDATE instead
of INVITE to avoid the SDP renegotiation.  Not only was pjproject
not setting the PJSIP_INV_SUPPORT_UPDATE flag, we were testing
that invite_tsx wasn't NULL which isn't always the case.

* Updated chan_pjsip/update_connected_line_information to drop the
  requirement that invite_tsx isn't NULL.
* Submitted patch to pjproject sip_inv.c that sets the
  PJSIP_INV_SUPPORT_UPDATE flag correctly.
* Updated pjsip.conf.sample to clarify what happens when "invite"
  is specified.

ASTERISK-27095

Change-Id: Ic2381b3567b8052c616d96fbe79564c530e81560
---
M channels/chan_pjsip.c
M configs/samples/pjsip.conf.sample
M res/res_pjsip.c
A third-party/pjproject/patches/0070-Set-PJSIP_INV_SUPPORT_UPDATE-correctly-in-pjsip_inv_.patch
4 files changed, 48 insertions(+), 6 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  George Joseph: Approved for Submit



diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index f565ea8..abf3507 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -1272,8 +1272,7 @@
 			int generate_new_sdp;
 
 			method = session->endpoint->id.refresh_method;
-			if (session->inv_session->invite_tsx
-				&& (session->inv_session->options & PJSIP_INV_SUPPORT_UPDATE)) {
+			if (session->inv_session->options & PJSIP_INV_SUPPORT_UPDATE) {
 				method = AST_SIP_SESSION_REFRESH_METHOD_UPDATE;
 			}
 
diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample
index f1bb87d..6510fcf 100644
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -608,8 +608,15 @@
 ;direct_media_glare_mitigation=none     ; Mitigation of direct media re INVITE
                                         ; glare (default: "none")
 ;direct_media_method=invite     ; Direct Media method type (default: "invite")
-;connected_line_method=invite   ; Connected line method type (default:
-                                ; "invite")
+;connected_line_method=invite   ; Connected line method type.
+                                ; When set to "invite", check the remote's
+                                ; Allow header and if UPDATE is allowed, send
+                                ; UPDATE instead of INVITE to avoid SDP
+                                ; renegotiation.  If UPDATE is not Allowed,
+                                ; send INVITE.
+                                ; If set to "update", send UPDATE regardless
+                                ; of what the remote Allows.
+                                ; (default: "invite")
 ;direct_media=yes       ; Determines whether media may flow directly between
                         ; endpoints (default: "yes")
 ;disable_direct_media_on_nat=no ; Disable direct media session refreshes when
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index d2e4a67..5e4ac86 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -193,11 +193,18 @@
 					<description>
 						<para>Method used when updating connected line information.</para>
 						<enumlist>
-							<enum name="invite" />
+							<enum name="invite">
+							<para>When set to <literal>invite</literal>, check the remote's Allow header and
+							if UPDATE is allowed, send UPDATE instead of INVITE to avoid SDP
+							renegotiation.  If UPDATE is not Allowed, send INVITE.</para>
+							</enum>
 							<enum name="reinvite">
 								<para>Alias for the <literal>invite</literal> value.</para>
 							</enum>
-							<enum name="update" />
+							<enum name="update">
+							<para>If set to <literal>update</literal>, send UPDATE regardless of what the remote
+							Allows. </para>
+							</enum>
 						</enumlist>
 					</description>
 				</configOption>
diff --git a/third-party/pjproject/patches/0070-Set-PJSIP_INV_SUPPORT_UPDATE-correctly-in-pjsip_inv_.patch b/third-party/pjproject/patches/0070-Set-PJSIP_INV_SUPPORT_UPDATE-correctly-in-pjsip_inv_.patch
new file mode 100644
index 0000000..9238e3e
--- /dev/null
+++ b/third-party/pjproject/patches/0070-Set-PJSIP_INV_SUPPORT_UPDATE-correctly-in-pjsip_inv_.patch
@@ -0,0 +1,29 @@
+From 1193681959816effa121c4470748d5faa3a59272 Mon Sep 17 00:00:00 2001
+From: George Joseph <gjoseph at digium.com>
+Date: Thu, 29 Jun 2017 13:42:10 -0600
+Subject: [PATCH] Set PJSIP_INV_SUPPORT_UPDATE correctly in
+ pjsip_inv_verify_request3
+
+pjsip_inv_verify_request3 was setting rem_options when UPDATE was
+detected in the Allow header.  That's just an internal variable and
+doesn't go anywhere.  It's '*options' that needs to be set.
+---
+ pjsip/src/pjsip-ua/sip_inv.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/pjsip/src/pjsip-ua/sip_inv.c b/pjsip/src/pjsip-ua/sip_inv.c
+index fbc8ebe..6db7e6b 100644
+--- a/pjsip/src/pjsip-ua/sip_inv.c
++++ b/pjsip/src/pjsip-ua/sip_inv.c
+@@ -1237,7 +1237,7 @@ PJ_DEF(pj_status_t) pjsip_inv_verify_request3(pjsip_rx_data *rdata,
+ 
+ 	if (i != allow->count) {
+ 	    /* UPDATE is present in Allow */
+-	    rem_option |= PJSIP_INV_SUPPORT_UPDATE;
++	    *options |= PJSIP_INV_SUPPORT_UPDATE;
+ 	}
+ 
+     }
+-- 
+2.9.4
+

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

Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic2381b3567b8052c616d96fbe79564c530e81560
Gerrit-Change-Number: 5931
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170705/3d130be6/attachment.html>


More information about the asterisk-code-review mailing list