[Asterisk-code-review] rtp engine.c: Fix performance issue with several channel dri... (asterisk[master])
Richard Mudgett
asteriskteam at digium.com
Thu Jul 30 20:48:09 CDT 2015
Richard Mudgett has uploaded a new change for review.
https://gerrit.asterisk.org/1017
Change subject: rtp_engine.c: Fix performance issue with several channel drivers that use RTP.
......................................................................
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
---
M main/rtp_engine.c
1 file changed, 57 insertions(+), 51 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/17/1017/1
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 587149a..1375897 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -229,7 +229,7 @@
* 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 @@
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 @@
}
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 @@
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 @@
{
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 @@
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 @@
/*! \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 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 @@
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 @@
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 @@
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);
--
To view, visit https://gerrit.asterisk.org/1017
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icb6de5cd90bfae07d44403a1352963db9109dac0
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list