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/

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/11@434614 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/main/channel.c b/main/channel.c
index 6ac350caaf8448352e30206e141df5ee798240da..f26f8652fa4218e654d13023db2e2b4646d5a4d8 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -5313,6 +5313,14 @@ static int set_format(struct ast_channel *chan,
        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);
 
        /* See if the underlying channel driver is capable of performing transcoding for us */

diff --git a/main/translate.c b/main/translate.c
index 224bcb73583f0056999af02fdbf6d5e9ec2e5ce2..944dc31e23b95a4eca012bf44403313f40de6437 100644 (file)
--- a/main/translate.c
+++ b/main/translate.c
@@ -1192,72 +1192,87 @@ int ast_translator_best_choice(struct ast_format_cap *dst_cap,
 {
        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) {
-                               ast_format_copy(&best, &tmp_fmt);
+                       if (AST_FORMAT_GET_TYPE(tmp_fmt.id) != AST_FORMAT_TYPE_AUDIO) {
                                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)) {
+
+                       if (!best.id
+                               || 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)