[Asterisk-code-review] res pjsip session/ sdp rtp: Handling of 'msid' is incorrect (asterisk[master])
Jenkins2
asteriskteam at digium.com
Wed Aug 9 08:15:24 CDT 2017
Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/6163 )
Change subject: res_pjsip_session/_sdp_rtp: Handling of 'msid' is incorrect
......................................................................
res_pjsip_session/_sdp_rtp: Handling of 'msid' is incorrect
Currently, the handling of the msid attribute is not quite right. According to
the spec the msid's between the offer/answer are not dependent upon one another.
Meaning the same msid's given in an offer do not have to be returned in the
answer for a given stream. And they probably shouldn't be (copied/reused) since
this can potentially cause some browser side confusion.
This patch generates new msids when both an offer and answer are sent from
Asterisk. However, Asterisk does reuse the original msid it sent out for a
reinvite. Also audio+video streams are paired together by sharing the same
stream id, but a different track id.
ASTERISK-27179 #close
Change-Id: Ifaec06dc7e65ad841633a24ebec8c8a9302d6643
---
M include/asterisk/res_pjsip_session.h
M res/res_pjsip_sdp_rtp.c
M res/res_pjsip_session.c
3 files changed, 32 insertions(+), 30 deletions(-)
Approvals:
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved
Jenkins2: Approved for Submit
diff --git a/include/asterisk/res_pjsip_session.h b/include/asterisk/res_pjsip_session.h
index caf10db..5f49c82 100644
--- a/include/asterisk/res_pjsip_session.h
+++ b/include/asterisk/res_pjsip_session.h
@@ -105,8 +105,10 @@
int bundle_group;
/*! \brief Whether this stream is currently bundled or not */
unsigned int bundled;
- /*! \brief RTP/Media streams association identifier */
- char *msid;
+ /*! \brief Media stream label */
+ char mslabel[AST_UUID_STR_LEN];
+ /*! \brief Track label */
+ char label[AST_UUID_STR_LEN];
};
/*!
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index f79c4cb..77cd807 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1025,44 +1025,46 @@
}
}
-static void process_msid_attribute(struct ast_sip_session *session,
- struct ast_sip_session_media *session_media, pjmedia_sdp_media *media)
-{
- pjmedia_sdp_attr *attr;
-
- if (!session->endpoint->media.webrtc) {
- return;
- }
-
- attr = pjmedia_sdp_media_find_attr2(media, "msid", NULL);
- if (attr) {
- ast_free(session_media->msid);
- ast_copy_pj_str2(&session_media->msid, &attr->value);
- }
-}
-
static void add_msid_to_stream(struct ast_sip_session *session,
- struct ast_sip_session_media *session_media, pj_pool_t *pool, pjmedia_sdp_media *media)
+ struct ast_sip_session_media *session_media, pj_pool_t *pool, pjmedia_sdp_media *media,
+ struct ast_stream *stream)
{
pj_str_t stmp;
pjmedia_sdp_attr *attr;
+ char msid[(AST_UUID_STR_LEN * 2) + 2];
if (!session->endpoint->media.webrtc) {
return;
}
- if (ast_strlen_zero(session_media->msid)) {
- char uuid1[AST_UUID_STR_LEN], uuid2[AST_UUID_STR_LEN];
+ if (ast_strlen_zero(session_media->mslabel)) {
+ if (ast_sip_session_is_pending_stream_default(session, stream)) {
+ int index;
- if (ast_asprintf(&session_media->msid, "{%s} {%s}",
- ast_uuid_generate_str(uuid1, sizeof(uuid1)),
- ast_uuid_generate_str(uuid2, sizeof(uuid2))) < 0) {
- session_media->msid = NULL;
- return;
+ /* If this is a default stream we group them together under the same stream, but as different tracks */
+ for (index = 0; index < AST_VECTOR_SIZE(&session->pending_media_state->sessions); ++index) {
+ struct ast_sip_session_media *other_session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, index);
+
+ if (session_media == other_session_media) {
+ continue;
+ }
+
+ ast_copy_string(session_media->mslabel, other_session_media->mslabel, sizeof(session_media->mslabel));
+ break;
+ }
+ }
+
+ if (ast_strlen_zero(session_media->mslabel)) {
+ ast_uuid_generate_str(session_media->mslabel, sizeof(session_media->mslabel));
}
}
- attr = pjmedia_sdp_attr_create(pool, "msid", pj_cstr(&stmp, session_media->msid));
+ if (ast_strlen_zero(session_media->label)) {
+ ast_uuid_generate_str(session_media->label, sizeof(session_media->label));
+ }
+
+ snprintf(msid, sizeof(msid), "%s %s", session_media->mslabel, session_media->label);
+ attr = pjmedia_sdp_attr_create(pool, "msid", pj_cstr(&stmp, msid));
pjmedia_sdp_attr_add(&media->attr_count, media->attr, attr);
}
@@ -1127,7 +1129,6 @@
}
process_ssrc_attributes(session, session_media, stream);
- process_msid_attribute(session, session_media, stream);
session_media_transport = ast_sip_session_media_get_transport(session, session_media);
if (session_media_transport == session_media || !session_media->bundled) {
@@ -1586,7 +1587,7 @@
}
add_ssrc_to_stream(session, session_media, pool, media);
- add_msid_to_stream(session, session_media, pool, media);
+ add_msid_to_stream(session, session_media, pool, media, stream);
add_rtcp_fb_to_stream(session, session_media, pool, media);
/* Add the media stream to the SDP */
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index bb349a4..607f329 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -395,7 +395,6 @@
}
ast_free(session_media->mid);
- ast_free(session_media->msid);
}
struct ast_sip_session_media *ast_sip_session_media_state_add(struct ast_sip_session *session,
--
To view, visit https://gerrit.asterisk.org/6163
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifaec06dc7e65ad841633a24ebec8c8a9302d6643
Gerrit-Change-Number: 6163
Gerrit-PatchSet: 2
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170809/487512b1/attachment.html>
More information about the asterisk-code-review
mailing list