<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/4000/">https://reviewboard.asterisk.org/r/4000/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 18th, 2014, 9:35 p.m. UTC, <b>Mark Michelson</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/4000/diff/1/?file=67396#file67396line259" style="color: black; font-weight: bold; text-decoration: underline;">/branches/13/res/res_pjsip_sdp_rtp.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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 set_caps(struct ast_sip_session *session, struct ast_sip_session_media *session_media,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">259</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ast_format_cap_<span class="hl">remove_by_type</span></span><span class="p"><span class="hl">(</span></span><span class="n">caps</span><span class="p">,</span> <span class="n">AST_MEDIA_TYPE_UNKNOWN</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">259</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ast_format_cap_<span class="hl">append_from_cap</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">session</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">req_</span>caps</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">joint</span></span><span class="p">,</span> <span class="n">AST_MEDIA_TYPE_UNKNOWN</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;">Since joint only has formats of type media_type, would specifying media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here?</pre>
</blockquote>
<p>On September 18th, 2014, 10:33 p.m. UTC, <b>rmudgett</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;">It's a case of six of one and half a dozen of another. It won't make any difference in this case since all formats will be appended anyway. It's a little more efficient to use the constant since the function has to test if it isn't UNKNOWN and then test to see if it is the specified type.
I'll leave it as is unless there is a stronger argument for changing it.</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;">Okay, that's good enough for me.</pre>
<br />
<p>- Mark</p>
<br />
<p>On September 18th, 2014, 6:26 p.m. UTC, rmudgett 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.</div>
<div>By rmudgett.</div>
<p style="color: grey;"><i>Updated Sept. 18, 2014, 6:26 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/AFS-162">AFS-162</a>
</div>
<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;">Outgoing PJSIP calls can result in non-negotiated formats listed in the channel's native formats if video formats are listed in the endpoint's configuration. The resulting call could then use a non-negotiated format resulting in one way audio.
* Simplified the update of session->req_caps in set_caps(). Why do something in five steps when only one is needed?</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
Called from D40 with g722 among other formats enabled to a Polycom that negotiates ulaw.
Before the patch, Asterisk would send g722 frames to the Polycom. The resulting call had one way audio because the Polycom does not understand g722.
After the patch, Asterisk sends ulaw frames to the Polycom.</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>/branches/13/res/res_pjsip_sdp_rtp.c <span style="color: grey">(423446)</span></li>
<li>/branches/13/channels/chan_pjsip.c <span style="color: grey">(423446)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4000/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>