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

diff --git a/include/asterisk/res_pjsip_session.h b/include/asterisk/res_pjsip_session.h
index caf10db1112b10bff36dff4da8b7ae168d1e9236..5f49c82371ea125f49ad8456a86c24db9903c2fc 100644 (file)
--- a/include/asterisk/res_pjsip_session.h
+++ b/include/asterisk/res_pjsip_session.h
@@ -105,8 +105,10 @@ struct ast_sip_session_media {
        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 f79c4cbf068f081055698243ada9a79ff1afb7be..77cd807825f27bbbf35c5436a87b8a81905c8df6 100644 (file)
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1025,44 +1025,46 @@ static void process_ssrc_attributes(struct ast_sip_session *session, struct ast_
        }
 }
 
-static void process_msid_attribute(struct ast_sip_session *session,
-       struct ast_sip_session_media *session_media, pjmedia_sdp_media *media)
+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_stream *stream)
 {
+       pj_str_t stmp;
        pjmedia_sdp_attr *attr;
+       char msid[(AST_UUID_STR_LEN * 2) + 2];
 
        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);
-       }
-}
+       if (ast_strlen_zero(session_media->mslabel)) {
+               if (ast_sip_session_is_pending_stream_default(session, stream)) {
+                       int index;
 
-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)
-{
-       pj_str_t stmp;
-       pjmedia_sdp_attr *attr;
+                       /* 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->endpoint->media.webrtc) {
-               return;
-       }
+                               if (session_media == other_session_media) {
+                                       continue;
+                               }
 
-       if (ast_strlen_zero(session_media->msid)) {
-               char uuid1[AST_UUID_STR_LEN], uuid2[AST_UUID_STR_LEN];
+                               ast_copy_string(session_media->mslabel, other_session_media->mslabel, sizeof(session_media->mslabel));
+                               break;
+                       }
+               }
 
-               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 (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 @@ static int negotiate_incoming_sdp_stream(struct ast_sip_session *session,
        }
 
        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 @@ static int create_outgoing_sdp_stream(struct ast_sip_session *session, struct as
        }
 
        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 bb349a4b67d18f658748ee9f9b74dcc52fcdb1eb..607f329abfc79bcbce58eb1f06878f3d307929e9 100644 (file)
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -395,7 +395,6 @@ static void session_media_dtor(void *obj)
        }
 
        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,