<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/2385/">https://reviewboard.asterisk.org/r/2385/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 5th, 2013, 4:17 a.m. EDT, <b>wdoekes</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;">Looks good to me. Thanks for fixing this eyesore.
I only wonder if this needs a mention somewhere. Perhaps a "BEWARE: please look over your sip.conf general settings. They may properly override the defaults now" in the commit message.
P.S. auto_* still trumps the other nat options, but only if they're specified. So a config like this will have auto_force_rport=yes comedia=yes
nat=auto_force_rport
nat=force_rport,comedia
But that could be considered a different bug.</pre>
</blockquote>
<p>On April 5th, 2013, 9:08 a.m. EDT, <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;">I think a note in an UPGRADE file wouldn't hurt :-)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Sounds good to me on mentioning this fix somewhere. The thought did go through my mind that some people may be surprised when their settings are now actually working. UPGRADE sounds good to me.
If I understand the comment about auto_* setting trumping the other nat options, I think it is by design if I understand the comment in the sample config correctly:
"... If one of the "auto" settings
; is used in conjunction with its non-auto counterpart (nat=comedia,auto_comedia), then
; the non-auto option will be ignored."</pre>
<br />
<p>- Michael</p>
<br />
<p>On April 4th, 2013, 7:59 p.m. EDT, Michael Young 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.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Michael Young.</div>
<p style="color: grey;"><i>Updated April 4, 2013, 7:59 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-21225">ASTERISK-21225</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;">The initial report was that the "nat" setting in the [general] section was not having any effect in overriding the default setting. Upon confirming this and checking into what was happening, it looks like other default settings would not be overridden as well.
This patch works similar to what occurs in build_peer(). We create a temporary ast_flags structure and using the mask, we override the default global options with whatever is set in the [general] section.
A different patch for 1.8 will be posted for review as well.</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;">Tested on my dev boxes.
Reporter also tested this patch on his machines and confirmed that everything is working properly. He also noted that the "directmedia" settings are now being set properly as a result of this patch.</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/11/channels/chan_sip.c <span style="color: grey">(384804)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2385/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>