<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/3722/">https://reviewboard.asterisk.org/r/3722/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 9th, 2014, 9:43 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/3722/diff/1/?file=62462#file62462line13461" style="color: black; font-weight: bold; text-decoration: underline;">/team/group/media_formats-reviewed-trunk/channels/chan_sip.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 enum sip_result add_sdp(struct sip_request *resp, struct sip_pvt *p, int oldsdp, int add_audio, int add_t38)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">13425</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span> - First what was requested by the calling channel</pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">13444</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span> - First what was requested by the calling channel</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">13445</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> - Then our mutually shared capabilities, determined previous in tmpcap</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">13426</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span> - Then preferences in order from sip.conf device config for this peer/user</pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">13446</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span> - Then preferences in order from sip.conf device config for this peer/user</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;">This may warrant mention in UPGRADE.txt. Not sure we want to touch UPGRADE.txt itself, or maybe start an UPGRADE-MF.txt (to avoid automerge conflicts).</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 think a note of what may have changed is appropriate. I'll add one in the commit.</pre>
<br />
<p>- Matt</p>
<br />
<p>On July 7th, 2014, 8:40 p.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.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated July 7, 2014, 8:40 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/ASTERISK-23957">ASTERISK-23957</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;">This patch is identical to r3703, except it tweaks up chan_sip such that the SDP_attribute_passthrough tests pass (more on that below).
The major changes to format_cap are:
* format_cap now has a method to get the preferred codec out of a capabilities structure by some type. This prevents the situation where the 'preferred codec' is not an audio codec (which can happen when format capabilities structures are copied, moved around, and perturbed).
* format_cap now explicitly prevents duplicate formats from being appended. A format is considered a duplicate if it has the same codec ID. This is a 'strict' interpretation of equivalence, as it means that a format that contains an attribute cannot coexist with another format of the same type. This was needed, however, to prevent capabilities structures from having duplicate video formats (some with attributes, some without), which the channel drivers cannot yet understand or process (and which is not strictly needed right now).
The chan_sip channel driver has been tweaked up a bit to make sure that when a capability is built out from an SDP, that capability (which is the jointcaps structure) is given preference when constructing an outgoing SDP. That helps ensure that adding a format to an outbound SDP gets the appropriate format w/ attributes, as opposed to the peer formats which would not have any attributes.
Note that since the jointcaps capability is guaranteed to be a subset of the peer's capabilities, this ends up being generally the same.</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;">The SDP_attribute_passthrough test can now pass, with some tweaks. The attributes are now added in a different order, but all attributes for speex, h263, and h264 are present.</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-trunk/res/res_pjsip_sdp_rtp.c <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/res/res_format_attr_silk.c <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/res/res_format_attr_opus.c <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/res/res_format_attr_h264.c <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/res/res_format_attr_h263.c <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/res/res_format_attr_celt.c <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/main/rtp_engine.c <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/main/format_cap.c <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/main/format.c <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/include/asterisk/rtp_engine.h <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/include/asterisk/format_cap.h <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/include/asterisk/format.h <span style="color: grey">(418167)</span></li>
<li>/team/group/media_formats-reviewed-trunk/channels/chan_sip.c <span style="color: grey">(418167)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3722/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>