<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>