[Asterisk-code-review] chan_sip: Filter pass-through audio/video formats away, again. (asterisk[master])

[George Joseph]

George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15396 )

Change subject: chan_sip: Filter pass-through audio/video formats away, again.
......................................................................

chan_sip: Filter pass-through audio/video formats away, again.

Instead of looking for pass-through formats in the list of transcodable
formats (which is going to find nothing), go through the result which
is going to be the jointcaps of the tech_pvt of the channel. Finally,
only with that list, ast_format_cap_remove(.) is going to succeed.

This restores the behaviour of Asterisk 1.8. However, it does not fix
ASTERISK_29282 because that issue report is about chan_sip and PJSIP.
Here, only chan_sip is fixed because PJSIP does not even call
ast_rtp_instance_available_formats -> ast_translate_available_format.

Change-Id: Icade2366ac2b82935b95a9981678c987da2e8c34
---
M channels/chan_sip.c
M main/translate.c
2 files changed, 18 insertions(+), 32 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 2b34947..da0e0de 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -13623,10 +13623,6 @@
 
 		/* Check if we need audio in this call */
 		needaudio = ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_AUDIO);
-		if (!needaudio && p->outgoing_call) {
-			/* p->caps are added conditionally, see below "Finally our remain..." */
-			needaudio = ast_format_cap_has_type(p->caps, AST_MEDIA_TYPE_AUDIO);
-		}
 
 		/* Check if we need video in this call */
 		if ((ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_VIDEO)) && !p->novideo) {
@@ -13757,7 +13753,6 @@
 		/* Now, start adding audio codecs. These are added in this order:
 		   - First what was requested by the calling channel
 		   - Then our mutually shared capabilities, determined previous in tmpcap
-		   - Then preferences in order from sip.conf device config for this peer/user
 		*/
 
 
@@ -13801,27 +13796,6 @@
 			ao2_ref(tmp_fmt, -1);
 		}
 
-		/* Finally our remaining audio/video codecs */
-		for (x = 0; p->outgoing_call && x < ast_format_cap_count(p->caps); x++) {
-			tmp_fmt = ast_format_cap_get_format(p->caps, x);
-
-			if (ast_format_cap_iscompatible_format(alreadysent, tmp_fmt) != AST_FORMAT_CMP_NOT_EQUAL) {
-				ao2_ref(tmp_fmt, -1);
-				continue;
-			}
-
-			if (ast_format_get_type(tmp_fmt) == AST_MEDIA_TYPE_AUDIO) {
-				add_codec_to_sdp(p, tmp_fmt, &m_audio, &a_audio, debug, &min_audio_packet_size, &max_audio_packet_size);
-			} else if (needvideo && ast_format_get_type(tmp_fmt) == AST_MEDIA_TYPE_VIDEO) {
-				add_vcodec_to_sdp(p, tmp_fmt, &m_video, &a_video, debug, &min_video_packet_size);
-			} else if (needtext && ast_format_get_type(tmp_fmt) == AST_MEDIA_TYPE_TEXT) {
-				add_tcodec_to_sdp(p, tmp_fmt, &m_text, &a_text, debug, &min_text_packet_size);
-			}
-
-			ast_format_cap_append(alreadysent, tmp_fmt, 0);
-			ao2_ref(tmp_fmt, -1);
-		}
-
 		/* Now add DTMF RFC2833 telephony-event as a codec */
 		for (x = 1LL; x <= AST_RTP_MAX; x <<= 1) {
 			if (!(p->jointnoncodeccapability & x))
diff --git a/main/translate.c b/main/translate.c
index 6648931..a9665ae 100644
--- a/main/translate.c
+++ b/main/translate.c
@@ -1509,16 +1509,19 @@
 	struct ast_format_cap *result, struct ast_format *src_fmt,
 	enum ast_media_type type)
 {
-	int index, src_index = format2index(src_fmt);
+	int i;
+
+	if (ast_format_get_type(src_fmt) != type) {
+		return;
+	}
+
 	/* For a given source format, traverse the list of
 	   known formats to determine whether there exists
 	   a translation path from the source format to the
 	   destination format. */
-	for (index = 0; (src_index >= 0) && index < cur_max_index; index++) {
-		struct ast_codec *codec = index2codec(index);
-		RAII_VAR(struct ast_format *, fmt, ast_format_create(codec), ao2_cleanup);
-
-		ao2_ref(codec, -1);
+	for (i = ast_format_cap_count(result) - 1; 0 <= i; i--) {
+		int index, src_index;
+		struct ast_format *fmt = ast_format_cap_get_format(result, i);
 
 		if (ast_format_get_type(fmt) != type) {
 			continue;
@@ -1535,6 +1538,15 @@
 			continue;
 		}
 
+		/* if this is a pass-through format, not in the source,
+		   we cannot transcode. Therefore, remove it from the result */
+		src_index = format2index(src_fmt);
+		index = format2index(fmt);
+		if (src_index < 0 || index < 0) {
+			ast_format_cap_remove(result, fmt);
+			continue;
+		}
+
 		/* if we don't have a translation path from the src
 		   to this format, remove it from the result */
 		if (!matrix_get(src_index, index)->step) {

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Icade2366ac2b82935b95a9981678c987da2e8c34
Gerrit-Change-Number: 15396
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210223/59c46c92/attachment-0001.html>