<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/2278/">https://reviewboard.asterisk.org/r/2278/</a>
</td>
</tr>
</table>
<br />
<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 review was geared towards looking at the code itself. I did not look over the massive new blocks of XML.</pre>
<br />
<div>
<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/2278/diff/1/?file=32927#file32927line740" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/config_options.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 internal_type_init(struct aco_type *type)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">657</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl"> </span><span class="k"><span class="hl">for</span></span><span class="hl"> </span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">x</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span><span class="hl"> </span><span class="mi"><span class="hl">0</span></span><span class="p"><span class="hl">;</span></span> <span class="n">info</span><span class="o">-></span><span class="n">files</span><span class="p">[</span><span class="n">x</span><span class="p"><span class="hl">];</span></span><span class="hl"> </span><span class="n"><span class="hl">x</span></span><span class="o"><span class="hl">++</span></span><span class="p">)</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">740</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl"> </span><span class="k"><span class="hl">while</span></span><span class="hl"> </span><span class="p"><span class="hl">((</span></span><span class="n"><span class="hl">file</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span> <span class="n">info</span><span class="o">-></span><span class="n">files</span><span class="p">[</span><span class="n">x</span><span class="o"><span class="hl">++</span></span><span class="p"><span class="hl">])</span>)</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">658</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl"> </span><span class="k"><span class="hl">for</span></span><span class="hl"> </span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">y</span></span> <span class="o">=</span> <span class="mi"><span class="hl">0</span></span><span class="p"><span class="hl">;</span></span><span class="hl"> </span><span class="n"><span class="hl">info</span></span><span class="o"><span class="hl">-></span></span><span class="n">file<span class="hl">s</span></span><span class="p"><span class="hl">[</span></span><span class="n"><span class="hl">x</span></span><span class="p"><span class="hl">]</span></span><span class="o">-></span><span class="n">types</span><span class="p">[</span><span class="n">y</span><span class="p"><span class="hl">];</span></span><span class="hl"> </span><span class="n"><span class="hl">y</span></span><span class="o"><span class="hl">++</span></span><span class="p">)</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">741</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl"> </span><span class="k"><span class="hl">while</span></span><span class="hl"> </span><span class="p"><span class="hl">((</span></span><span class="n"><span class="hl">type</span></span> <span class="o">=</span> <span class="n">file</span><span class="o">-></span><span class="n">types</span><span class="p">[</span><span class="n">y</span><span class="o"><span class="hl">++</span></span><span class="p"><span class="hl">])</span>)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Probably need to set y to zero between these whiles</pre>
</div>
<br />
<div>
<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/2278/diff/1/?file=32927#file32927line1010" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/config_options.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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void cli_show_modules(struct ast_cli_args *a)</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">1010</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">it_items</span> <span class="o">=</span> <span class="n">ao2_iterator_init</span><span class="p">(</span><span class="n">xmldocs</span><span class="p">,</span> <span class="mi">0</span><span class="p">);</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">1011</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">ao2_container_count</span><span class="p">(</span><span class="n">it_items</span><span class="p">.</span><span class="n">c</span><span class="p">)</span> <span class="o">==</span> <span class="mi">0</span><span class="p">)</span> <span class="p">{</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">1012</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_cli</span><span class="p">(</span><span class="n">a</span><span class="o">-></span><span class="n">fd</span><span class="p">,</span> <span class="s">"No modules found.</span><span class="se">\n</span><span class="s">"</span><span class="p">);</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">1013</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ao2_iterator_destroy</span><span class="p">(</span><span class="o">&</span><span class="n">it_items</span><span class="p">);</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">1014</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span><span class="p">;</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">1015</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This seems a touch odd. Why create and destroy an iterator when you can just use ao2_container_count(xmldocs) to determine if you should even create an iterator?</pre>
</div>
<br />
<div>
<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/2278/diff/1/?file=32927#file32927line1018" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/config_options.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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void cli_show_modules(struct ast_cli_args *a)</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">1018</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">while</span> <span class="p">((</span><span class="n">item</span> <span class="o">=</span> <span class="n">ao2_iterator_next</span><span class="p">(</span><span class="o">&</span><span class="n">it_items</span><span class="p">)))</span> <span class="p">{</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">1019</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_cli</span><span class="p">(</span><span class="n">a</span><span class="o">-></span><span class="n">fd</span><span class="p">,</span> <span class="s">"</span><span class="se">\t</span><span class="s">%s</span><span class="se">\n</span><span class="s">"</span><span class="p">,</span> <span class="n">item</span><span class="o">-></span><span class="n">name</span><span class="p">);</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">1020</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Have you tested that RAII_VAR is working like you would expect it to with these items? I have this feeling that since item is scoped to the function, it's not getting properly cleaned up after each iteration of this while loop.</pre>
</div>
<br />
<p>- Mark</p>
<br />
<p>On January 24th, 2013, 9:30 a.m., Matt Jordan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/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 Matt Jordan.</div>
<p style="color: grey;"><i>Updated Jan. 24, 2013, 9:30 a.m.</i></p>
<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 is a continuation of Terry's work at https://reviewboard.asterisk.org/r/2058/. It is pretty much the same, but with some additional cleanup that Terry noted on the review.
* The CLI commands have been collapsed down to a single command - 'config show help'. This models itself off of the way (I think) users view configuration data - as a hierarchy of modules, types (the semantic meaning of the different context possibilities in a conf file), and the options for those types.
* Some things move around to better separate concerns between xmldoc and config_options
* Added default values and tied the type information to the config options
* Lots and lots of documentation cleanup
I have a feeling we'll find more things to tweak in this - both in terms of documentation and the things we'd like to show in the output of the CLI command. Given that this won't go into 11, we have some time for 12 to review the output of this and tweak it.
I'm planning on writing the XSLT for the schema and push the documentation to a 'test area' on the wiki. That should help find other problems in the current documentation.</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;">Lots of typing 'config show help'. Some sample outputs:
*CLI> config show help
The following modules have configuration information:
udptl
chan_motif
app_confbridge
res_xmpp
app_skel
named_acl
*CLI> config show help app_confbridge
Conference Bridge Application
Configuration option types for app_confbridge:
global: Unused, but reserved.
user_profile: A named profile to apply to specific callers.
bridge_profile: A named profile to apply to specific bridges.
menu: A conference user menu
*CLI> config show help app_confbridge user_profile
user_profile: [category !~ /^general$/ matchfield: type = user]
A named profile to apply to specific callers.
Callers in a ConfBridge have a profile associated with them that determine
their options. A configuration section is determined to be a user_profile
when the 'type' parameter has a value of 'user'.
type: Define this configuration category as a user profile.
admin: Sets if the user is an admin or not
marked: Sets if this is a marked user or not
startmuted: Sets if all users should start out muted
music_on_hold_when_e: Play MOH when user is alone or waiting on a marked user
quiet: Silence enter/leave prompts and user intros for this user
announce_user_count: Sets if the number of users should be announced to the user
announce_user_count_: Announce user count to all the other users when this user joins
announce_only_user: Announce to a user when they join an empty conference
wait_marked: Sets if the user must wait for a marked user to enter before joining a confere
end_marked: Kick the user from the conference when the last marked user leaves
talk_detection_event: Set whether or not notifications of when a user begins and ends talking should
dtmf_passthrough: Sets whether or not DTMF should pass through the conference
announce_join_leave: Prompt user for their name when joining a conference and play it to the confer
pin: Sets a PIN the user must enter before joining the conference
music_on_hold_class: The MOH class to use for this user
announcement: Sound file to play to the user when they join a conference
denoise: Apply a denoise filter to the audio before mixing
dsp_drop_silence: Drop what Asterisk detects as silence from audio sent to the bridge
dsp_silence_threshol: The number ofmilliseconds of detected silence necessary to trigger silence det
dsp_talking_threshol: The number of milliseconds of detected non-silence necessary to triger talk de
jitterbuffer: Place a jitter buffer on the user's audio stream before audio mixing is perfor
template: When using the CONFBRIDGE dialplan function, use a user profile as a template
*CLI> config show help app_confbridge user_profile dsp_silence_threshold
[user_profile]
dsp_silence_threshold = [Unsigned Integer] (Default: 2500) (Regex: false)
The number of milliseconds of detected silence necessary to trigger silence
detection
The time in milliseconds of sound falling within the what the dsp has
established as baseline silence before a user is considered be silent. This
value affects several operations and should not be changed unless the impact
on call quality is fully understood.
What this value affects internally:
1. When talk detection AMI events are enabled, this value determines when
the user has stopped talking after a period of talking. If this value is
set too low AMI events indicating the user has stopped talking may get falsely
sent out when the user briefly pauses during mid sentence.
2. The drop_silence option depends on this value to determine when the
user's audio should begin to be dropped from the conference bridge after
the user stops talking. If this value is set too low the user's audio stream
may sound choppy to the other participants. This is caused by the user
transitioning constantly from silence to talking during mid sentence.
The best way to approach this option is to set it slightly above the maximum
amount of ms of silence a user may generate during natural speech.
By default this value is 2500ms. Valid values are 1 through 2^31.
</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>/trunk/Makefile <span style="color: grey">(380042)</span></li>
<li>/trunk/apps/app_skel.c <span style="color: grey">(380042)</span></li>
<li>/trunk/apps/confbridge/conf_config_parser.c <span style="color: grey">(380042)</span></li>
<li>/trunk/channels/chan_motif.c <span style="color: grey">(380042)</span></li>
<li>/trunk/configs/motif.conf.sample <span style="color: grey">(380042)</span></li>
<li>/trunk/configs/xmpp.conf.sample <span style="color: grey">(380042)</span></li>
<li>/trunk/doc/appdocsxml.dtd <span style="color: grey">(380042)</span></li>
<li>/trunk/include/asterisk/_private.h <span style="color: grey">(380042)</span></li>
<li>/trunk/include/asterisk/config_options.h <span style="color: grey">(380042)</span></li>
<li>/trunk/include/asterisk/xml.h <span style="color: grey">(380042)</span></li>
<li>/trunk/include/asterisk/xmldoc.h <span style="color: grey">(380042)</span></li>
<li>/trunk/main/asterisk.c <span style="color: grey">(380042)</span></li>
<li>/trunk/main/config_options.c <span style="color: grey">(380042)</span></li>
<li>/trunk/main/named_acl.c <span style="color: grey">(380042)</span></li>
<li>/trunk/main/udptl.c <span style="color: grey">(380042)</span></li>
<li>/trunk/main/xml.c <span style="color: grey">(380042)</span></li>
<li>/trunk/main/xmldoc.c <span style="color: grey">(380042)</span></li>
<li>/trunk/res/res_xmpp.c <span style="color: grey">(380042)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2278/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>