<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/2971/">https://reviewboard.asterisk.org/r/2971/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 30th, 2013, 6:36 p.m. UTC, <b>rmudgett</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/2971/diff/1/?file=47672#file47672line1883" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/apps/confbridge/conf_config_parser.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">1883</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="n">memcpy</span><span class="p">(</span><span class="n">new_menu_action</span><span class="p">,</span> <span class="n">menu_action</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span><span class="o">*</span><span class="n">new_menu_action</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">1884</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="n">AST_LIST_INSERT_HEAD</span><span class="p">(</span><span class="o">&</span><span class="n">dst</span><span class="o">-></span><span class="n">actions</span><span class="p">,</span> <span class="n">new_menu_action</span><span class="p">,</span> <span class="n">action</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;">This looks like a pre-existing bug.
The order of the DTMF actions is reversed by this copy function.</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;">Switched to INSERT_TAIL</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 30th, 2013, 6:36 p.m. UTC, <b>rmudgett</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/2971/diff/1/?file=47672#file47672line2246" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/apps/confbridge/conf_config_parser.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; ">int conf_find_menu_entry_by_sequence(const char *dtmf_sequence, struct conf_menu *menu, struct conf_menu_entry *result)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2154</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="p">(</span><span class="n">menu</span> <span class="o">=</span> <span class="n">menu_find</span><span class="p">(</span><span class="n">cfg</span><span class="o">-></span><span class="n">menus</span><span class="p">,</span> <span class="n">menu_name</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">2228</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">created_menu</span> <span class="o">=</span> <span class="n">menu_alloc</span><span class="p">(</span><span class="n">ast_strlen_zero</span><span class="p">(</span><span class="n">menu_profile_name</span><span class="p">)</span> <span class="o">?</span> <span class="n">DEFAULT_MENU</span> <span class="o">:</span> <span class="n">menu_profile_name</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;">You don't need to copy the menu into a new struct. The purpose of this function is to install the DTMF hooks of the selected menu profile into the bridge features structure.</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;">So in retrospect, I guess it is safe enough to just use a reference for menus created through configuration given how the config framework works. The referenced menus will persist without being editable and they'll simply be replaced in the list.
However, I probably do still need to have a copy of the menu stored in the channel datastore for the reasons I mentioned before. If the menu in the datastore can be manipulated by external AMI setting dialplan functions, that might lead to awkward behavior.</pre>
<br />
<p>- jrose</p>
<br />
<p>On October 29th, 2013, 7:10 p.m. UTC, jrose 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, Matt Jordan, Mark Michelson, and rmudgett.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated Oct. 29, 2013, 7:10 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-22760">ASTERISK-22760</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;">Adds the ability to dynamically create menus using the CONFBRIDGE dialplan function. This includes the ability to use the existing options as well as to specify templates with the 'template' option.
I suppose it's worth mentioning that there is one minor bit of awkwardness. Both the channel datastore and the conference user will end up with an allocation of the menu. This is probably for the best for a couple reasons though. For instance, the CONFBRIDGE function can modify the contents of the datastore allocated menu while the user is still in the conference if it is set via AMI. We probably don't want the menu to be modified in place during this time (even if it would be cool to be able to edit a menu while the conference is in progress).</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 following extensions were tested for successful entry with the expected options as well as not having any obvious memory leaks:
exten => 6060,1,NoOp
exten => 6060,n,Set(CONFBRIDGE(menu,*)=playback_and_continue(conf-usermenu))
exten => 6060,n,Set(CONFBRIDGE(menu,0)=leave_conference)
exten => 6060,n,Set(CONFBRIDGE(menu,*9)=increase_volume)
exten => 6060,n,Set(CONFBRIDGE(menu,*7)=decrease_volume)
exten => 6060,n,Answer()
exten => 6060,n,Confbridge(this_conference)
exten => 6061,1,Answer()
exten => 6061,n,Confbridge(this_conference)
exten => 6062,1,Answer()
exten => 6062,n,Confbridge(this_conference,,,sample_user_menu)
exten => 6063,1,NoOp
exten => 6063,n,Set(CONFBRIDGE(menu,template)=sample_user_menu)
exten => 6063,n,Set(CONFBRIDGE(menu,0)=leave_conference)
exten => 6063,n,Answer()
exten => 6063,n,Confbridge(this_conference)
With the following sample_user_menu configuration in confbridge.conf
[sample_user_menu]
type=menu
*=playback_and_continue(conf-usermenu)
*1=toggle_mute
1=toggle_mute
*4=decrease_listening_volume
4=decrease_listening_volume
*6=increase_listening_volume
6=increase_listening_volume
*7=decrease_talking_volume
7=decrease_talking_volume
*8=leave_conference
8=leave_conference
*9=increase_talking_volume
9=increase_talking_volume</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/apps/confbridge/include/confbridge.h <span style="color: grey">(402126)</span></li>
<li>/trunk/apps/confbridge/conf_config_parser.c <span style="color: grey">(402126)</span></li>
<li>/trunk/apps/app_confbridge.c <span style="color: grey">(402126)</span></li>
<li>/trunk/CHANGES <span style="color: grey">(402126)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2971/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>