<p>Kevin Harwell has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6163">View Change</a></p><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;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/63/6163/1</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..ad1abd2 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/6163">change 6163</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/6163"/><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: Ifaec06dc7e65ad841633a24ebec8c8a9302d6643 </div>
<div style="display:none"> Gerrit-Change-Number: 6163 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Kevin Harwell <kharwell@digium.com> </div>