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

1 files changed, 59 insertions, 36 deletions

diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 0671374ef..b91bc4115 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;