Re #1300: Implemented symmetric payload type in generating SDP answer in SDP negotiator.

This should work for all codecs, audio & video. Can be disabled at compile-time
using PJMEDIA_SDP_NEG_REWRITE_ANSWER_PT macro setting.




git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@3837 74dad513-b988-da41-8d7b-12977e46ad98

5 files changed, 166 insertions, 13 deletions

diff --git a/pjmedia/include/pjmedia/config.h b/pjmedia/include/pjmedia/config.h
index 7e41402d..e7d2e149 100644
--- a/pjmedia/include/pjmedia/config.h
+++ b/pjmedia/include/pjmedia/config.h
@@ -620,6 +620,18 @@
 
 
 /**
+ * This specifies if the SDP negotiator should rewrite answer payload
+ * type numbers to use the same payload type numbers as the remote offer
+ * for all matched codecs.
+ *
+ * Default is 1 (yes)
+ */
+#ifndef PJMEDIA_SDP_NEG_ANSWER_SYMMETRIC_PT
+#   define PJMEDIA_SDP_NEG_ANSWER_SYMMETRIC_PT		1
+#endif
+
+
+/**
  * Support for sending and decoding RTCP port in SDP (RFC 3605).
  * Default is equal to PJMEDIA_ADVERTISE_RTCP setting.
  */
diff --git a/pjmedia/include/pjmedia/stream.h b/pjmedia/include/pjmedia/stream.h
index aa8fa0bd..ea3a1b64 100644
--- a/pjmedia/include/pjmedia/stream.h
+++ b/pjmedia/include/pjmedia/stream.h
@@ -110,6 +110,7 @@ typedef struct pjmedia_stream_info
     pjmedia_codec_info	fmt;	    /**< Incoming codec format info.	    */
     pjmedia_codec_param *param;	    /**< Optional codec param.		    */
     unsigned		tx_pt;	    /**< Outgoing codec paylaod type.	    */
+    unsigned		rx_pt;	    /**< Incoming codec paylaod type.	    */
     unsigned		tx_maxptime;/**< Outgoing codec max ptime.	    */
     int		        tx_event_pt;/**< Outgoing pt for telephone-events.  */
     int			rx_event_pt;/**< Incoming pt for telephone-events.  */
diff --git a/pjmedia/src/pjmedia/sdp_neg.c b/pjmedia/src/pjmedia/sdp_neg.c
index 1053b635..874f1d2d 100644
--- a/pjmedia/src/pjmedia/sdp_neg.c
+++ b/pjmedia/src/pjmedia/sdp_neg.c
@@ -1120,6 +1120,82 @@ static pj_status_t process_answer(pj_pool_t *pool,
     return has_active ? PJ_SUCCESS : PJMEDIA_SDPNEG_ENOMEDIA;
 }
 
