[Asterisk-code-review] res pjsip sdp rtp: No rtpmap for static RTP payload IDs in SDP. (asterisk[master])

Jenkins2 asteriskteam at digium.com
Wed Apr 26 10:44:01 CDT 2017


Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5431 )

Change subject: res_pjsip_sdp_rtp: No rtpmap for static RTP payload IDs in SDP.
......................................................................


res_pjsip_sdp_rtp: No rtpmap for static RTP payload IDs in SDP.

This saves around 100 bytes when G.711, G.722, G.729, and GSM are advertised in
SDP. This reduces the chance to hit the MTU bearer of 1300 bytes for SIP over
UDP, if many codecs are allowed in Asterisk. This new feature is enabled
together with the optional feature compact_headers=yes via the file pjsip.conf.

ASTERISK-26932 #close

Change-Id: Iaa556ab4c8325cd34c334387ab2847fab07b1689
---
M channels/chan_sip.c
M include/asterisk/rtp_engine.h
M main/rtp_engine.c
M res/res_pjsip_sdp_rtp.c
4 files changed, 26 insertions(+), 15 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Jenkins2: Approved for Submit
  Sean Bright: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 5419a1d..792df5c 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -13097,7 +13097,7 @@
 	/* Opus mandates 2 channels in rtpmap */
 	if (ast_format_cmp(format, ast_format_opus) == AST_FORMAT_CMP_EQUAL) {
 		ast_str_append(a_buf, 0, "a=rtpmap:%d %s/%u/2\r\n", rtp_code, mime, rate);
-	} else if ((35 <= rtp_code) || !(sip_cfg.compactheaders)) {
+	} else if ((AST_RTP_PT_LAST_STATIC < rtp_code) || !(sip_cfg.compactheaders)) {
 		ast_str_append(a_buf, 0, "a=rtpmap:%d %s/%u\r\n", rtp_code, mime, rate);
 	}
 
diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h
index fa7fed8..55acf65 100644
--- a/include/asterisk/rtp_engine.h
+++ b/include/asterisk/rtp_engine.h
@@ -81,6 +81,12 @@
 /*! Maximum number of payload types RTP can support. */
 #define AST_RTP_MAX_PT 128
 
+/*!
+ * Last RTP payload type statically assigned, see
+ * http://www.iana.org/assignments/rtp-parameters
+ */
+#define AST_RTP_PT_LAST_STATIC 34
+
 /*! First dynamic RTP payload type */
 #define AST_RTP_PT_FIRST_DYNAMIC 96
 
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 00f9d59..19ca31c 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -1348,28 +1348,31 @@
 	 * https://tools.ietf.org/html/draft-roach-mmusic-unified-plan#section-3.2.1.2
 	 * https://tools.ietf.org/html/draft-wu-avtcore-dynamic-pt-usage#section-3
 	 */
-	res = find_unused_payload_in_range(codecs, MAX(ast_option_rtpptdynamic, 35),
+	res = find_unused_payload_in_range(
+		codecs, MAX(ast_option_rtpptdynamic, AST_RTP_PT_LAST_STATIC + 1),
 		AST_RTP_PT_LAST_REASSIGN, static_RTP_PT);
 	if (res != -1) {
 		return res;
 	}
 
-	/* Yet, reusing mappings below 35 is not supported in Asterisk because
-	 * when Compact Headers are activated, no rtpmap is send for those below
-	 * 35. If you want to use 35 and below
+	/* Yet, reusing mappings below AST_RTP_PT_LAST_STATIC (35) is not supported
+	 * in Asterisk because when Compact Headers are activated, no rtpmap is
+	 * send for those below 35. If you want to use 35 and below
 	 * A) do not use Compact Headers,
 	 * B) remove that code in chan_sip/res_pjsip, or
 	 * C) add a flag that this RTP Payload Type got reassigned dynamically
 	 *    and requires a rtpmap even with Compact Headers enabled.
 	 */
 	res = find_unused_payload_in_range(
-		codecs, MAX(ast_option_rtpptdynamic, 20), 35, static_RTP_PT);
+		codecs, MAX(ast_option_rtpptdynamic, 20),
+		AST_RTP_PT_LAST_STATIC + 1, static_RTP_PT);
 	if (res != -1) {
 		return res;
 	}
 
 	return find_unused_payload_in_range(
-		codecs, MAX(ast_option_rtpptdynamic, 0), 20, static_RTP_PT);
+		codecs, MAX(ast_option_rtpptdynamic, 0),
+		20, static_RTP_PT);
 }
 
 /*!
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 701edc3..64c5066 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -447,6 +447,7 @@
 static pjmedia_sdp_attr* generate_rtpmap_attr(struct ast_sip_session *session, pjmedia_sdp_media *media, pj_pool_t *pool,
 					      int rtp_code, int asterisk_format, struct ast_format *format, int code)
 {
+	extern pj_bool_t pjsip_use_compact_form;
 	pjmedia_sdp_rtpmap rtpmap;
 	pjmedia_sdp_attr *attr = NULL;
 	char tmp[64];
@@ -455,6 +456,11 @@
 
 	snprintf(tmp, sizeof(tmp), "%d", rtp_code);
 	pj_strdup2(pool, &media->desc.fmt[media->desc.fmt_count++], tmp);
+
+	if (rtp_code <= AST_RTP_PT_LAST_STATIC && pjsip_use_compact_form) {
+		return NULL;
+	}
+
 	rtpmap.pt = media->desc.fmt[media->desc.fmt_count - 1];
 	rtpmap.clock_rate = ast_rtp_lookup_sample_rate2(asterisk_format, format, code);
 	pj_strdup2(pool, &rtpmap.enc_name, ast_rtp_lookup_mime_subtype2(asterisk_format, format, code, options));
@@ -1254,11 +1260,9 @@
 			continue;
 		}
 
-		if (!(attr = generate_rtpmap_attr(session, media, pool, rtp_code, 1, format, 0))) {
-			ao2_ref(format, -1);
-			continue;
+		if ((attr = generate_rtpmap_attr(session, media, pool, rtp_code, 1, format, 0))) {
+			media->attr[media->attr_count++] = attr;
 		}
-		media->attr[media->attr_count++] = attr;
 
 		if ((attr = generate_fmtp_attr(pool, format, rtp_code))) {
 			media->attr[media->attr_count++] = attr;
@@ -1287,11 +1291,9 @@
 				continue;
 			}
 
-			if (!(attr = generate_rtpmap_attr(session, media, pool, rtp_code, 0, NULL, index))) {
-				continue;
+			if ((attr = generate_rtpmap_attr(session, media, pool, rtp_code, 0, NULL, index))) {
+				media->attr[media->attr_count++] = attr;
 			}
-
-			media->attr[media->attr_count++] = attr;
 
 			if (index == AST_RTP_DTMF) {
 				snprintf(tmp, sizeof(tmp), "%d 0-16", rtp_code);

-- 
To view, visit https://gerrit.asterisk.org/5431
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iaa556ab4c8325cd34c334387ab2847fab07b1689
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>



More information about the asterisk-code-review mailing list