<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/4010/">https://reviewboard.asterisk.org/r/4010/</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 22nd, 2014, 8:48 a.m. UTC, <b>wdoekes</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/4010/diff/1/?file=67441#file67441line1428" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/res/res_musiconhold.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 local_ast_moh_start(struct ast_channel *chan, const char *mclass, const char *interpclass)</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">1428</font></th>
<td bgcolor="#c5ffc4" 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="n">mohclass</span> <span class="o">&&</span> <span class="o">!</span><span class="n">var</span> <span class="o">&&</span> <span class="o">!</span><span class="n">ast_strlen_zero</span><span class="p">(</span><span class="n">chan</span><span class="o">-></span><span class="n">musicclass</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">1429</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">mohclass</span> <span class="o">=</span> <span class="n">get_mohbyname</span><span class="p">(</span><span class="n">chan</span><span class="o">-></span><span class="n">musicclass</span><span class="p">,</span> <span class="mi">1</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">1430</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="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">mohclass</span> <span class="o">&&</span> <span class="n">realtime_possible</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">1431</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="tb"> </span><span class="n">var</span> <span class="o">=</span> <span class="n">ast_load_realtime</span><span class="p">(</span><span class="s">"musiconhold"</span><span class="p">,</span> <span class="s">"name"</span><span class="p">,</span> <span class="n">chan</span><span class="o">-></span><span class="n">musicclass</span><span class="p">,</span> <span class="n">SENTINEL</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">1432</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="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">1433</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </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;">(A) If !MOH_APPOVERRIDECHANNEL, then you're loading this twice. Don't do that, especially not when there is realtime involved. Add a MOH_APPOVERRIDECHANNEL test.
(B) chan->musicclass? Looks like old code. Did this compile on trunk? You mean ast_channel_musicclass(chan)..</pre>
</blockquote>
<p>On September 22nd, 2014, 12:22 p.m. UTC, <b>Kristian Høgh</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;">(A) Do I need that? mohclass is set, right?
(B) fixed.</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;">No. Not if it isn't found.
I agree that that's a non-standard configuration. But the code looks awkward as it is.
Perhaps you should refactor the whole thing to something like this instead. Pseudocode:
const char *classes[4];
if (!ast_test_flag(global_flags, MOH_APPOVERRIDECHANNEL)) {
classes = {mclass, ast_channel_musicclass(chan)), interpclass, "default"};
} else {
classes = {ast_channel_musicclass(chan)), mclass, interpclass, "default"};
}
for (i = 0; i < ARRAY_LEN(classes); ++i)
if (!ast_strlen_zero(classes[i]))
// attempt loading and break when found
</pre>
<br />
<p>- wdoekes</p>
<br />
<p>On September 22nd, 2014, 12:23 p.m. UTC, Kristian Høgh 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 Kristian Høgh.</div>
<p style="color: grey;"><i>Updated Sept. 22, 2014, 12:23 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-24276">ASTERISK-24276</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;">General option to musiconhold.conf, to make applications MOH override channels musicclass.
Defaults to no, as it is current behavior.
We use a single trunk for multiple users. The default musicclass from sip.conf is therefore the same for all calls.
The trunk is connected to a kamailio server, where all in- and outgoing calls are send/received.
Each customer (identified by custom headers) can choose a musicclass for MOH, which we apply to the channel.
When a user dials an extension, which point to a queue, we want the user to receive the queue music.
But we still want the user to keep the preferred musicclass, if the call is put on hold later.
Therefore we cant unset musicclass before calling the queue.
Company A prefer musicclass heavy-metal.
That musicclass is therefore applied to incoming calls.
The customer service need to calm people down. Therefore they use other music as queue music.</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;">Whithout this patch or new option appoverridechannel=no or no appoverridechannel set in musiconhold.conf:
- The queue "myqueue", has musicclass=queueclass
- Set(CHANNEL(musiconhold)=mohclass) followed by Queue(myqueue), makes caller hear "mohclass" while waiting for agent to answer.
Whith appoverridechannel=yes in musiconhold.conf:
- The queue "myqueue", has musicclass=queueclass
- Set(CHANNEL(musiconhold)=myclass) followed by Queue(myqueue), makes caller hear "queueclass" while waiting for agent to answer.
</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/res/res_musiconhold.c <span style="color: grey">(423656)</span></li>
<li>/trunk/configs/samples/musiconhold.conf.sample <span style="color: grey">(423656)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4010/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>