<p>George Joseph has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/8379">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_session: properly handle SDP from a forked call with early media<br><br>In handle_negotiated_sdp, use session->active_media_state when<br>session->pending_media_state is empty. The 200's SDP should be<br>fed into handle_negotiated_sdp_session_media together with<br>the already negotiated state, which is now in session->active_media_state<br>instead. Only if both the session's pending & active media are empty,<br>should handle_negotiated_sdp break.<br><br>ASTERISK-27441<br><br>Change-Id: If0d5150ffe6f38d8a854831fef37942258d4629c<br>---<br>M res/res_pjsip_session.c<br>1 file changed, 34 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/79/8379/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c<br>index fcd190b..ebbe395 100644<br>--- a/res/res_pjsip_session.c<br>+++ b/res/res_pjsip_session.c<br>@@ -874,16 +874,29 @@<br> {<br> int i;<br> struct ast_stream_topology *topology;<br>+ struct ast_sip_session_media_state* media_state;<br> unsigned int changed = 0;<br> <br>- /* This situation can legitimately happen when an SDP is received in a<br>- * 183 Session Progress message. In that case, everything's been done<br>- * by the time this function is called and there are no more pending<br>- * streams.<br>- */<br>- if (!session->pending_media_state->topology) {<br>+ if (session->pending_media_state->topology) {<br>+ /* Normal case: use pending media state to negotiate;<br>+ * the pending state becomes the active state after negotiation<br>+ */<br>+ media_state = session->pending_media_state;<br>+<br>+ } else if (session->active_media_state) {<br>+ /* This happens when we're negotiated media after receiving a 183,<br>+ * and we're now receiving a 200 with new SDP.<br>+ * In this case, there is active_media_state, but the pending_media_state<br>+ * has been reset.<br>+ */<br> ast_debug(1, "Pending topology was NULL for channel '%s'\n",<br>- session->channel ? ast_channel_name(session->channel) : "unknown");<br>+ session->channel ? ast_channel_name(session->channel) : "unknown");<br>+ // Use active_media_state to negotiate<br>+ media_state = session->active_media_state;<br>+<br>+ } else {<br>+ ast_log(LOG_WARNING, "No pending or active media state for channel '%s'\n",<br>+ session->channel ? ast_channel_name(session->channel) : "unknown");<br> return 0;<br> }<br> <br>@@ -892,11 +905,11 @@<br> * the local SDP, and there should be the same number of session medias<br> * and streams as there are local SDP streams<br> */<br>- if (ast_stream_topology_get_count(session->pending_media_state->topology) != local->media_count<br>- || AST_VECTOR_SIZE(&session->pending_media_state->sessions) != local->media_count) {<br>+ if (ast_stream_topology_get_count(media_state->topology) != local->media_count<br>+ || AST_VECTOR_SIZE(&media_state->sessions) != local->media_count) {<br> ast_log(LOG_WARNING, "Local SDP for channel '%s' contains %d media streams while we expected it to contain %u\n",<br> session->channel ? ast_channel_name(session->channel) : "unknown",<br>- ast_stream_topology_get_count(session->pending_media_state->topology), local->media_count);<br>+ ast_stream_topology_get_count(media_state->topology), local->media_count);<br> return -1;<br> }<br> <br>@@ -908,8 +921,8 @@<br> continue;<br> }<br> <br>- session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, i);<br>- stream = ast_stream_topology_get_stream(session->pending_media_state->topology, i);<br>+ session_media = AST_VECTOR_GET(&media_state->sessions, i);<br>+ stream = ast_stream_topology_get_stream(media_state->topology, i);<br> <br> /* The stream state will have already been set to removed when either we<br> * negotiate the incoming SDP stream or when we produce our own local SDP.<br>@@ -945,8 +958,8 @@<br> continue;<br> }<br> <br>- session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, i);<br>- stream = ast_stream_topology_get_stream(session->pending_media_state->topology, i);<br>+ session_media = AST_VECTOR_GET(&media_state->sessions, i);<br>+ stream = ast_stream_topology_get_stream(media_state->topology, i);<br> <br> if (ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED<br> && session_media->handler) {<br>@@ -960,7 +973,7 @@<br> }<br> <br> /* Update the topology on the channel to match the accepted one */<br>- topology = ast_stream_topology_clone(session->pending_media_state->topology);<br>+ topology = ast_stream_topology_clone(media_state->topology);<br> if (topology) {<br> ast_channel_set_stream_topology(session->channel, topology);<br> }<br>@@ -971,16 +984,18 @@<br> }<br> <br> /* Add all the file descriptors from the pending media state */<br>- for (i = 0; i < AST_VECTOR_SIZE(&session->pending_media_state->read_callbacks); ++i) {<br>+ for (i = 0; i < AST_VECTOR_SIZE(&media_state->read_callbacks); ++i) {<br> struct ast_sip_session_media_read_callback_state *callback_state;<br> <br>- callback_state = AST_VECTOR_GET_ADDR(&session->pending_media_state->read_callbacks, i);<br>+ callback_state = AST_VECTOR_GET_ADDR(&media_state->read_callbacks, i);<br> ast_channel_internal_fd_set(session->channel, i + AST_EXTENDED_FDS, callback_state->fd);<br> }<br> <br> /* Active and pending flip flop as needed */<br>- SWAP(session->active_media_state, session->pending_media_state);<br>- ast_sip_session_media_state_reset(session->pending_media_state);<br>+ if (media_state == session->pending_media_state) {<br>+ SWAP(session->active_media_state, session->pending_media_state);<br>+ ast_sip_session_media_state_reset(session->pending_media_state);<br>+ }<br> <br> ast_channel_unlock(session->channel);<br> <br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/8379">change 8379</a>. To unsubscribe, 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/8379"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: If0d5150ffe6f38d8a854831fef37942258d4629c </div>
<div style="display:none"> Gerrit-Change-Number: 8379 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: lvl <digium@lvlconsultancy.nl> </div>