[Asterisk-code-review] rtp engine: allocate RTP dynamic payloads per session (asterisk[master])

Anonymous Coward asteriskteam at digium.com
Fri Mar 24 16:22:56 CDT 2017


Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5260 )

Change subject: rtp_engine: allocate RTP dynamic payloads per session
......................................................................


rtp_engine: allocate RTP dynamic payloads per session

Dynamic payload types were statically defined in Asterisk. This unfortunately
limited the number of dynamic payloads that could be registered. With this patch
dynamic payload type numbers are now assigned dynamically and per RTP instance.
However, in order to limit any issues where some clients expect the old
statically defined value this patch makes it so the value Asterisk used to pre-
designate is used for the dynamic assignment if available.

An option, "rtp_use_dynamic", has also been added (can be set in asterisk.conf)
that turns the new dynamic behavior on or off. When off it reverts back to using
statically defined payload values. This option defaults to "yes" in Asterisk 15.

ASTERISK-26515 #close
patches:
  ASTERISK-26515.diff submitted by jcolp (license 5000

Change-Id: I7653465c5ebeaf968f1a1cc8f3f4f5c4321da7fc
---
M CHANGES
M configs/samples/asterisk.conf.sample
M include/asterisk/options.h
M main/asterisk.c
M main/rtp_engine.c
5 files changed, 175 insertions(+), 109 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Matthew Fredrickson: Looks good to me, but someone else must approve



diff --git a/CHANGES b/CHANGES
index e391d1e..00f318b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -62,6 +62,11 @@
    when you use more than 32 formats and calls are not accepted by a remote
    implementation, please report this and go back to rtp_pt_dynamic = 96.
 
+ * A new setting, "rtp_use_dynamic", has been added in asterisk.conf". When set
+   to "yes" RTP dynamic payload types are assigned dynamically per RTP instance.
+   When set to "no" RTP dynamic payload types are globally initialized to pre-
+   designated numbers and function similar to static payload types.
+
 app_originate
 ------------------
  * Added support to gosub predial routines on both original channel and on the
diff --git a/configs/samples/asterisk.conf.sample b/configs/samples/asterisk.conf.sample
index e13a944..30934e4 100644
--- a/configs/samples/asterisk.conf.sample
+++ b/configs/samples/asterisk.conf.sample
@@ -97,6 +97,10 @@
 				; This is currently is used by DUNDi and
 				; Exchanging Device and Mailbox State
 				; using protocols: XMPP, Corosync and PJSIP.
+;rtp_use_dynamic = yes          ; When set to "yes" RTP dynamic payload types
+                                ; are assigned dynamically per RTP instance vs.
+                                ; allowing Asterisk to globally initialize them
+                                ; to pre-designated numbers (defaults to "yes").
 ;rtp_pt_dynamic = 35		; Normally the Dynamic RTP Payload Type numbers
 				; are 96-127, which allow just 32 formats. The
 				; starting point 35 enables the range 35-63 and
diff --git a/include/asterisk/options.h b/include/asterisk/options.h
index 05ad6c5..0a20f10 100644
--- a/include/asterisk/options.h
+++ b/include/asterisk/options.h
@@ -196,6 +196,7 @@
 
 extern int ast_language_is_prefix;
 
+extern int ast_option_rtpusedynamic;
 extern unsigned int ast_option_rtpptdynamic;
 
 #if defined(__cplusplus) || defined(c_plusplus)
diff --git a/main/asterisk.c b/main/asterisk.c
index 69183c1..68859ab 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -339,6 +339,7 @@
 #if defined(HAVE_SYSINFO)
 long option_minmemfree;				/*!< Minimum amount of free system memory - stop accepting calls if free memory falls below this watermark */
 #endif
+int ast_option_rtpusedynamic;
 unsigned int ast_option_rtpptdynamic;
 
 /*! @} */
@@ -602,6 +603,7 @@
 	ast_cli(a->fd, "  Transmit silence during rec: %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_TRANSMIT_SILENCE) ? "Enabled" : "Disabled");
 	ast_cli(a->fd, "  Generic PLC:                 %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_GENERIC_PLC) ? "Enabled" : "Disabled");
 	ast_cli(a->fd, "  Min DTMF duration::          %u\n", option_dtmfminduration);
+	ast_cli(a->fd, "  RTP use dynamic payloads:    %u\n", ast_option_rtpusedynamic);
 
 	if (ast_option_rtpptdynamic == AST_RTP_PT_LAST_REASSIGN) {
 		ast_cli(a->fd, "  RTP dynamic payload types:   %u,%u-%u\n",
@@ -3499,6 +3501,7 @@
 
 	/* Set default value */
 	option_dtmfminduration = AST_MIN_DTMF_DURATION;
+	ast_option_rtpusedynamic = 1;
 	ast_option_rtpptdynamic = 35;
 
 	/* init with buildtime config */
@@ -3655,6 +3658,8 @@
 			if (sscanf(v->value, "%30u", &option_dtmfminduration) != 1) {
 				option_dtmfminduration = AST_MIN_DTMF_DURATION;
 			}
+		} else if (!strcasecmp(v->name, "rtp_use_dynamic")) {
+			ast_option_rtpusedynamic = ast_true(v->value);
 		/* http://www.iana.org/assignments/rtp-parameters
 		 * RTP dynamic payload types start at 96 normally; extend down to 0 */
 		} else if (!strcasecmp(v->name, "rtp_pt_dynamic")) {
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 931f89d..00f9d59 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -266,14 +266,28 @@
 	ao2_cleanup(payload->format);
 }
 
+static struct ast_rtp_payload_type *rtp_payload_type_alloc(struct ast_format *format,
+	int payload, int rtp_code, int primary_mapping)
+{
+	struct ast_rtp_payload_type *type = ao2_alloc_options(
+		sizeof(*type), rtp_payload_type_dtor, AO2_ALLOC_OPT_LOCK_NOLOCK);
+
+	if (!type) {
+		return NULL;
+	}
+
+	type->format = ao2_bump(format);
+	type->asterisk_format = type->format != NULL;
+	type->payload = payload;
+	type->rtp_code = rtp_code;
+	type->primary_mapping = primary_mapping;
+
+	return type;
+}
+
 struct ast_rtp_payload_type *ast_rtp_engine_alloc_payload_type(void)
 {
-	struct ast_rtp_payload_type *payload;
-
-	payload = ao2_alloc_options(sizeof(*payload), rtp_payload_type_dtor,
-		AO2_ALLOC_OPT_LOCK_NOLOCK);
-
-	return payload;
+	return rtp_payload_type_alloc(NULL, 0, 0, 0);
 }
 
 int ast_rtp_engine_register2(struct ast_rtp_engine *engine, struct ast_module *module)
@@ -1250,31 +1264,112 @@
 
 /*!
  * \internal
- * \brief Find the first unused dynamic rx payload type.
- * \since 14.0.0
+ * \brief Find the first unused payload type in a given range
  *
- * \param codecs Codecs structure to look in
+ * \param codecs The codec structure to look in
+ * \param start Starting index
+ * \param end Ending index
+ * \param ignore Skip these payloads types
  *
- * \note It is assumed that codecs is at least read locked before calling.
+ * \note The static_RTP_PT_lock object must be locked before calling
  *
  * \retval Numerical payload type
  * \retval -1 if not found.
  */
-static int rtp_codecs_find_empty_dynamic_rx(struct ast_rtp_codecs *codecs)
+static int find_unused_payload_in_range(const struct ast_rtp_codecs *codecs,
+		int start, int end, struct ast_rtp_payload_type *ignore[])
 {
-	struct ast_rtp_payload_type *type;
-	int idx;
-	int payload = -1;
+	int x;
 
-	idx = AST_RTP_PT_FIRST_DYNAMIC;
-	for (; idx < AST_VECTOR_SIZE(&codecs->payload_mapping_rx); ++idx) {
-		type = AST_VECTOR_GET(&codecs->payload_mapping_rx, idx);
+	for (x = start; x < end; ++x) {
+		struct ast_rtp_payload_type *type;
+
+		if (ignore[x]) {
+			continue;
+		} else if (!codecs || x >= AST_VECTOR_SIZE(&codecs->payload_mapping_rx)) {
+			return x;
+		}
+
+		type = AST_VECTOR_GET(&codecs->payload_mapping_rx, x);
 		if (!type) {
-			payload = idx;
-			break;
+			return x;
 		}
 	}
-	return payload;
+	return -1;
+}
+
+/*!
+ * \internal
+ * \brief Find an unused payload type
+ *
+ * \param codecs Codecs structure to look in
+ *
+ * \note Both static_RTP_PT_lock and codecs (if given) must be at least
+ *       read locked before calling.
+ *
+ * \retval Numerical payload type
+ * \retval -1 if not found.
+ */
+static int find_unused_payload(const struct ast_rtp_codecs *codecs)
+{
+	int res;
+
+	/* find next available dynamic payload slot */
+	res = find_unused_payload_in_range(
+		codecs, AST_RTP_PT_FIRST_DYNAMIC, AST_RTP_MAX_PT, static_RTP_PT);
+	if (res != -1) {
+		return res;
+	}
+
+	if (ast_option_rtpusedynamic) {
+		/*
+		 * We're using default values for some dynamic types. So if an unused
+		 * slot was not found try again, but this time ignore the default
+		 * values declared for dynamic types (except for 101 and 121) .
+		 */
+		static struct ast_rtp_payload_type *ignore[AST_RTP_MAX_PT] = {0};
+
+		ignore[101] = static_RTP_PT[101];
+		ignore[121] = static_RTP_PT[121];
+
+		res = find_unused_payload_in_range(
+			codecs, AST_RTP_PT_FIRST_DYNAMIC, AST_RTP_MAX_PT, ignore);
+		if (res != -1) {
+			return res;
+		}
+	}
+
+	/* http://www.iana.org/assignments/rtp-parameters
+	 * RFC 3551, Section 3: "[...] applications which need to define more
+	 * than 32 dynamic payload types MAY bind codes below 96, in which case
+	 * it is RECOMMENDED that unassigned payload type numbers be used
+	 * first". Updated by RFC 5761, Section 4: "[...] values in the range
+	 * 64-95 MUST NOT be used [to avoid conflicts with RTCP]". Summaries:
+	 * 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),
+		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
+	 * 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);
+	if (res != -1) {
+		return res;
+	}
+
+	return find_unused_payload_in_range(
+		codecs, MAX(ast_option_rtpptdynamic, 0), 20, static_RTP_PT);
 }
 
 /*!
@@ -1331,30 +1426,26 @@
 	struct ast_rtp_payload_type *new_type;
 
 	payload = find_static_payload_type(asterisk_format, format, code);
-	if (payload < 0) {
+
+	if (payload < 0 && (!asterisk_format || ast_option_rtpusedynamic)) {
 		return payload;
 	}
 
-	new_type = ast_rtp_engine_alloc_payload_type();
+	new_type = rtp_payload_type_alloc(format, payload, code, 1);
 	if (!new_type) {
 		return -1;
 	}
-	new_type->format = ao2_bump(format);
-	new_type->asterisk_format = asterisk_format;
-	new_type->rtp_code = code;
-	new_type->payload = payload;
-	new_type->primary_mapping = 1;
 
 	ast_rwlock_wrlock(&codecs->codecs_lock);
-	if (payload < AST_RTP_PT_FIRST_DYNAMIC
+	if (payload > -1 && (payload < AST_RTP_PT_FIRST_DYNAMIC
 		|| AST_VECTOR_SIZE(&codecs->payload_mapping_rx) <= payload
-		|| !AST_VECTOR_GET(&codecs->payload_mapping_rx, payload)) {
+		|| !AST_VECTOR_GET(&codecs->payload_mapping_rx, payload))) {
 		/*
 		 * The payload type is a static assignment
 		 * or our default dynamic position is available.
 		 */
-		rtp_codecs_payload_replace_rx(codecs, payload, new_type);
-	} else if (-1 < (payload = rtp_codecs_find_empty_dynamic_rx(codecs))
+               rtp_codecs_payload_replace_rx(codecs, payload, new_type);
+	} else if (-1 < (payload = find_unused_payload(codecs))
 		|| -1 < (payload = rtp_codecs_find_non_primary_dynamic_rx(codecs))) {
 		/*
 		 * We found the first available empty dynamic position
@@ -1370,7 +1461,8 @@
 		 *
 		 * I don't think this is really possible.
 		 */
-		ast_log(LOG_WARNING, "No dynamic RTP payload type values available!\n");
+		ast_log(LOG_WARNING, "No dynamic RTP payload type values available "
+			"for %s - %d!\n", format ? ast_format_get_name(format) : "", code);
 	}
 	ast_rwlock_unlock(&codecs->codecs_lock);
 
@@ -2293,95 +2385,47 @@
 	ast_rwlock_unlock(&mime_types_lock);
 }
 
-static void add_static_payload(int map, struct ast_format *format, int rtp_code)
+static void add_static_payload(int payload, struct ast_format *format, int rtp_code)
 {
-	int x;
 	struct ast_rtp_payload_type *type;
 
 	/*
 	 * ARRAY_LEN's result is cast to an int so 'map' is not autocast to a size_t,
 	 * which if negative would cause an assertion.
 	 */
-	ast_assert(map < (int)ARRAY_LEN(static_RTP_PT));
+	ast_assert(payload < (int)ARRAY_LEN(static_RTP_PT));
 
+	if (ast_option_rtpusedynamic && payload < 0) {
+		/*
+		 * We're going to build dynamic payloads dynamically. An RTP code is
+		 * required otherwise one will be dynamically allocated per instance.
+		 */
+		return;
+	}
+
+	/*
+	 * Either the given payload is truly a static type, or Asterisk is
+	 * globally storing the dynamic payloads in the static_RTP_PT object.
+	 */
 	ast_rwlock_wrlock(&static_RTP_PT_lock);
-	if (map < 0) {
-		/* find next available dynamic payload slot */
-		for (x = AST_RTP_PT_FIRST_DYNAMIC; x < AST_RTP_MAX_PT; ++x) {
-			if (!static_RTP_PT[x]) {
-				map = x;
-				break;
-			}
-		}
 
-		/* http://www.iana.org/assignments/rtp-parameters
-		 * RFC 3551, Section 3: "[...] applications which need to define more
-		 * than 32 dynamic payload types MAY bind codes below 96, in which case
-		 * it is RECOMMENDED that unassigned payload type numbers be used
-		 * first". Updated by RFC 5761, Section 4: "[...] values in the range
-		 * 64-95 MUST NOT be used [to avoid conflicts with RTCP]". Summaries:
-		 * 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
+	if (payload < 0) {
+		/*
+		 * This is a dynamic payload that will be stored globally,
+		 * so find the next available empty slot.
 		 */
-		if (map < 0) {
-			for (x = MAX(ast_option_rtpptdynamic, 35); x <= AST_RTP_PT_LAST_REASSIGN; ++x) {
-				if (!static_RTP_PT[x]) {
-					map = x;
-					break;
-				}
-			}
-		}
-		/* 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
-		 * 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.
-		 */
-		if (map < 0) {
-			for (x = MAX(ast_option_rtpptdynamic, 20); x < 35; ++x) {
-				if (!static_RTP_PT[x]) {
-					map = x;
-					break;
-				}
-			}
-		}
-		if (map < 0) {
-			for (x = MAX(ast_option_rtpptdynamic, 0); x < 20; ++x) {
-				if (!static_RTP_PT[x]) {
-					map = x;
-					break;
-				}
-			}
-		}
-
-		if (map < 0) {
-			if (format) {
-				ast_log(LOG_WARNING, "No Dynamic RTP mapping available for format %s\n",
-					ast_format_get_name(format));
-			} else {
-				ast_log(LOG_WARNING, "No Dynamic RTP mapping available for RTP code %d\n",
-					rtp_code);
-			}
-			ast_rwlock_unlock(&static_RTP_PT_lock);
+		payload = find_unused_payload(NULL);
+		if (payload < 0) {
+			ast_log(LOG_WARNING, "No dynamic RTP payload type values available "
+				"for %s - %d!\n", format ? ast_format_get_name(format) : "", rtp_code);
 			return;
 		}
 	}
 
-	type = ast_rtp_engine_alloc_payload_type();
+	type = rtp_payload_type_alloc(format, payload, rtp_code, 1);
 	if (type) {
-		if (format) {
-			ao2_ref(format, +1);
-			type->format = format;
-			type->asterisk_format = 1;
-		} else {
-			type->rtp_code = rtp_code;
-		}
-		type->payload = map;
-		type->primary_mapping = 1;
-		ao2_cleanup(static_RTP_PT[map]);
-		static_RTP_PT[map] = type;
+		ao2_cleanup(static_RTP_PT[payload]);
+		static_RTP_PT[payload] = type;
 	}
 	ast_rwlock_unlock(&static_RTP_PT_lock);
 }
@@ -2797,23 +2841,34 @@
 	add_static_payload(26, ast_format_jpeg, 0);
 	add_static_payload(31, ast_format_h261, 0);
 	add_static_payload(34, ast_format_h263, 0);
+
+	/*
+	 * Dynamic payload types - Even when dynamically assigning them we'll fall
+	 * back to using the statically declared values as the default number.
+	 */
+	add_static_payload(96, ast_format_slin192, 0);
 	add_static_payload(97, ast_format_ilbc, 0);
 	add_static_payload(98, ast_format_h263p, 0);
 	add_static_payload(99, ast_format_h264, 0);
+	add_static_payload(100, ast_format_vp8, 0);
 	add_static_payload(101, NULL, AST_RTP_DTMF);
 	add_static_payload(102, ast_format_siren7, 0);
 	add_static_payload(103, ast_format_h263p, 0);
 	add_static_payload(104, ast_format_mp4, 0);
 	add_static_payload(105, ast_format_t140_red, 0);   /* Real time text chat (with redundancy encoding) */
 	add_static_payload(106, ast_format_t140, 0);     /* Real time text chat */
+	add_static_payload(107, ast_format_opus, 0);
+
 	add_static_payload(110, ast_format_speex, 0);
 	add_static_payload(111, ast_format_g726, 0);
 	add_static_payload(112, ast_format_g726_aal2, 0);
+
 	add_static_payload(115, ast_format_siren14, 0);
 	add_static_payload(116, ast_format_g719, 0);
 	add_static_payload(117, ast_format_speex16, 0);
 	add_static_payload(118, ast_format_slin16, 0); /* 16 Khz signed linear */
 	add_static_payload(119, ast_format_speex32, 0);
+
 	add_static_payload(121, NULL, AST_RTP_CISCO_DTMF);   /* Must be type 121 */
 	add_static_payload(122, ast_format_slin12, 0);
 	add_static_payload(123, ast_format_slin24, 0);
@@ -2822,10 +2877,6 @@
 	add_static_payload(126, ast_format_slin48, 0);
 	add_static_payload(127, ast_format_slin96, 0);
 	/* payload types above 127 are not valid */
-	add_static_payload(96, ast_format_slin192, 0);
-	/* Opus and VP8 */
-	add_static_payload(100, ast_format_vp8, 0);
-	add_static_payload(107, ast_format_opus, 0);
 
 	return 0;
 }

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I7653465c5ebeaf968f1a1cc8f3f4f5c4321da7fc
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>



More information about the asterisk-code-review mailing list