[asterisk-commits] rmudgett: branch 11 r434614 - in /branches/11/main: channel.c translate.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Apr 10 11:25:18 CDT 2015


Author: rmudgett
Date: Fri Apr 10 11:25:13 2015
New Revision: 434614

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=434614
Log:
translate.c: Only select audio codecs to determine the best translation choice.

Given a source capability of h264 and ulaw, a destination capability of
h264 and g722 then ast_translator_best_choice() would pick h264 as the
best choice even though h264 is a video codec and Asterisk only supports
translation of audio codecs.  When the audio starts flowing, there are
warnings about a codec mismatch when the channel tries to write a frame to
the peer.

* Made ast_translator_best_choice() only select audio codecs.

* Restore a check in channel.c:set_format() lost after v1.8 to prevent
trying to set a non-audio codec.

This is an intermediate patch for a series of patches aimed at improving
translation path choices for ASTERISK-24841.

This patch is a complete enough fix for ASTERISK-21777 as the v11 version
of ast_translator_best_choice() does the same thing.  However, chan_sip.c
still somehow tries to call ast_codec_choose() which then calls
ast_best_codec() with a capability set that doesn't contain any audio
formats for the incoming call.  The remaining warning message seems to be
a benign transient.

ASTERISK-21777 #close
Reported by: Nick Ruggles

ASTERISK-24380 #close
Reported by: Matt Jordan

Review: https://reviewboard.asterisk.org/r/4605/

Modified:
    branches/11/main/channel.c
    branches/11/main/translate.c

Modified: branches/11/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/main/channel.c?view=diff&rev=434614&r1=434613&r2=434614
==============================================================================
--- branches/11/main/channel.c (original)
+++ branches/11/main/channel.c Fri Apr 10 11:25:13 2015
@@ -5312,6 +5312,14 @@
 	struct ast_format best_native_fmt;
 	int res;
 	char from[200], to[200];
+
+	if (!ast_format_cap_has_type(cap_set, AST_FORMAT_TYPE_AUDIO)) {
+		/*
+		 * Not setting any audio formats?
+		 * Assume a call without any sounds (video, text)
+		 */
+		return 0;
+	}
 
 	ast_best_codec(cap_set, &best_set_fmt);
 

