[Asterisk-code-review] res_pjsip_sdp_rtp: Preserve order of RTP codecs (asterisk[19])

[Florentin Mayer]

Florentin Mayer has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/17730 )


Change subject: res_pjsip_sdp_rtp: Preserve order of RTP codecs
......................................................................

res_pjsip_sdp_rtp: Preserve order of RTP codecs

The ast_rtp_codecs_payloads functions do not preserve the order in which
the payloads were specified on an incoming SDP media line. This leads to
a problem with the codec negotiation functionality, as the format
capabilities of the stream are extracted from the ast_rtp_codecs. This
commit moves the ast_rtp_codec to ast_format conversion to the place
where the order is still known.

ASTERISK-28863
ASTERISK-29320

Change-Id: I3aabcfed3f379c36654f59c1872c313d0cb57e25
---
M res/res_pjsip_sdp_rtp.c
1 file changed, 20 insertions(+), 10 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/30/17730/1

diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index ae200a7..cad2c01 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -313,13 +313,14 @@
 }
 
 static void get_codecs(struct ast_sip_session *session, const struct pjmedia_sdp_media *stream, struct ast_rtp_codecs *codecs,
-	struct ast_sip_session_media *session_media)
+	struct ast_sip_session_media *session_media, struct ast_format_cap *astformats)
 {
 	pjmedia_sdp_attr *attr;
 	pjmedia_sdp_rtpmap *rtpmap;
 	pjmedia_sdp_fmtp fmtp;
 	struct ast_format *format;
 	int i, num = 0, tel_event = 0;
+	int payload = 0;
 	char name[256];
 	char media[20];
 	char fmt_param[256];
@@ -329,10 +330,20 @@
 
 	ast_rtp_codecs_payloads_initialize(codecs);
 
+	ast_format_cap_remove_by_type(astformats, AST_MEDIA_TYPE_UNKNOWN);
+
 	/* Iterate through provided formats */
 	for (i = 0; i < stream->desc.fmt_count; ++i) {
 		/* The payload is kept as a string for things like t38 but for video it is always numerical */
-		ast_rtp_codecs_payloads_set_m_type(codecs, NULL, pj_strtoul(&stream->desc.fmt[i]));
+		payload = pj_strtoul(&stream->desc.fmt[i]);
+		ast_rtp_codecs_payloads_set_m_type(codecs, NULL, payload);
+
+		/* Fill the ast_format_cap struct in the correct order */
+		if ((format = ast_rtp_codecs_get_payload_format(codecs, payload))) {
+			ast_format_cap_append(astformats, format, 0);
+			ao2_ref(format, -1);
+		}
+
 		/* Look for the optional rtpmap attribute */
 		if (!(attr = pjmedia_sdp_media_find_attr2(stream, "rtpmap", &stream->desc.fmt[i]))) {
 			continue;
@@ -350,7 +361,7 @@
 
 		ast_copy_pj_str(media, (pj_str_t*)&stream->desc.media, sizeof(media));
 		ast_rtp_codecs_payloads_set_rtpmap_type_rate(codecs, NULL,
-			pj_strtoul(&stream->desc.fmt[i]), media, name, options, rtpmap->clock_rate);
+			payload, media, name, options, rtpmap->clock_rate);
 		/* Look for an optional associated fmtp attribute */
 		if (!(attr = pjmedia_sdp_media_find_attr2(stream, "fmtp", &rtpmap->pt))) {
 			continue;
@@ -370,9 +381,11 @@
 				format_parsed = ast_format_parse_sdp_fmtp(format, fmt_param);
 				if (format_parsed) {
 					ast_rtp_codecs_payload_replace_format(codecs, num, format_parsed);
+					ast_format_cap_append(astformats, format_parsed, 0);
 					ao2_ref(format_parsed, -1);
+				} else {
+					ast_format_cap_append(astformats, format, 0);
 				}
-
 				ao2_ref(format, -1);
 			}
 		}
@@ -398,6 +411,7 @@
 		unsigned long framing = pj_strtoul(pj_strltrim(&attr->value));
 		if (framing && session->endpoint->media.rtp.use_ptime) {
 			ast_rtp_codecs_set_framing(codecs, framing);
+			ast_format_cap_set_framing(astformats, framing);
 		}
 	}
 
@@ -442,7 +456,6 @@
 	struct ast_format_cap *incoming_call_offer_cap;
 	struct ast_format_cap *remote;
 	struct ast_rtp_codecs codecs = AST_RTP_CODECS_NULL_INIT;
-	int fmts = 0;
 	SCOPE_ENTER(1, "%s\n", ast_sip_session_get_name(session));
 
 
@@ -454,8 +467,7 @@
 	}
 
 	/* Get the peer's capabilities*/
-	get_codecs(session, stream, &codecs, session_media);
-	ast_rtp_codecs_payload_formats(&codecs, remote, &fmts);
+	get_codecs(session, stream, &codecs, session_media, remote);
 
 	incoming_call_offer_cap = ast_sip_session_create_joint_call_cap(
 		session, session_media->type, remote);
@@ -493,7 +505,6 @@
 	RAII_VAR(struct ast_format_cap *, joint, NULL, ao2_cleanup);
 	enum ast_media_type media_type = session_media->type;
 	struct ast_rtp_codecs codecs = AST_RTP_CODECS_NULL_INIT;
-	int fmts = 0;
 	int direct_media_enabled = !ast_sockaddr_isnull(&session_media->direct_media_addr) &&
 		ast_format_cap_count(session->direct_media_cap);
 	int dsp_features = 0;
@@ -516,8 +527,7 @@
 	}
 
 	/* get the capabilities on the peer */
-	get_codecs(session, stream, &codecs,  session_media);
-	ast_rtp_codecs_payload_formats(&codecs, peer, &fmts);
+	get_codecs(session, stream, &codecs, session_media, peer);
 
 	/* get the joint capabilities between peer and endpoint */
 	ast_format_cap_get_compatible(caps, peer, joint);

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/17730
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 19
Gerrit-Change-Id: I3aabcfed3f379c36654f59c1872c313d0cb57e25
Gerrit-Change-Number: 17730
Gerrit-PatchSet: 1
Gerrit-Owner: Florentin Mayer <f.mayer at commend.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20211217/58bb4f9b/attachment.html>