<p>Joshua Colp has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14274">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">fax: Fix crashes in PJSIP re-negotiation scenarios.<br><br>This change fixes a few re-negotiation issues<br>uncovered with fax.<br><br>1. The fax support uses its own mechanism for<br>re-negotiation by conveying T.38 information in<br>its own frames. The new support for re-negotiating<br>when adding/removing/changing streams was also<br>being triggered for this causing multiple re-INVITEs.<br>The new support will no longer trigger when<br>transitioning between fax.<br><br>2. In off-nominal re-negotiation cases it was<br>possible for some state information to be left<br>over and used by the next re-negotiation. This<br>is now cleared.<br><br>3. Both bridge_simple and bridge_native_rtp were<br>modifying an immutable format capabilities instead<br>of creating a new one and replacing the existing.<br><br>ASTERISK-28811<br>ASTERISK-28839<br><br>Change-Id: I8ed5924b53be9fe06a385c58817e5584b0f25cc2<br>---<br>M bridges/bridge_native_rtp.c<br>M bridges/bridge_simple.c<br>M res/res_pjsip_session.c<br>3 files changed, 49 insertions(+), 10 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/74/14274/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/bridges/bridge_native_rtp.c b/bridges/bridge_native_rtp.c</span><br><span>index a6addf2..b998e34 100644</span><br><span>--- a/bridges/bridge_native_rtp.c</span><br><span>+++ b/bridges/bridge_native_rtp.c</span><br><span>@@ -876,7 +876,8 @@</span><br><span> stream = ast_stream_topology_get_stream(existing_topology, i);</span><br><span> </span><br><span> if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||</span><br><span style="color: hsl(0, 100%, 40%);">- ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED ||</span><br><span style="color: hsl(120, 100%, 40%);">+ !ast_stream_get_formats(stream)) {</span><br><span> continue;</span><br><span> }</span><br><span> </span><br><span>@@ -886,6 +887,8 @@</span><br><span> </span><br><span> if (audio_formats) {</span><br><span> for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_format_cap *joint;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> stream = ast_stream_topology_get_stream(new_topology, i);</span><br><span> </span><br><span> if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||</span><br><span>@@ -893,8 +896,19 @@</span><br><span> continue;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,</span><br><span style="color: hsl(0, 100%, 40%);">- AST_MEDIA_TYPE_AUDIO);</span><br><span style="color: hsl(120, 100%, 40%);">+ joint = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!joint) {</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</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%);">+ if (ast_stream_get_formats(stream)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_format_cap_append_from_cap(joint, ast_stream_get_formats(stream),</span><br><span style="color: hsl(120, 100%, 40%);">+ AST_MEDIA_TYPE_AUDIO);</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%);">+ ast_format_cap_append_from_cap(joint, audio_formats, AST_MEDIA_TYPE_AUDIO);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_stream_set_formats(stream, joint);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(joint, -1);</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>diff --git a/bridges/bridge_simple.c b/bridges/bridge_simple.c</span><br><span>index 545b3ad..7d5c801 100644</span><br><span>--- a/bridges/bridge_simple.c</span><br><span>+++ b/bridges/bridge_simple.c</span><br><span>@@ -80,7 +80,8 @@</span><br><span> stream = ast_stream_topology_get_stream(existing_topology, i);</span><br><span> </span><br><span> if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||</span><br><span style="color: hsl(0, 100%, 40%);">- ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED ||</span><br><span style="color: hsl(120, 100%, 40%);">+ !ast_stream_get_formats(stream)) {</span><br><span> continue;</span><br><span> }</span><br><span> </span><br><span>@@ -90,6 +91,8 @@</span><br><span> </span><br><span> if (audio_formats) {</span><br><span> for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_format_cap *joint;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> stream = ast_stream_topology_get_stream(new_topology, i);</span><br><span> </span><br><span> if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||</span><br><span>@@ -97,8 +100,19 @@</span><br><span> continue;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,</span><br><span style="color: hsl(0, 100%, 40%);">- AST_MEDIA_TYPE_AUDIO);</span><br><span style="color: hsl(120, 100%, 40%);">+ joint = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!joint) {</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</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%);">+ if (ast_stream_get_formats(stream)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_format_cap_append_from_cap(joint, ast_stream_get_formats(stream),</span><br><span style="color: hsl(120, 100%, 40%);">+ AST_MEDIA_TYPE_AUDIO);</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%);">+ ast_format_cap_append_from_cap(joint, audio_formats, AST_MEDIA_TYPE_AUDIO);</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_stream_set_formats(stream, joint);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(joint, -1);</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c</span><br><span>index 83ba6f5..502d7c8 100644</span><br><span>--- a/res/res_pjsip_session.c</span><br><span>+++ b/res/res_pjsip_session.c</span><br><span>@@ -1067,11 +1067,14 @@</span><br><span> if (topology) {</span><br><span> ast_channel_set_stream_topology(session->channel, topology);</span><br><span> /* If this is a remotely done renegotiation that has changed the stream topology notify what is</span><br><span style="color: hsl(0, 100%, 40%);">- * currently handling this channel.</span><br><span style="color: hsl(120, 100%, 40%);">+ * currently handling this channel. Note that fax uses its own process, so if we are transitioning</span><br><span style="color: hsl(120, 100%, 40%);">+ * between audio and fax or vice versa we don't notify.</span><br><span> */</span><br><span> if (pjmedia_sdp_neg_was_answer_remote(session->inv_session->neg) == PJ_FALSE &&</span><br><span> session->active_media_state && session->active_media_state->topology &&</span><br><span style="color: hsl(0, 100%, 40%);">- !ast_stream_topology_equal(session->active_media_state->topology, topology)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ !ast_stream_topology_equal(session->active_media_state->topology, topology) &&</span><br><span style="color: hsl(120, 100%, 40%);">+ !session->active_media_state->default_session[AST_MEDIA_TYPE_IMAGE] &&</span><br><span style="color: hsl(120, 100%, 40%);">+ !session->pending_media_state->default_session[AST_MEDIA_TYPE_IMAGE]) {</span><br><span> changed = 2;</span><br><span> }</span><br><span> }</span><br><span>@@ -2047,6 +2050,7 @@</span><br><span> pjsip_dialog *dlg;</span><br><span> RAII_VAR(struct ast_sip_session *, session, NULL, ao2_cleanup);</span><br><span> pjsip_rdata_sdp_info *sdp_info;</span><br><span style="color: hsl(120, 100%, 40%);">+ int deferred;</span><br><span> </span><br><span> if (rdata->msg_info.msg->line.req.method.id != PJSIP_INVITE_METHOD ||</span><br><span> !(dlg = pjsip_ua_find_dialog(&rdata->msg_info.cid->id, &rdata->msg_info.to->tag, &rdata->msg_info.from->tag, PJ_FALSE)) ||</span><br><span>@@ -2136,7 +2140,11 @@</span><br><span> return PJ_FALSE;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!sdp_requires_deferral(session, sdp_info->sdp)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ deferred = sdp_requires_deferral(session, sdp_info->sdp);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (deferred == -1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_sip_session_media_state_reset(session->pending_media_state);</span><br><span style="color: hsl(120, 100%, 40%);">+ return PJ_FALSE;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (!deferred) {</span><br><span> return PJ_FALSE;</span><br><span> }</span><br><span> </span><br><span>@@ -4334,6 +4342,7 @@</span><br><span> </span><br><span> session = inv->mod_data[session_module.id];</span><br><span> if (handle_incoming_sdp(session, offer)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_sip_session_media_state_reset(session->pending_media_state);</span><br><span> return;</span><br><span> }</span><br><span> </span><br><span>@@ -4412,7 +4421,9 @@</span><br><span> return;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- handle_negotiated_sdp(session, local, remote);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (handle_negotiated_sdp(session, local, remote)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_sip_session_media_state_reset(session->pending_media_state);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> static pjsip_redirect_op session_inv_on_redirected(pjsip_inv_session *inv, const pjsip_uri *target, const pjsip_event *e)</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/14274">change 14274</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/+/14274"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I8ed5924b53be9fe06a385c58817e5584b0f25cc2 </div>
<div style="display:none"> Gerrit-Change-Number: 14274 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>