<p>Maximilian Fridrich has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/19585">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">core & res_pjsip: Improve topology change handling.<br><br>This PR contains two relatively separate changes in channel.c and<br>res_pjsip_session.c which ensure that topology changes are not ignored<br>in cases where they should be handled.<br><br>For channel.c:<br><br>The function ast_channel_request_stream_topology_change only triggers a<br>stream topology request change indication, if the channel's topology<br>does not equal the requested topology. However, a channel could be in a<br>state where it is currently "negotiating" a new topology but hasn't<br>updated it yet, so the topology request change would be lost. Channels<br>need to be able to handle such situations internally and stream<br>topology requests should therefore always be passed on.<br><br>In the case of chan_pjsip for example, it queues a session refresh<br>(re-INVITE) if it is currently in the middle of a transaction or has<br>pending requests (among other reasons).<br><br>Now, ast_channel_request_stream_topology_change always indicates a<br>stream topology request change even if the requested topology equals the<br>channel's topology.<br><br>For res_pjsip_session.c:<br><br>The function resolve_refresh_media_states does not process stream state<br>changes if the delayed active state differs from the current active<br>state. I.e. if the currently active stream state has changed between the<br>time the sip session refresh request was queued and the time it is being<br>processed, the session refresh is ignored. However, res_pjsip_session<br>contains logic that ensures that session refreshes are queued and<br>re-queued correctly if a session refresh is currently not possible. So<br>this check is not necessary and led to some session refreshes being<br>lost.<br><br>Now, a session refresh is done even if the delayed active state differs<br>from the current active state and it is checked whether the delayed<br>pending state differs from the current active - because that means a<br>refresh is necessary.<br><br>Further, the unit test of resolve_refresh_media_states was adapted to<br>reflect the new behavior. I.e. the changes to delayed pending are<br>prioritized over the changes to current active because we want to<br>preserve the original intention of the pending state.<br><br>ASTERISK-30184<br><br>Change-Id: Icd0703295271089057717006730b555b9a1d4e5a<br>---<br>M main/channel.c<br>M res/res_pjsip_session.c<br>2 files changed, 62 insertions(+), 19 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/85/19585/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/main/channel.c b/main/channel.c</span><br><span>index 97ba0f8..8fa4509 100644</span><br><span>--- a/main/channel.c</span><br><span>+++ b/main/channel.c</span><br><span>@@ -11163,15 +11163,6 @@</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (ast_stream_topology_equal(ast_channel_get_stream_topology(chan), topology)) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_debug(2, "%s: Topologies already match. Current: %s Requested: %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- ast_channel_name(chan),</span><br><span style="color: hsl(0, 100%, 40%);">- ast_str_tmp(256, ast_stream_topology_to_str(ast_channel_get_stream_topology(chan), &STR_TMP)),</span><br><span style="color: hsl(0, 100%, 40%);">- ast_str_tmp(256, ast_stream_topology_to_str(topology, &STR_TMP)));</span><br><span style="color: hsl(0, 100%, 40%);">- ast_channel_unlock(chan);</span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> ast_channel_internal_set_stream_topology_change_source(chan, change_source);</span><br><span> </span><br><span> res = ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE, topology, sizeof(topology));</span><br><span>diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c</span><br><span>index c5890e0..a7bde0d 100644</span><br><span>--- a/res/res_pjsip_session.c</span><br><span>+++ b/res/res_pjsip_session.c</span><br><span>@@ -1935,15 +1935,7 @@</span><br><span> /* All the same state, no need to update. */</span><br><span> SCOPE_EXIT_EXPR(continue, "%s: All in the same state so nothing to do\n", session_name);</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if (da_state != ca_state) {</span><br><span style="color: hsl(0, 100%, 40%);">- /*</span><br><span style="color: hsl(0, 100%, 40%);">- * Something set the CA state between the time this request was queued</span><br><span style="color: hsl(0, 100%, 40%);">- * and now. The CA state wins so we don't do anything.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- SCOPE_EXIT_EXPR(continue, "%s: Ignoring request to change state from %s to %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- session_name, ast_stream_state2str(ca_state), ast_stream_state2str(dp_state));</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- if (dp_state != da_state) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (dp_state != ca_state) {</span><br><span> /* DP needs to update the state */</span><br><span> ast_stream_set_state(np_stream, dp_state);</span><br><span> SCOPE_EXIT_EXPR(continue, "%s: Changed NP stream state from %s to %s\n",</span><br><span>@@ -5879,6 +5871,7 @@</span><br><span> </span><br><span> test_media_add(expected_pending_state, "audio", AST_MEDIA_TYPE_AUDIO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> test_media_add(expected_pending_state, "myvideo1", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_media_add(expected_pending_state, "myvideo2", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> test_media_add(expected_pending_state, "myvideo3", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> CHECKER();</span><br><span> </span><br><span>@@ -5917,8 +5910,9 @@</span><br><span> </span><br><span> test_media_add(expected_pending_state, "audio", AST_MEDIA_TYPE_AUDIO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> test_media_add(expected_pending_state, "myvideo1", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span style="color: hsl(0, 100%, 40%);">- test_media_add(expected_pending_state, "myvideo3", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_media_add(expected_pending_state, "myvideo2", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> test_media_add(expected_pending_state, "myvideo4", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_media_add(expected_pending_state, "myvideo3", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> CHECKER();</span><br><span> </span><br><span> RESET_STATE(7);</span><br><span>@@ -5964,6 +5958,7 @@</span><br><span> </span><br><span> test_media_add(expected_pending_state, "audio", AST_MEDIA_TYPE_AUDIO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> test_media_add(expected_pending_state, "myvideo1", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_media_add(expected_pending_state, "myvideo2", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> test_media_add(expected_pending_state, "myvideo3", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> test_media_add(expected_pending_state, "myvideo4", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> CHECKER();</span><br><span>@@ -5984,6 +5979,8 @@</span><br><span> test_media_add(current_active_state, "myvideo2", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_REMOVED, -1);</span><br><span> </span><br><span> test_media_add(expected_pending_state, "audio", AST_MEDIA_TYPE_AUDIO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_media_add(expected_pending_state, "myvideo1", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ test_media_add(expected_pending_state, "myvideo2", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> test_media_add(expected_pending_state, "myvideo3", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> test_media_add(expected_pending_state, "myvideo4", AST_MEDIA_TYPE_VIDEO, AST_STREAM_STATE_SENDRECV, -1);</span><br><span> CHECKER();</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19585">change 19585</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/+/19585"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 20 </div>
<div style="display:none"> Gerrit-Change-Id: Icd0703295271089057717006730b555b9a1d4e5a </div>
<div style="display:none"> Gerrit-Change-Number: 19585 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Maximilian Fridrich <m.fridrich@commend.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>