[Asterisk-code-review] fax: Fix crashes in PJSIP re-negotiation scenarios. (asterisk[certified/16.8])

[George Joseph]

George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/14371 )

Change subject: fax: Fix crashes in PJSIP re-negotiation scenarios.
......................................................................

fax: Fix crashes in PJSIP re-negotiation scenarios.

This change fixes a few re-negotiation issues
uncovered with fax.

1. The fax support uses its own mechanism for
re-negotiation by conveying T.38 information in
its own frames. The new support for re-negotiating
when adding/removing/changing streams was also
being triggered for this causing multiple re-INVITEs.
The new support will no longer trigger when
transitioning between fax.

2. In off-nominal re-negotiation cases it was
possible for some state information to be left
over and used by the next re-negotiation. This
is now cleared.

ASTERISK-28811
ASTERISK-28839

Change-Id: I8ed5924b53be9fe06a385c58817e5584b0f25cc2
(cherry picked from commit f3ac42b9bd0d436ce2128e5bbb0502ddb587f98a)
---
M res/res_pjsip_session.c
1 file changed, 15 insertions(+), 4 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 9cc62ce..1bbc6fa 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -1066,11 +1066,14 @@
 	if (topology) {
 		ast_channel_set_stream_topology(session->channel, topology);
 		/* If this is a remotely done renegotiation that has changed the stream topology notify what is
-		 * currently handling this channel.
+		 * currently handling this channel. Note that fax uses its own process, so if we are transitioning
+		 * between audio and fax or vice versa we don't notify.
 		 */
 		if (pjmedia_sdp_neg_was_answer_remote(session->inv_session->neg) == PJ_FALSE &&
 			session->active_media_state && session->active_media_state->topology &&
-			!ast_stream_topology_equal(session->active_media_state->topology, topology)) {
+			!ast_stream_topology_equal(session->active_media_state->topology, topology) &&
+			!session->active_media_state->default_session[AST_MEDIA_TYPE_IMAGE] &&
+			!session->pending_media_state->default_session[AST_MEDIA_TYPE_IMAGE]) {
 			changed = 2;
 		}
 	}
@@ -2046,6 +2049,7 @@
 	pjsip_dialog *dlg;
 	RAII_VAR(struct ast_sip_session *, session, NULL, ao2_cleanup);
 	pjsip_rdata_sdp_info *sdp_info;
+	int deferred;
 
 	if (rdata->msg_info.msg->line.req.method.id != PJSIP_INVITE_METHOD ||
 		!(dlg = pjsip_ua_find_dialog(&rdata->msg_info.cid->id, &rdata->msg_info.to->tag, &rdata->msg_info.from->tag, PJ_FALSE)) ||
@@ -2135,7 +2139,11 @@
 		return PJ_FALSE;
 	}
 
-	if (!sdp_requires_deferral(session, sdp_info->sdp)) {
+	deferred = sdp_requires_deferral(session, sdp_info->sdp);
+	if (deferred == -1) {
+		ast_sip_session_media_state_reset(session->pending_media_state);
+		return PJ_FALSE;
+	} else if (!deferred) {
 		return PJ_FALSE;
 	}
 
@@ -4251,6 +4259,7 @@
 
 	session = inv->mod_data[session_module.id];
 	if (handle_incoming_sdp(session, offer)) {
+		ast_sip_session_media_state_reset(session->pending_media_state);
 		return;
 	}
 
@@ -4329,7 +4338,9 @@
 		return;
 	}
 
-	handle_negotiated_sdp(session, local, remote);
+	if (handle_negotiated_sdp(session, local, remote)) {
+		ast_sip_session_media_state_reset(session->pending_media_state);
+	}
 }
 
 static pjsip_redirect_op session_inv_on_redirected(pjsip_inv_session *inv, const pjsip_uri *target, const pjsip_event *e)

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14371
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: certified/16.8
Gerrit-Change-Id: I8ed5924b53be9fe06a385c58817e5584b0f25cc2
Gerrit-Change-Number: 14371
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200430/26c03cd5/attachment.html>