rtp_engine.c: Fix performance issue with several channel drivers that use RTP.

ast_rtp_codecs_get_payload() gets called once or twice for every received
RTP frame so it would be nice to not allocate an ao2 object to then have
it destroyed shortly thereafter.  The ao2 object gets allocated only if
the payload type is not set by the channel driver as a negotiated value.
The issue affects chan_skinny, chan_unistim, chan_rtp, and chan_ooh323.

* Made static_RTP_PT[] an array of ao2 objects that
ast_rtp_codecs_get_payload() can return instead of an array of structs
that must be copied into a created ao2 object.

ASTERISK-25296 #close
Reported by: Richard Mudgett

Change-Id: Icb6de5cd90bfae07d44403a1352963db9109dac0

1 files changed, 57 insertions, 51 deletions

diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 587149aa3..1375897d6 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -229,7 +229,7 @@ static int mime_types_len = 0;
  * See http://www.iana.org/assignments/rtp-parameters for a list of
  * assigned values
  */
-static struct ast_rtp_payload_type static_RTP_PT[AST_RTP_MAX_PT];
+static struct ast_rtp_payload_type *static_RTP_PT[AST_RTP_MAX_PT];
 static ast_rwlock_t static_RTP_PT_lock;
 
 /*! \brief \ref stasis topic for RTP related messages */
@@ -651,23 +651,23 @@ void ast_rtp_codecs_payloads_set_m_type(struct ast_rtp_codecs *codecs, struct as
 		return;
 	}
 
-	new_type = ast_rtp_engine_alloc_payload_type();
+	ast_rwlock_rdlock(&static_RTP_PT_lock);
+	new_type = ao2_bump(static_RTP_PT[payload]);
+	ast_rwlock_unlock(&static_RTP_PT_lock);
 	if (!new_type) {
+		ast_debug(1, "Don't have a default tx payload type %d format for m type on %p\n",
+			payload, codecs);
 		return;
 	}
 
-	ast_rwlock_rdlock(&static_RTP_PT_lock);
+	ast_debug(1, "Setting tx payload type %d based on m type on %p\n",
+		payload, codecs);
+
 	ast_rwlock_wrlock(&codecs->codecs_lock);
+
 	if (payload < AST_VECTOR_SIZE(&codecs->payloads)) {
 		ao2_t_cleanup(AST_VECTOR_GET(&codecs->payloads, payload), "cleaning up replaced payload type");
 	}
