[Asterisk-code-review] rtp: Only accept the first payload for a format in SDP. (asterisk[14.0])
Joshua Colp
asteriskteam at digium.com
Tue Sep 20 10:03:15 CDT 2016
Joshua Colp has submitted this change and it was merged.
Change subject: rtp: Only accept the first payload for a format in SDP.
......................................................................
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
---
M main/rtp_engine.c
1 file changed, 59 insertions(+), 36 deletions(-)
Approvals:
George Joseph: Looks good to me, but someone else must approve
Richard Mudgett: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Verified
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index c2b1c8c..6e1440c 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -747,18 +747,18 @@
/*!
* \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;
@@ -766,12 +766,18 @@
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) {
@@ -781,13 +787,10 @@
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;
}
/*!
@@ -827,13 +830,14 @@
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. */
@@ -841,6 +845,17 @@
ast_rwlock_unlock(&dest->codecs_lock);
sched_yield();
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);
@@ -915,17 +930,19 @@
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);
}
@@ -989,17 +1006,20 @@
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;
}
@@ -1082,11 +1102,14 @@
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;
--
To view, visit https://gerrit.asterisk.org/3895
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1e8150860a3518cab36d00b1fab50f9352b64e60
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: 14.0
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list