From 0376af95192b0e902e52c33f9230dc5ecc90be42 Mon Sep 17 00:00:00 2001
From: Joshua Colp <jcolp@digium.com>
Date: Wed, 14 Sep 2016 07:53:36 -0400
Subject: [PATCH] rtp: Only accept the first payload for a format in SDP.
When receiving an SDP offer with multiple payloads for
the same format we would generate an answer with the first
payload, but during the payload crossover operation
(to set the payloads for receiving) we would remove all
payloads but the last. This would result in incoming
traffic being matched against the wrong format and outgoing
traffic being sent using the wrong payload.
This change makes it so that once a format has a payload
number put into the mapping all subsequent ones are ignored.
This ensures there is only ever one payload in the mapping
and that it is the payload placed into the answer SDP.
ASTERISK-26365 #close
Change-Id: I1e8150860a3518cab36d00b1fab50f9352b64e60
---
main/rtp_engine.c | 95 +++++++++++++++++++++++++++++------------------
1 file changed, 59 insertions(+), 36 deletions(-)
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 0671374ef5..b91bc41157 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -753,18 +753,18 @@ static void rtp_codecs_payloads_copy_rx(struct ast_rtp_codecs *src, struct ast_r
/*!
* \internal
- * \brief Remove other matching payload mappings.
+ * \brief Determine if a type of payload is already present in mappings.
* \since 14.0.0
*
- * \param codecs Codecs that need tx mappings removed.
- * \param instance RTP instance to notify of any payloads removed.
+ * \param codecs Codecs to be checked for mappings.
* \param to_match Payload type object to compare against.
*
* \note It is assumed that codecs is write locked before calling.
*
- * \return Nothing
+ * \retval 0 not found
+ * \retval 1 found
*/
-static void payload_mapping_tx_remove_other_mappings(struct ast_rtp_codecs *codecs, struct ast_rtp_instance *instance, struct ast_rtp_payload_type *to_match)
+static int payload_mapping_tx_is_present(const struct ast_rtp_codecs *codecs, const struct ast_rtp_payload_type *to_match)
{
int idx;
struct ast_rtp_payload_type *current;
@@ -772,12 +772,18 @@ static void payload_mapping_tx_remove_other_mappings(struct ast_rtp_codecs *code
for (idx = 0; idx < AST_VECTOR_SIZE(&codecs->payload_mapping_tx); ++idx) {
current = AST_VECTOR_GET(&codecs->payload_mapping_tx, idx);
- if (!current || current == to_match) {
+ if (!current) {
continue;
}
+ if (current == to_match) {
+ /* The exact object is already in the mapping. */
+ return 1;
+ }
if (current->asterisk_format && to_match->asterisk_format) {
- if (ast_format_cmp(current->format, to_match->format) == AST_FORMAT_CMP_NOT_EQUAL) {
+ if (ast_format_get_codec_id(current->format) != ast_format_get_codec_id(to_match->format)) {
continue;
+ } else if (current->payload == to_match->payload) {
+ return 0;
}
} else if (!current->asterisk_format && !to_match->asterisk_format) {
if (current->rtp_code != to_match->rtp_code) {
@@ -787,13 +793,10 @@ static void payload_mapping_tx_remove_other_mappings(struct ast_rtp_codecs *code
continue;
}
- /* Remove other mapping */
- AST_VECTOR_REPLACE(&codecs->payload_mapping_tx, idx, NULL);
- ao2_ref(current, -1);
- if (instance && instance->engine && instance->engine->payload_set) {
- instance->engine->payload_set(instance, idx, 0, NULL, 0);
- }
+ return 1;
}
+
+ return 0;
}
/*!
@@ -833,13 +836,14 @@ static void rtp_codecs_payloads_copy_tx(struct ast_rtp_codecs *src, struct ast_r
if (instance && instance->engine && instance->engine->payload_set) {
instance->engine->payload_set(instance, idx, type->asterisk_format, type->format, type->rtp_code);
}
-
- payload_mapping_tx_remove_other_mappings(dest, instance, type);
}
}
void ast_rtp_codecs_payloads_copy(struct ast_rtp_codecs *src, struct ast_rtp_codecs *dest, struct ast_rtp_instance *instance)
{
+ int idx;
+ struct ast_rtp_payload_type *type;
+
ast_rwlock_wrlock(&dest->codecs_lock);
/* Deadlock avoidance because of held write lock. */
@@ -849,6 +853,17 @@ void ast_rtp_codecs_payloads_copy(struct ast_rtp_codecs *src, struct ast_rtp_cod
ast_rwlock_wrlock(&dest->codecs_lock);
}
+ /*
+ * This represents a completely new mapping of what the remote party is
+ * expecting for payloads, so we clear out the entire tx payload mapping
+ * vector and replace it.
+ */
+ for (idx = 0; idx < AST_VECTOR_SIZE(&dest->payload_mapping_tx); ++idx) {
+ type = AST_VECTOR_GET(&dest->payload_mapping_tx, idx);
+ ao2_t_cleanup(type, "destroying ast_rtp_codec tx mapping");
+ AST_VECTOR_REPLACE(&dest->payload_mapping_tx, idx, NULL);
+ }
+
rtp_codecs_payloads_copy_rx(src, dest, instance);
rtp_codecs_payloads_copy_tx(src, dest, instance);
dest->framing = src->framing;
@@ -921,18 +936,20 @@ void ast_rtp_codecs_payloads_set_m_type(struct ast_rtp_codecs *codecs, struct as
ast_rwlock_wrlock(&codecs->codecs_lock);
- if (payload < AST_VECTOR_SIZE(&codecs->payload_mapping_tx)) {
- ao2_t_cleanup(AST_VECTOR_GET(&codecs->payload_mapping_tx, payload),
- "cleaning up replaced tx payload type");
- }
- AST_VECTOR_REPLACE(&codecs->payload_mapping_tx, payload, new_type);
+ if (!payload_mapping_tx_is_present(codecs, new_type)) {
+ if (payload < AST_VECTOR_SIZE(&codecs->payload_mapping_tx)) {
+ ao2_t_cleanup(AST_VECTOR_GET(&codecs->payload_mapping_tx, payload),
+ "cleaning up replaced tx payload type");
+ }
+ AST_VECTOR_REPLACE(&codecs->payload_mapping_tx, payload, new_type);
- if (instance && instance->engine && instance->engine->payload_set) {
- instance->engine->payload_set(instance, payload, new_type->asterisk_format, new_type->format, new_type->rtp_code);
+ if (instance && instance->engine && instance->engine->payload_set) {
+ instance->engine->payload_set(instance, payload, new_type->asterisk_format, new_type->format, new_type->rtp_code);
+ }
+ } else {
+ ao2_ref(new_type, -1);
}
- payload_mapping_tx_remove_other_mappings(codecs, instance, new_type);
-
ast_rwlock_unlock(&codecs->codecs_lock);
}
@@ -995,17 +1012,20 @@ int ast_rtp_codecs_payloads_set_rtpmap_type_rate(struct ast_rtp_codecs *codecs,
new_type->format = ast_format_parse_sdp_fmtp(new_type->format, "");
}
- if (pt < AST_VECTOR_SIZE(&codecs->payload_mapping_tx)) {
- ao2_t_cleanup(AST_VECTOR_GET(&codecs->payload_mapping_tx, pt),
- "cleaning up replaced tx payload type");
- }
- AST_VECTOR_REPLACE(&codecs->payload_mapping_tx, pt, new_type);
+ if (!payload_mapping_tx_is_present(codecs, new_type)) {
+ if (pt < AST_VECTOR_SIZE(&codecs->payload_mapping_tx)) {
+ ao2_t_cleanup(AST_VECTOR_GET(&codecs->payload_mapping_tx, pt),
+ "cleaning up replaced tx payload type");
+ }
+ AST_VECTOR_REPLACE(&codecs->payload_mapping_tx, pt, new_type);
- if (instance && instance->engine && instance->engine->payload_set) {
- instance->engine->payload_set(instance, pt, new_type->asterisk_format, new_type->format, new_type->rtp_code);
+ if (instance && instance->engine && instance->engine->payload_set) {
+ instance->engine->payload_set(instance, pt, new_type->asterisk_format, new_type->format, new_type->rtp_code);
+ }
+ } else {
+ ao2_ref(new_type, -1);
}
- payload_mapping_tx_remove_other_mappings(codecs, instance, new_type);
break;
}
@@ -1088,11 +1108,14 @@ int ast_rtp_codecs_payload_replace_format(struct ast_rtp_codecs *codecs, int pay
type->primary_mapping = 1;
ast_rwlock_wrlock(&codecs->codecs_lock);
- if (payload < AST_VECTOR_SIZE(&codecs->payload_mapping_tx)) {
- ao2_cleanup(AST_VECTOR_GET(&codecs->payload_mapping_tx, payload));
+ if (!payload_mapping_tx_is_present(codecs, type)) {
+ if (payload < AST_VECTOR_SIZE(&codecs->payload_mapping_tx)) {
+ ao2_cleanup(AST_VECTOR_GET(&codecs->payload_mapping_tx, payload));
+ }
+ AST_VECTOR_REPLACE(&codecs->payload_mapping_tx, payload, type);
+ } else {
+ ao2_ref(type, -1);
}
- AST_VECTOR_REPLACE(&codecs->payload_mapping_tx, payload, type);
- payload_mapping_tx_remove_other_mappings(codecs, NULL, type);
ast_rwlock_unlock(&codecs->codecs_lock);
return 0;
--
GitLab