-
-	new_type->asterisk_format = static_RTP_PT[payload].asterisk_format;
-	new_type->rtp_code = static_RTP_PT[payload].rtp_code;
-	new_type->payload = payload;
-	new_type->format = ao2_bump(static_RTP_PT[payload].format);
-
-	ast_debug(1, "Setting payload %d (%p) based on m type on %p\n", payload, new_type, codecs);
 	AST_VECTOR_REPLACE(&codecs->payloads, payload, new_type);
 
 	if (instance && instance->engine && instance->engine->payload_set) {
@@ -675,7 +675,6 @@ void ast_rtp_codecs_payloads_set_m_type(struct ast_rtp_codecs *codecs, struct as
 	}
 
 	ast_rwlock_unlock(&codecs->codecs_lock);
-	ast_rwlock_unlock(&static_RTP_PT_lock);
 }
 
 int ast_rtp_codecs_payloads_set_rtpmap_type_rate(struct ast_rtp_codecs *codecs, struct ast_rtp_instance *instance, int pt,
@@ -791,15 +790,8 @@ struct ast_rtp_payload_type *ast_rtp_codecs_get_payload(struct ast_rtp_codecs *c
 	ast_rwlock_unlock(&codecs->codecs_lock);
 
 	if (!type) {
-		type = ast_rtp_engine_alloc_payload_type();
-		if (!type) {
-			return NULL;
-		}
 		ast_rwlock_rdlock(&static_RTP_PT_lock);
-		type->asterisk_format = static_RTP_PT[payload].asterisk_format;
-		type->rtp_code = static_RTP_PT[payload].rtp_code;
-		type->payload = payload;
-		type->format = ao2_bump(static_RTP_PT[payload].format);
+		type = ao2_bump(static_RTP_PT[payload]);
 		ast_rwlock_unlock(&static_RTP_PT_lock);
 	}
 
@@ -810,17 +802,24 @@ int ast_rtp_codecs_payload_replace_format(struct ast_rtp_codecs *codecs, int pay
 {
 	struct ast_rtp_payload_type *type;
 
-	if (payload < 0 || payload >= AST_RTP_MAX_PT) {
+	if (payload < 0 || payload >= AST_RTP_MAX_PT || !format) {
+		return -1;
+	}
+
+	type = ast_rtp_engine_alloc_payload_type();
+	if (!type) {
 		return -1;
 	}
+	ao2_ref(format, +1);
+	type->format = format;
+	type->asterisk_format = 1;
+	type->payload = payload;
 
 	ast_rwlock_wrlock(&codecs->codecs_lock);
 	if (payload < AST_VECTOR_SIZE(&codecs->payloads)) {
-		type = AST_VECTOR_GET(&codecs->payloads, payload);
-		if (type && type->asterisk_format) {
-			ao2_replace(type->format, format);
-		}
+		ao2_cleanup(AST_VECTOR_GET(&codecs->payloads, payload));
 	}
+	AST_VECTOR_REPLACE(&codecs->payloads, payload, type);
 	ast_rwlock_unlock(&codecs->codecs_lock);
 
 	return 0;
@@ -923,12 +922,15 @@ int ast_rtp_codecs_payload_code(struct ast_rtp_codecs *codecs, int asterisk_form
 	if (payload < 0) {
 		ast_rwlock_rdlock(&static_RTP_PT_lock);
 		for (i = 0; i < AST_RTP_MAX_PT; i++) {
-			if (static_RTP_PT[i].asterisk_format && asterisk_format && format &&
-				(ast_format_cmp(format, static_RTP_PT[i].format) != AST_FORMAT_CMP_NOT_EQUAL)) {
+			if (!static_RTP_PT[i]) {
+				continue;
+			}
+			if (static_RTP_PT[i]->asterisk_format && asterisk_format && format &&
+				(ast_format_cmp(format, static_RTP_PT[i]->format) != AST_FORMAT_CMP_NOT_EQUAL)) {
 				payload = i;
 				break;
-			} else if (!static_RTP_PT[i].asterisk_format && !asterisk_format &&
-				(static_RTP_PT[i].rtp_code == code)) {
+			} else if (!static_RTP_PT[i]->asterisk_format && !asterisk_format &&
+				(static_RTP_PT[i]->rtp_code == code)) {
 				payload = i;
 				break;
 			}
@@ -1695,16 +1697,6 @@ void ast_rtp_dtls_cfg_free(struct ast_rtp_dtls_cfg *dtls_cfg)
 
 /*! \internal
  * \brief Small helper routine that cleans up entry i in
- * \c static_RTP_PT.
- */
-static void rtp_engine_static_RTP_PT_cleanup(int i)
-{
-	ao2_cleanup(static_RTP_PT[i].format);
-	memset(&static_RTP_PT[i], 0, sizeof(struct ast_rtp_payload_type));
-}
-
-/*! \internal
- * \brief Small helper routine that cleans up entry i in
  * \c ast_rtp_mime_types.
  */
 static void rtp_engine_mime_type_cleanup(int i)
@@ -1744,6 +1736,7 @@ static void set_next_mime_type(struct ast_format *format, int rtp_code, const ch
 static void add_static_payload(int map, struct ast_format *format, int rtp_code)
 {
 	int x;
+	struct ast_rtp_payload_type *type;
 
 	ast_assert(map < ARRAY_LEN(static_RTP_PT));
 
@@ -1751,24 +1744,36 @@ static void add_static_payload(int map, struct ast_format *format, int rtp_code)
 	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].asterisk_format && !static_RTP_PT[x].rtp_code) {
+			if (!static_RTP_PT[x]) {
 				map = x;
 				break;
 			}
 		}
 		if (map < 0) {
-			ast_log(LOG_WARNING, "No Dynamic RTP mapping available for format %s\n",
-				ast_format_get_name(format));
+			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);
 			return;
 		}
 	}
 
-	if (format) {
-		static_RTP_PT[map].asterisk_format = 1;
-		static_RTP_PT[map].format = ao2_bump(format);
-	} else {
-		static_RTP_PT[map].rtp_code = rtp_code;
+	type = ast_rtp_engine_alloc_payload_type();
+	if (type) {
+		if (format) {
+			ao2_ref(format, +1);
+			type->format = format;
+			type->asterisk_format = 1;
+		} else {
+			type->rtp_code = rtp_code;
+		}
+		type->payload = map;
+		ao2_cleanup(static_RTP_PT[map]);
+		static_RTP_PT[map] = type;
 	}
 	ast_rwlock_unlock(&static_RTP_PT_lock);
 }
@@ -1797,8 +1802,10 @@ int ast_rtp_engine_unload_format(struct ast_format *format)
 	ast_rwlock_wrlock(&static_RTP_PT_lock);
 	/* remove everything pertaining to this format id from the lists */
 	for (x = 0; x < AST_RTP_MAX_PT; x++) {
-		if (ast_format_cmp(static_RTP_PT[x].format, format) == AST_FORMAT_CMP_EQUAL) {
-			rtp_engine_static_RTP_PT_cleanup(x);
+		if (static_RTP_PT[x]
+			&& ast_format_cmp(static_RTP_PT[x]->format, format) == AST_FORMAT_CMP_EQUAL) {
+			ao2_ref(static_RTP_PT[x], -1);
+			static_RTP_PT[x] = NULL;
 		}
 	}
 	ast_rwlock_unlock(&static_RTP_PT_lock);
@@ -2087,9 +2094,8 @@ static void rtp_engine_shutdown(void)
 
 	ast_rwlock_wrlock(&static_RTP_PT_lock);
 	for (x = 0; x < AST_RTP_MAX_PT; x++) {
-		if (static_RTP_PT[x].format) {
-			rtp_engine_static_RTP_PT_cleanup(x);
-		}
+		ao2_cleanup(static_RTP_PT[x]);
+		static_RTP_PT[x] = NULL;
 	}
 	ast_rwlock_unlock(&static_RTP_PT_lock);