<p>N A has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/19527">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">translate.c: Prefer better codecs upon translate ties.<br><br>If multiple codecs are available for the same<br>resource and the translation costs between<br>multiple codecs are the same, ties are<br>currently broken arbitrarily, which means a<br>lower quality codec would be used. This forces<br>Asterisk to explicitly use the higher quality<br>codec, ceteris paribus.<br><br>ASTERISK-29455<br><br>Change-Id: I4b7297e1baca7aac14fe4a3c7538e18e2dbe9fd6<br>---<br>A doc/UPGRADE-staging/translate.txt<br>M include/asterisk/codec.h<br>M main/codec.c<br>M main/codec_builtin.c<br>M main/translate.c<br>5 files changed, 85 insertions(+), 3 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/27/19527/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/doc/UPGRADE-staging/translate.txt b/doc/UPGRADE-staging/translate.txt</span><br><span>new file mode 100644</span><br><span>index 0000000..1657469</span><br><span>--- /dev/null</span><br><span>+++ b/doc/UPGRADE-staging/translate.txt</span><br><span>@@ -0,0 +1,5 @@</span><br><span style="color: hsl(120, 100%, 40%);">+Subject: translate.c</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+When setting up translation between two codecs the quality was not taken into account,</span><br><span style="color: hsl(120, 100%, 40%);">+resulting in suboptimal translation. The quality is now taken into account,</span><br><span style="color: hsl(120, 100%, 40%);">+which can reduce the number of translation steps required, and improve the resulting quality.</span><br><span>diff --git a/include/asterisk/codec.h b/include/asterisk/codec.h</span><br><span>index 79798ac..b5861fa 100644</span><br><span>--- a/include/asterisk/codec.h</span><br><span>+++ b/include/asterisk/codec.h</span><br><span>@@ -78,6 +78,8 @@</span><br><span> unsigned int smooth;</span><br><span> /*! \brief Flags to be passed to the smoother */</span><br><span> unsigned int smoother_flags;</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! \brief Format quality, on scale from 0 to 150 (100 is ulaw, the reference). This allows better format to be used, ceterus paribus. */</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int quality;</span><br><span> /*! \brief The module that registered this codec */</span><br><span> struct ast_module *mod;</span><br><span> };</span><br><span>diff --git a/main/codec.c b/main/codec.c</span><br><span>index 32350f1..757f242 100644</span><br><span>--- a/main/codec.c</span><br><span>+++ b/main/codec.c</span><br><span>@@ -142,7 +142,7 @@</span><br><span> "\tIt does not indicate anything about your configuration.\n");</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ast_cli(a->fd, "%8s %-5s %-12s %-16s %s\n","ID","TYPE","NAME","FORMAT","DESCRIPTION");</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_cli(a->fd, "%8s %-5s %-12s %-16s %7s %s\n","ID","TYPE","NAME","FORMAT","QUALITY", "DESCRIPTION");</span><br><span> ast_cli(a->fd, "------------------------------------------------------------------------------------------------\n");</span><br><span> </span><br><span> ao2_rdlock(codecs);</span><br><span>@@ -171,11 +171,12 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ast_cli(a->fd, "%8u %-5s %-12s %-16s (%s)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_cli(a->fd, "%8u %-5s %-12s %-16s %7d (%s)\n",</span><br><span> codec->external.id,</span><br><span> ast_codec_media_type2str(codec->external.type),</span><br><span> codec->external.name,</span><br><span> S_OR(codec->format_name, "no cached format"),</span><br><span style="color: hsl(120, 100%, 40%);">+ codec->external.quality,</span><br><span> codec->external.description);</span><br><span> }</span><br><span> </span><br><span>diff --git a/main/codec_builtin.c b/main/codec_builtin.c</span><br><span>index 5c460bf..665a4a2 100644</span><br><span>--- a/main/codec_builtin.c</span><br><span>+++ b/main/codec_builtin.c</span><br><span>@@ -105,6 +105,7 @@</span><br><span> .minimum_bytes = 20,</span><br><span> .samples_count = g723_samples,</span><br><span> .get_length = g723_length,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 20,</span><br><span> };</span><br><span> </span><br><span> static int codec2_samples(struct ast_frame *frame)</span><br><span>@@ -175,6 +176,7 @@</span><br><span> .samples_count = ulaw_samples,</span><br><span> .get_length = ulaw_length,</span><br><span> .smooth = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 100, /* We are the gold standard. */</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec alaw = {</span><br><span>@@ -189,6 +191,7 @@</span><br><span> .samples_count = ulaw_samples,</span><br><span> .get_length = ulaw_length,</span><br><span> .smooth = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 100, /* Just as good as ulaw */</span><br><span> };</span><br><span> </span><br><span> static int gsm_samples(struct ast_frame *frame)</span><br><span>@@ -213,6 +216,7 @@</span><br><span> .samples_count = gsm_samples,</span><br><span> .get_length = gsm_length,</span><br><span> .smooth = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 60,</span><br><span> };</span><br><span> </span><br><span> static int g726_samples(struct ast_frame *frame)</span><br><span>@@ -237,6 +241,7 @@</span><br><span> .samples_count = g726_samples,</span><br><span> .get_length = g726_length,</span><br><span> .smooth = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 85,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec g726aal2 = {</span><br><span>@@ -251,6 +256,7 @@</span><br><span> .samples_count = g726_samples,</span><br><span> .get_length = g726_length,</span><br><span> .smooth = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 85,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec adpcm = {</span><br><span>@@ -265,6 +271,7 @@</span><br><span> .samples_count = g726_samples,</span><br><span> .get_length = g726_length,</span><br><span> .smooth = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 80,</span><br><span> };</span><br><span> </span><br><span> static int slin_samples(struct ast_frame *frame)</span><br><span>@@ -290,6 +297,7 @@</span><br><span> .get_length = slin_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_BE | AST_SMOOTHER_FLAG_FORCED,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 115, /* Better than ulaw */</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec slin12 = {</span><br><span>@@ -305,6 +313,7 @@</span><br><span> .get_length = slin_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_BE | AST_SMOOTHER_FLAG_FORCED,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 116,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec slin16 = {</span><br><span>@@ -320,6 +329,7 @@</span><br><span> .get_length = slin_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_BE | AST_SMOOTHER_FLAG_FORCED,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 117,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec slin24 = {</span><br><span>@@ -335,6 +345,7 @@</span><br><span> .get_length = slin_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_BE | AST_SMOOTHER_FLAG_FORCED,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 118,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec slin32 = {</span><br><span>@@ -350,6 +361,7 @@</span><br><span> .get_length = slin_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_BE | AST_SMOOTHER_FLAG_FORCED,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 119,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec slin44 = {</span><br><span>@@ -365,6 +377,7 @@</span><br><span> .get_length = slin_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_BE | AST_SMOOTHER_FLAG_FORCED,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 120,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec slin48 = {</span><br><span>@@ -380,6 +393,7 @@</span><br><span> .get_length = slin_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_BE | AST_SMOOTHER_FLAG_FORCED,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 121,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec slin96 = {</span><br><span>@@ -395,6 +409,7 @@</span><br><span> .get_length = slin_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_BE | AST_SMOOTHER_FLAG_FORCED,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 122,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec slin192 = {</span><br><span>@@ -410,6 +425,7 @@</span><br><span> .get_length = slin_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_BE | AST_SMOOTHER_FLAG_FORCED,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 123,</span><br><span> };</span><br><span> </span><br><span> static int lpc10_samples(struct ast_frame *frame)</span><br><span>@@ -433,6 +449,7 @@</span><br><span> .minimum_bytes = 7,</span><br><span> .samples_count = lpc10_samples,</span><br><span> .smooth = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 25,</span><br><span> };</span><br><span> </span><br><span> static int g729_samples(struct ast_frame *frame)</span><br><span>@@ -458,6 +475,7 @@</span><br><span> .get_length = g729_length,</span><br><span> .smooth = 1,</span><br><span> .smoother_flags = AST_SMOOTHER_FLAG_G729,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 20,</span><br><span> };</span><br><span> </span><br><span> static unsigned char get_n_bits_at(unsigned char *data, int n, int bit)</span><br><span>@@ -584,6 +602,7 @@</span><br><span> .default_ms = 20,</span><br><span> .minimum_bytes = 10,</span><br><span> .samples_count = speex8_samples,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 40,</span><br><span> };</span><br><span> </span><br><span> static int speex16_samples(struct ast_frame *frame)</span><br><span>@@ -601,6 +620,7 @@</span><br><span> .default_ms = 20,</span><br><span> .minimum_bytes = 10,</span><br><span> .samples_count = speex16_samples,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 40,</span><br><span> };</span><br><span> </span><br><span> static int speex32_samples(struct ast_frame *frame)</span><br><span>@@ -618,6 +638,7 @@</span><br><span> .default_ms = 20,</span><br><span> .minimum_bytes = 10,</span><br><span> .samples_count = speex32_samples,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 40,</span><br><span> };</span><br><span> </span><br><span> static int ilbc_samples(struct ast_frame *frame)</span><br><span>@@ -641,6 +662,7 @@</span><br><span> .minimum_bytes = 38,</span><br><span> .samples_count = ilbc_samples,</span><br><span> .smooth = 0,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 45,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec g722 = {</span><br><span>@@ -655,6 +677,7 @@</span><br><span> .samples_count = g726_samples,</span><br><span> .get_length = g726_length,</span><br><span> .smooth = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 110, /* In theory, better than ulaw */</span><br><span> };</span><br><span> </span><br><span> static int siren7_samples(struct ast_frame *frame)</span><br><span>@@ -678,6 +701,7 @@</span><br><span> .minimum_bytes = 80,</span><br><span> .samples_count = siren7_samples,</span><br><span> .get_length = siren7_length,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 85,</span><br><span> };</span><br><span> </span><br><span> static int siren14_samples(struct ast_frame *frame)</span><br><span>@@ -701,6 +725,7 @@</span><br><span> .minimum_bytes = 120,</span><br><span> .samples_count = siren14_samples,</span><br><span> .get_length = siren14_length,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 90,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec testlaw = {</span><br><span>@@ -738,6 +763,7 @@</span><br><span> .minimum_bytes = 160,</span><br><span> .samples_count = g719_samples,</span><br><span> .get_length = g719_length,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 95,</span><br><span> };</span><br><span> </span><br><span> static int opus_samples(struct ast_frame *frame)</span><br><span>@@ -765,6 +791,7 @@</span><br><span> .default_ms = 20,</span><br><span> .samples_count = opus_samples,</span><br><span> .minimum_bytes = 10,</span><br><span style="color: hsl(120, 100%, 40%);">+ .quality = 50,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec jpeg = {</span><br><span>@@ -873,7 +900,7 @@</span><br><span> .maximum_ms = 100,</span><br><span> .default_ms = 20,</span><br><span> .minimum_bytes = 160,</span><br><span style="color: hsl(0, 100%, 40%);">- .samples_count = silk_samples</span><br><span style="color: hsl(120, 100%, 40%);">+ .samples_count = silk_samples,</span><br><span> };</span><br><span> </span><br><span> static struct ast_codec silk12 = {</span><br><span>diff --git a/main/translate.c b/main/translate.c</span><br><span>index 3101598..5d811be 100644</span><br><span>--- a/main/translate.c</span><br><span>+++ b/main/translate.c</span><br><span>@@ -1465,11 +1465,39 @@</span><br><span> beststeps = matrix_get(x, y)->multistep;</span><br><span> } else if (matrix_get(x, y)->table_cost == besttablecost</span><br><span> && matrix_get(x, y)->multistep == beststeps) {</span><br><span style="color: hsl(120, 100%, 40%);">+ int replace = 0;</span><br><span> unsigned int gap_selected = format_sample_rate_absdiff(best, bestdst);</span><br><span> unsigned int gap_current = format_sample_rate_absdiff(src, dst);</span><br><span> </span><br><span> if (gap_current < gap_selected) {</span><br><span> /* better than what we have so far */</span><br><span style="color: hsl(120, 100%, 40%);">+ replace = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (gap_current == gap_selected) {</span><br><span style="color: hsl(120, 100%, 40%);">+ int src_quality, best_quality;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_codec *src_codec, *best_codec;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ src_codec = ast_format_get_codec(src);</span><br><span style="color: hsl(120, 100%, 40%);">+ best_codec = ast_format_get_codec(best);</span><br><span style="color: hsl(120, 100%, 40%);">+ src_quality = src_codec->quality;</span><br><span style="color: hsl(120, 100%, 40%);">+ best_quality = best_codec->quality;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_cleanup(src_codec);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_cleanup(best_codec);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* We have a tie, so choose the format with the higher quality, if they differ. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (src_quality > best_quality) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Better than what we had before. */</span><br><span style="color: hsl(120, 100%, 40%);">+ replace = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_debug(2, "Tiebreaker: preferring format %s (%d) to %s (%d)\n", ast_format_get_name(src), src_quality,</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_format_get_name(best), best_quality);</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* This isn't necessarily indicative of a problem, but in reality this shouldn't really happen, unless</span><br><span style="color: hsl(120, 100%, 40%);">+ * there are 2 formats that are basically the same. */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_debug(1, "Completely ambiguous tie between formats %s and %s (quality %d): sticking with %s, but this is arbitrary\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_format_get_name(src), ast_format_get_name(best), best_quality, ast_format_get_name(best));</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (replace) {</span><br><span> ao2_replace(best, src);</span><br><span> ao2_replace(bestdst, dst);</span><br><span> besttablecost = matrix_get(x, y)->table_cost;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19527">change 19527</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/19527"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 18 </div>
<div style="display:none"> Gerrit-Change-Id: I4b7297e1baca7aac14fe4a3c7538e18e2dbe9fd6 </div>
<div style="display:none"> Gerrit-Change-Number: 19527 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: N A <asterisk@phreaknet.org> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>