Modified: branches/11/main/translate.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/main/translate.c?view=diff&rev=434614&r1=434613&r2=434614
==============================================================================
--- branches/11/main/translate.c (original)
+++ branches/11/main/translate.c Fri Apr 10 11:25:13 2015
@@ -1192,72 +1192,87 @@
 {
 	unsigned int besttablecost = INT_MAX;
 	unsigned int beststeps = INT_MAX;
+	struct ast_format cur_dst;
+	struct ast_format cur_src;
 	struct ast_format best;
 	struct ast_format bestdst;
-	struct ast_format_cap *joint_cap = ast_format_cap_joint(dst_cap, src_cap);
+	struct ast_format_cap *joint_cap;
+
 	ast_format_clear(&best);
-	ast_format_clear(&bestdst);
-
+
+	joint_cap = ast_format_cap_joint(dst_cap, src_cap);
 	if (joint_cap) { /* yes, pick one and return */
 		struct ast_format tmp_fmt;
+
 		ast_format_cap_iter_start(joint_cap);
 		while (!ast_format_cap_iter_next(joint_cap, &tmp_fmt)) {
-			/* We are guaranteed to find one common format. */
-			if (!best.id) {
+			if (AST_FORMAT_GET_TYPE(tmp_fmt.id) != AST_FORMAT_TYPE_AUDIO) {
+				continue;
+			}
+
+			if (!best.id
+				|| ast_format_rate(&best) < ast_format_rate(&tmp_fmt)) {
 				ast_format_copy(&best, &tmp_fmt);
-				continue;
-			}
-			/* If there are multiple common formats, pick the one with the highest sample rate */
-			if (ast_format_rate(&best) < ast_format_rate(&tmp_fmt)) {
-				ast_format_copy(&best, &tmp_fmt);
-				continue;
-			}
-
+			}
 		}
 		ast_format_cap_iter_end(joint_cap);
-
-		/* We are done, this is a common format to both. */
-		ast_format_copy(dst_fmt_out, &best);
-		ast_format_copy(src_fmt_out, &best);
 		ast_format_cap_destroy(joint_cap);
-		return 0;
-	} else {      /* No, we will need to translate */
-		struct ast_format cur_dst;
-		struct ast_format cur_src;
-		AST_RWLIST_RDLOCK(&translators);
-
-		ast_format_cap_iter_start(dst_cap);
-		while (!ast_format_cap_iter_next(dst_cap, &cur_dst)) {
-			ast_format_cap_iter_start(src_cap);
-			while (!ast_format_cap_iter_next(src_cap, &cur_src)) {
-				int x = format2index(cur_src.id);
-				int y = format2index(cur_dst.id);
-				if (x < 0 || y < 0) {
-					continue;
-				}
-				if (!matrix_get(x, y) || !(matrix_get(x, y)->step)) {
-					continue;
-				}
-				if (((matrix_get(x, y)->table_cost < besttablecost) || (matrix_get(x, y)->multistep < beststeps))) {
-					/* better than what we have so far */
-					ast_format_copy(&best, &cur_src);
-					ast_format_copy(&bestdst, &cur_dst);
-					besttablecost = matrix_get(x, y)->table_cost;
-					beststeps = matrix_get(x, y)->multistep;
-				}
-			}
-			ast_format_cap_iter_end(src_cap);
-		}
-
-		ast_format_cap_iter_end(dst_cap);
-		AST_RWLIST_UNLOCK(&translators);
+
 		if (best.id) {
-			ast_format_copy(dst_fmt_out, &bestdst);
+			/* We are done, this is a common format to both. */
+			ast_format_copy(dst_fmt_out, &best);
 			ast_format_copy(src_fmt_out, &best);
 			return 0;
 		}
+	}
+
+	/* No, we will need to translate */
+	ast_format_clear(&bestdst);
+
+	AST_RWLIST_RDLOCK(&translators);
+	ast_format_cap_iter_start(dst_cap);
+	while (!ast_format_cap_iter_next(dst_cap, &cur_dst)) {
+		if (AST_FORMAT_GET_TYPE(cur_dst.id) != AST_FORMAT_TYPE_AUDIO) {
+			continue;
+		}
+
+		ast_format_cap_iter_start(src_cap);
+		while (!ast_format_cap_iter_next(src_cap, &cur_src)) {
+			int x;
+			int y;
+
+			if (AST_FORMAT_GET_TYPE(cur_src.id) != AST_FORMAT_TYPE_AUDIO) {
+				continue;
+			}
+
+			x = format2index(cur_src.id);
+			y = format2index(cur_dst.id);
+			if (x < 0 || y < 0) {
+				continue;
+			}
+			if (!matrix_get(x, y) || !(matrix_get(x, y)->step)) {
+				continue;
+			}
+			if (matrix_get(x, y)->table_cost < besttablecost
+				|| matrix_get(x, y)->multistep < beststeps) {
+				/* better than what we have so far */
+				ast_format_copy(&best, &cur_src);
+				ast_format_copy(&bestdst, &cur_dst);
+				besttablecost = matrix_get(x, y)->table_cost;
+				beststeps = matrix_get(x, y)->multistep;
+			}
+		}
+		ast_format_cap_iter_end(src_cap);
+	}
+	ast_format_cap_iter_end(dst_cap);
+	AST_RWLIST_UNLOCK(&translators);
+
+	if (!best.id) {
 		return -1;
 	}
+	ast_format_copy(dst_fmt_out, &bestdst);
+	ast_format_copy(src_fmt_out, &best);
+	return 0;
 }
 
 unsigned int ast_translate_path_steps(struct ast_format *dst_format, struct ast_format *src_format)




More information about the asterisk-commits mailing list