+
+/* Internal function to rewrite the format string in SDP attribute rtpmap
+ * and fmtp.
+ */
+PJ_INLINE(void) rewrite_pt(pj_pool_t *pool, pj_str_t *attr_val,
+			   const pj_str_t *old_pt, const pj_str_t *new_pt)
+{
+    int len_diff = new_pt->slen - old_pt->slen;
+
+    /* Note that attribute value should be null-terminated. */
+    if (len_diff > 0) {
+	pj_str_t new_val;
+	new_val.ptr = (char*)pj_pool_alloc(pool, attr_val->slen+len_diff+1);
+	new_val.slen = attr_val->slen + len_diff;
+	pj_memcpy(new_val.ptr + len_diff, attr_val->ptr, attr_val->slen + 1);
+	*attr_val = new_val;
+    } else if (len_diff < 0) {
+	pj_memmove(attr_val->ptr, attr_val->ptr - len_diff,
+		   attr_val->slen + len_diff + 1);
+    }
+    pj_memcpy(attr_val->ptr, new_pt->ptr, new_pt->slen);
+}
+
+
+/* Internal function to apply symmetric PT for the local answer. */
+static void apply_answer_symmetric_pt(pj_pool_t *pool,
+				      pjmedia_sdp_media *answer,
+				      unsigned pt_cnt,
+				      const pj_str_t pt_offer[],
+				      const pj_str_t pt_answer[])
+{
+    pjmedia_sdp_attr *a_tmp[PJMEDIA_MAX_SDP_ATTR];
+    unsigned i, a_tmp_cnt = 0;
+
+    /* Rewrite the payload types in the answer if different to
+     * the ones in the offer.
+     */
+    for (i = 0; i < pt_cnt; ++i) {
+	pjmedia_sdp_attr *a;
+
+	/* Skip if the PTs are the same already, e.g: static PT. */
+	if (pj_strcmp(&pt_answer[i], &pt_offer[i]) == 0)
+	    continue;
+
+	/* Rewrite payload type in the answer to match to the offer */
+	pj_strdup(pool, &answer->desc.fmt[i], &pt_offer[i]);
+
+	/* Also update payload type in rtpmap */
+	a = pjmedia_sdp_media_find_attr2(answer, "rtpmap", &pt_answer[i]);
+	if (a) {
+	    rewrite_pt(pool, &a->value, &pt_answer[i], &pt_offer[i]);
+	    /* Temporarily remove the attribute in case the new payload
+	     * type is being used by another format in the media.
+	     */
+	    pjmedia_sdp_media_remove_attr(answer, a);
+	    a_tmp[a_tmp_cnt++] = a;
+	}
+
+	/* Also update payload type in fmtp */
+	a = pjmedia_sdp_media_find_attr2(answer, "fmtp", &pt_answer[i]);
+	if (a) {
+	    rewrite_pt(pool, &a->value, &pt_answer[i], &pt_offer[i]);
+	    /* Temporarily remove the attribute in case the new payload
+	     * type is being used by another format in the media.
+	     */
+	    pjmedia_sdp_media_remove_attr(answer, a);
+	    a_tmp[a_tmp_cnt++] = a;
+	}
+    }
+
+    /* Return back 'rtpmap' and 'fmtp' attributes */
+    for (i = 0; i < a_tmp_cnt; ++i)
+	pjmedia_sdp_media_add_attr(answer, a_tmp[i]);
+}
+
+
 /* Try to match offer with answer. */
 static pj_status_t match_offer(pj_pool_t *pool,
 			       pj_bool_t prefer_remote_codec_order,
@@ -1137,6 +1213,7 @@ static pj_status_t match_offer(pj_pool_t *pool,
 	      found_matching_other = 0;
     unsigned pt_answer_count = 0;
     pj_str_t pt_answer[PJMEDIA_MAX_SDP_FMT];
+    pj_str_t pt_offer[PJMEDIA_MAX_SDP_FMT];
     pjmedia_sdp_media *answer;
     const pjmedia_sdp_media *master, *slave;
     pj_str_t pt_amr_need_adapt = {NULL, 0};
@@ -1201,6 +1278,7 @@ static pj_status_t match_offer(pj_pool_t *pool,
 		    p = pj_strtoul(&slave->desc.fmt[j]);
 		    if (p == pt && pj_isdigit(*slave->desc.fmt[j].ptr)) {
 			found_matching_codec = 1;
+			pt_offer[pt_answer_count] = slave->desc.fmt[j];
 			pt_answer[pt_answer_count++] = slave->desc.fmt[j];
 			break;
 		    }
@@ -1300,6 +1378,10 @@ static pj_status_t match_offer(pj_pool_t *pool,
 				found_matching_telephone_event = 1;
 			    }
 
+			    pt_offer[pt_answer_count] = 
+						prefer_remote_codec_order?
+						offer->desc.fmt[i]:
+						offer->desc.fmt[j];
 			    pt_answer[pt_answer_count++] = 
 						prefer_remote_codec_order? 
 						preanswer->desc.fmt[j]:
@@ -1325,6 +1407,9 @@ static pj_status_t match_offer(pj_pool_t *pool,
 		if (!pj_strcmp(&master->desc.fmt[i], &slave->desc.fmt[j])) {
 		    /* Match */
 		    found_matching_other = 1;
+		    pt_offer[pt_answer_count] = prefer_remote_codec_order?
+						offer->desc.fmt[i]:
+						offer->desc.fmt[j];
 		    pt_answer[pt_answer_count++] = prefer_remote_codec_order? 
 						   preanswer->desc.fmt[j]:
 						   preanswer->desc.fmt[i];
@@ -1390,6 +1475,11 @@ static pj_status_t match_offer(pj_pool_t *pool,
     }
     answer->desc.fmt_count = pt_answer_count;
 
+#if PJMEDIA_SDP_NEG_ANSWER_SYMMETRIC_PT
+    apply_answer_symmetric_pt(pool, answer, pt_answer_count,
+			      pt_offer, pt_answer);
+#endif
+
     /* Update media direction. */
     update_media_direction(pool, offer, answer);
 
diff --git a/pjmedia/src/pjmedia/stream.c b/pjmedia/src/pjmedia/stream.c
index 1fe2863d..8534caa8 100644
--- a/pjmedia/src/pjmedia/stream.c
+++ b/pjmedia/src/pjmedia/stream.c
@@ -2220,7 +2220,7 @@ PJ_DEF(pj_status_t) pjmedia_stream_create( pjmedia_endpt *endpt,
     /* Create decoder channel: */
 
     status = create_channel( pool, stream, PJMEDIA_DIR_DECODING, 
-			     info->fmt.pt, info, &stream->dec);
+			     info->rx_pt, info, &stream->dec);
     if (status != PJ_SUCCESS)
 	goto err_cleanup;
 
@@ -2834,6 +2834,9 @@ static pj_status_t get_audio_codec_info_param(pjmedia_stream_info *si,
     if ( fmti >= local_m->desc.fmt_count )
 	return PJMEDIA_EINVALIDPT;
 
+    /* Get payload type for receiving direction */
+    si->rx_pt = pt;
+
     /* Get codec info.
      * For static payload types, get the info from codec manager.
      * For dynamic payload types, MUST get the rtpmap.
@@ -2893,6 +2896,9 @@ static pj_status_t get_audio_codec_info_param(pjmedia_stream_info *si,
 	si->tx_pt = pt;
 
     } else {
+	pjmedia_codec_id codec_id;
+	pj_str_t codec_id_st;
+	const pjmedia_codec_info *p_info;
 
 	attr = pjmedia_sdp_media_find_attr(local_m, &ID_RTPMAP, 
 					   &local_m->desc.fmt[fmti]);
@@ -2907,7 +2913,7 @@ static pj_status_t get_audio_codec_info_param(pjmedia_stream_info *si,
 
 	si->fmt.type = si->type;
 	si->fmt.pt = pj_strtoul(&local_m->desc.fmt[fmti]);
-	pj_strdup(pool, &si->fmt.encoding_name, &rtpmap->enc_name);
+	si->fmt.encoding_name = rtpmap->enc_name;
 	si->fmt.clock_rate = rtpmap->clock_rate;
 
 	/* For audio codecs, rtpmap parameters denotes the number of
@@ -2919,6 +2925,23 @@ static pj_status_t get_audio_codec_info_param(pjmedia_stream_info *si,
 	    si->fmt.channel_cnt = 1;
 	}
 
+	/* Normalize the codec info from codec manager. Note that the
+	 * payload type will be resetted to its default (it might have
+	 * been rewritten by the SDP negotiator to match to the remote
+	 * offer), this is intentional as currently some components may
+	 * prefer (or even require) the default PT in codec info.
+	 */
+	pjmedia_codec_info_to_id(&si->fmt, codec_id, sizeof(codec_id));
+
+	i = 1;
+	codec_id_st = pj_str(codec_id);
+	status = pjmedia_codec_mgr_find_codecs_by_id(mgr, &codec_id_st,
+						     &i, &p_info, NULL);
+	if (status != PJ_SUCCESS)
+	    return status;
+
+	pj_memcpy(&si->fmt, p_info, sizeof(pjmedia_codec_info));
+
 	/* Determine payload type for outgoing channel, by finding
 	 * dynamic payload type in remote SDP that matches the answer.
 	 */
@@ -2965,7 +2988,7 @@ static pj_status_t get_audio_codec_info_param(pjmedia_stream_info *si,
 				   &si->param->setting.enc_fmtp);
 
     /* Get local fmtp for our decoder. */
-    pjmedia_stream_info_parse_fmtp(pool, local_m, si->fmt.pt,
+    pjmedia_stream_info_parse_fmtp(pool, local_m, si->rx_pt,
 				   &si->param->setting.dec_fmtp);
 
     /* Get the remote ptime for our encoder. */
diff --git a/pjmedia/src/pjmedia/vid_stream.c b/pjmedia/src/pjmedia/vid_stream.c
index b22e82ed..72d7eea0 100644
--- a/pjmedia/src/pjmedia/vid_stream.c
+++ b/pjmedia/src/pjmedia/vid_stream.c
@@ -1782,22 +1782,27 @@ static pj_status_t get_video_codec_info_param(pjmedia_vid_stream_info *si,
 
     pt = pj_strtoul(&local_m->desc.fmt[0]);
 
-    /* Get codec info. */
-    status = pjmedia_vid_codec_mgr_get_codec_info(mgr, pt, &p_info);
-    if (status != PJ_SUCCESS)
-	return status;
-
-    si->codec_info = *p_info;
-
     /* Get payload type for receiving direction */
     si->rx_pt = pt;
 
-    /* Get payload type for transmitting direction */
+    /* Get codec info and payload type for transmitting direction. */
     if (pt < 96) {
-	/* For static payload type, pt's are symetric */
-	si->tx_pt = pt;
+	/* For static payload types, get the codec info from codec manager. */
+	status = pjmedia_vid_codec_mgr_get_codec_info(mgr, pt, &p_info);
+	if (status != PJ_SUCCESS)
+	    return status;
+
+	si->codec_info = *p_info;
 
+	/* Get payload type for transmitting direction.
+	 * For static payload type, pt's are symetric.
+	 */
+	si->tx_pt = pt;
     } else {
+	const pjmedia_sdp_attr *attr;
+	pjmedia_sdp_rtpmap *rtpmap;
+	pjmedia_codec_id codec_id;
+	pj_str_t codec_id_st;
 	unsigned i;
 
 	/* Determine payload type for outgoing channel, by finding
@@ -1818,6 +1823,28 @@ static pj_status_t get_video_codec_info_param(pjmedia_vid_stream_info *si,
 
 	if (si->tx_pt == 0xFFFF)
 	    return PJMEDIA_EMISSINGRTPMAP;
+
+	/* For dynamic payload types, get codec name from the rtpmap */
+	attr = pjmedia_sdp_media_find_attr(local_m, &ID_RTPMAP, 
+					   &local_m->desc.fmt[0]);
+	if (attr == NULL)
+	    return PJMEDIA_EMISSINGRTPMAP;
+
+	status = pjmedia_sdp_attr_to_rtpmap(pool, attr, &rtpmap);
+	if (status != PJ_SUCCESS)
+	    return status;
+
+	/* Then get the codec info from the codec manager */
+	pj_ansi_snprintf(codec_id, sizeof(codec_id), "%.*s/", 
+			 rtpmap->enc_name.slen, rtpmap->enc_name.ptr);
+	codec_id_st = pj_str(codec_id);
+	i = 1;
+	status = pjmedia_vid_codec_mgr_find_codecs_by_id(mgr, &codec_id_st,
+							 &i, &p_info, NULL);
+	if (status != PJ_SUCCESS)
+	    return status;
+
+	si->codec_info = *p_info;
     }