<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6164">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  George Joseph: Looks good to me, approved

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip_session/_sdp_rtp: Handling of 'msid' is incorrect<br><br>Currently, the handling of the msid attribute is not quite right. According to<br>the spec the msid's between the offer/answer are not dependent upon one another.<br>Meaning the same msid's given in an offer do not have to be returned in the<br>answer for a given stream. And they probably shouldn't be (copied/reused) since<br>this can potentially cause some browser side confusion.<br><br>This patch generates new msids when both an offer and answer are sent from<br>Asterisk. However, Asterisk does reuse the original msid it sent out for a<br>reinvite. Also audio+video streams are paired together by sharing the same<br>stream id, but a different track id.<br><br>ASTERISK-27179 #close<br><br>Change-Id: Ifaec06dc7e65ad841633a24ebec8c8a9302d6643<br>---<br>M include/asterisk/res_pjsip_session.h<br>M res/res_pjsip_sdp_rtp.c<br>M res/res_pjsip_session.c<br>3 files changed, 32 insertions(+), 30 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/res_pjsip_session.h b/include/asterisk/res_pjsip_session.h<br>index caf10db..5f49c82 100644<br>--- a/include/asterisk/res_pjsip_session.h<br>+++ b/include/asterisk/res_pjsip_session.h<br>@@ -105,8 +105,10 @@<br>   int bundle_group;<br>     /*! \brief Whether this stream is currently bundled or not */<br>         unsigned int bundled;<br>-        /*! \brief RTP/Media streams association identifier */<br>-       char *msid;<br>+  /*! \brief Media stream label */<br>+     char mslabel[AST_UUID_STR_LEN];<br>+      /*! \brief Track label */<br>+    char label[AST_UUID_STR_LEN];<br> };<br> <br> /*!<br>diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c<br>index f79c4cb..77cd807 100644<br>--- a/res/res_pjsip_sdp_rtp.c<br>+++ b/res/res_pjsip_sdp_rtp.c<br>@@ -1025,44 +1025,46 @@<br>     }<br> }<br> <br>-static void process_msid_attribute(struct ast_sip_session *session,<br>-       struct ast_sip_session_media *session_media, pjmedia_sdp_media *media)<br>-{<br>-   pjmedia_sdp_attr *attr;<br>-<br>-   if (!session->endpoint->media.webrtc) {<br>-                return;<br>-      }<br>-<br>- attr = pjmedia_sdp_media_find_attr2(media, "msid", NULL);<br>-  if (attr) {<br>-          ast_free(session_media->msid);<br>-            ast_copy_pj_str2(&session_media->msid, &attr->value);<br>-  }<br>-}<br>-<br> static void add_msid_to_stream(struct ast_sip_session *session,<br>-   struct ast_sip_session_media *session_media, pj_pool_t *pool, pjmedia_sdp_media *media)<br>+      struct ast_sip_session_media *session_media, pj_pool_t *pool, pjmedia_sdp_media *media,<br>+      struct ast_stream *stream)<br> {<br>        pj_str_t stmp;<br>        pjmedia_sdp_attr *attr;<br>+      char msid[(AST_UUID_STR_LEN * 2) + 2];<br> <br>     if (!session->endpoint->media.webrtc) {<br>                 return;<br>       }<br> <br>- if (ast_strlen_zero(session_media->msid)) {<br>-               char uuid1[AST_UUID_STR_LEN], uuid2[AST_UUID_STR_LEN];<br>+       if (ast_strlen_zero(session_media->mslabel)) {<br>+            if (ast_sip_session_is_pending_stream_default(session, stream)) {<br>+                    int index;<br> <br>-                if (ast_asprintf(&session_media->msid, "{%s} {%s}",<br>-                 ast_uuid_generate_str(uuid1, sizeof(uuid1)),<br>-                 ast_uuid_generate_str(uuid2, sizeof(uuid2))) < 0) {<br>-                       session_media->msid = NULL;<br>-                       return;<br>+                      /* If this is a default stream we group them together under the same stream, but as different tracks */<br>+                      for (index = 0; index < AST_VECTOR_SIZE(&session->pending_media_state->sessions); ++index) {<br>+                            struct ast_sip_session_media *other_session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, index);<br>+<br>+                             if (session_media == other_session_media) {<br>+                                  continue;<br>+                            }<br>+<br>+                         ast_copy_string(session_media->mslabel, other_session_media->mslabel, sizeof(session_media->mslabel));<br>+                              break;<br>+                       }<br>+            }<br>+<br>+         if (ast_strlen_zero(session_media->mslabel)) {<br>+                    ast_uuid_generate_str(session_media->mslabel, sizeof(session_media->mslabel));<br>          }<br>     }<br> <br>- attr = pjmedia_sdp_attr_create(pool, "msid", pj_cstr(&stmp, session_media->msid));<br>+  if (ast_strlen_zero(session_media->label)) {<br>+              ast_uuid_generate_str(session_media->label, sizeof(session_media->label));<br>+     }<br>+<br>+ snprintf(msid, sizeof(msid), "%s %s", session_media->mslabel, session_media->label);<br>+ attr = pjmedia_sdp_attr_create(pool, "msid", pj_cstr(&stmp, msid));<br>     pjmedia_sdp_attr_add(&media->attr_count, media->attr, attr);<br> }<br> <br>@@ -1127,7 +1129,6 @@<br>  }<br> <br>  process_ssrc_attributes(session, session_media, stream);<br>-     process_msid_attribute(session, session_media, stream);<br>       session_media_transport = ast_sip_session_media_get_transport(session, session_media);<br> <br>     if (session_media_transport == session_media || !session_media->bundled) {<br>@@ -1586,7 +1587,7 @@<br>  }<br> <br>  add_ssrc_to_stream(session, session_media, pool, media);<br>-     add_msid_to_stream(session, session_media, pool, media);<br>+     add_msid_to_stream(session, session_media, pool, media, stream);<br>      add_rtcp_fb_to_stream(session, session_media, pool, media);<br> <br>        /* Add the media stream to the SDP */<br>diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c<br>index bb349a4..607f329 100644<br>--- a/res/res_pjsip_session.c<br>+++ b/res/res_pjsip_session.c<br>@@ -395,7 +395,6 @@<br>       }<br> <br>  ast_free(session_media->mid);<br>-     ast_free(session_media->msid);<br> }<br> <br> struct ast_sip_session_media *ast_sip_session_media_state_add(struct ast_sip_session *session,<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6164">change 6164</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/6164"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ifaec06dc7e65ad841633a24ebec8c8a9302d6643 </div>
<div style="display:none"> Gerrit-Change-Number: 6164 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>