<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3665/">https://reviewboard.asterisk.org/r/3665/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 23rd, 2014, 12:10 a.m. CDT, <b>Corey Farrell</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3665/diff/3/?file=60232#file60232line280" style="color: black; font-weight: bold; text-decoration: underline;">/team/group/media_formats-reviewed/include/asterisk/format_cache.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">275</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * \retval 0 if the format is not a SLIN format</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">276</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * \retval 1 if the format is a SLIN format</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If SLIN is an acronym for signed, abbreviation for linear (S. lin), then it's "an" SLIN format. a/an is used based on the immediate next sound, like an FBI agent or a Federal Agent.
On the other hand maybe I've been pronouncing slin wrong and it actually sounds similar to slim?</pre>
</blockquote>
<p>On June 23rd, 2014, 9:32 a.m. CDT, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Boy, I have no idea! When I read 'SLIN' in my head, I always read it the full way - 'Signed Linear'. So 'a Signed Linear format' - in my head at any rate - is "sounds" correct.
But I'm fine with 'an' as well.
Anyone else have any objections?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'll just change it to 'an' :-)</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 23rd, 2014, 12:10 a.m. CDT, <b>Corey Farrell</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3665/diff/3/?file=60235#file60235line87" style="color: black; font-weight: bold; text-decoration: underline;">/team/group/media_formats-reviewed/main/codec.c</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int codec_cmp(void *obj, void *arg, int flags)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">87</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="n">right</span><span class="o">-></span><span class="n">sample_rate</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Why?</pre>
</blockquote>
<p>On June 23rd, 2014, 9:32 a.m. CDT, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Because there are times when a user wants to look up a codec by name only, and we currently don't have a mechanism to do that.
Take, for example, "Opus". If I perform a 'core show translation path opus', it will fail without this check. Without providing the sample rate, we don't get a hit on the codec. The fact that Opus only has a single sample rate doesn't matter. We can't really infer what we should provide the lookup from the CLI command either.
I think forcing the sample rate is a little pessimistic: 90% of the time, you don't need to provide a sample rate to find the codec you're looking for. If you do have to provide the sample rate, this comparison function will still work: it will match on the sample rate as well if you provide it (and if you provide a sample rate and there is no codec that matches, it will return NULL as well).</pre>
</blockquote>
<p>On June 23rd, 2014, 11:47 a.m. CDT, <b>Corey Farrell</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">What about situations where multiple sample rate's exist for a codec? This seems to be related to my question about completion for sample rate's from the CLI command.</pre>
</blockquote>
<p>On June 23rd, 2014, 11:52 a.m. CDT, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes, it is explicitly for that.
If a codec (such as slin) has multiple sample rates and you fail to specify what you want, it will return back the first one that has a matching name. You didn't specify what you wanted, and you got the first match :-P</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">For now, I'll put a BUGBUG here as well. We should review whether or not this is necessary when we deal with the BUGBUG in translate.c.</pre>
<br />
<p>- Matt</p>
<br />
<p>On June 23rd, 2014, 9:51 a.m. CDT, Matt Jordan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Corey Farrell and Joshua Colp.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated June 23, 2014, 9:51 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This patch includes all of Corey's fine work on r3625, more that he did in channel/rtp_engine/dsp, and enough work in format_cache/elsewhere to get Asterisk's core to compile, along with some improvements in translate.
With this patch, Asterisk (with very little loaded) should run and generally display the codec path translations. I'm still not convinced we're computing computational complexity correctly for everything - particularly translations provided by codec_resample - but the table produced matches Asterisk 11/12, so that's a good step.
Major changes made in this patch:
* Removed ast_best_codec, as it was a farce [1]. All channel drivers will now use the first codec listed in their configured set of codecs as their preferred codec.
* Formats now store their name, as it can differ from the codec. This now has the accessor ast_format_get_name; codecs get the new ast_format_get_codec_name. Similarly, formats can now be constructed either entirely from the codec, or from a codec + name.
* Updated the format_cache with the expected short-hand pointers to the cached formats.
* channel.c was updated. That's large. Note that this was done mostly by Corey Farrell
* Codecs can do an explicit name match without their sample rate. This is done to make it a bit easier for CLI commands to query codecs with singular but odd sample rates (looking at you Opus)
* CLI commands in translate.c should now mostly work. translate.c will now correctly register translation paths - previously, it used the passed in codecs, which did not contain the codec->id field.
[1] http://lists.digium.com/pipermail/asterisk-dev/2014-June/068133.html</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/team/group/media_formats-reviewed/tests/test_format_cache.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/res/res_pjsip_sdp_rtp.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/translate.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/slinfactory.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/rtp_engine.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/frame.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/format_cap.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/format_cache.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/format.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/dsp.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/core_unreal.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/codec_builtin.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/codec.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/channel.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/main/asterisk.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/include/asterisk/rtp_engine.h <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/include/asterisk/format_cache.h <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/include/asterisk/format.h <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/include/asterisk/channel.h <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/include/asterisk/astobj2.h <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/include/asterisk/_private.h <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_unistim.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_skinny.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_sip.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_phone.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_multicast_rtp.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_misdn.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_mgcp.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_jingle.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_iax2.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_h323.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/channels/chan_gtalk.c <span style="color: grey">(417074)</span></li>
<li>/team/group/media_formats-reviewed/addons/chan_ooh323.c <span style="color: grey">(417074)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3665/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>