<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/1559/">https://reviewboard.asterisk.org/r/1559/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 3rd, 2011, 4:32 p.m., <b>schmidts</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/1559/diff/1/?file=21572#file21572line4392" style="color: black; font-weight: bold; text-decoration: underline;">branches/1.8/main/channel.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 ast_indicate_data(struct ast_channel *chan, int _condition,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4392</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="n">condition</span> <span class="o">==</span> <span class="n">AST_CONTROL_UNHOLD</span> <span class="o">||</span> <span class="n">_condition</span> <span class="o"><</span> <span class="mi">0</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4392</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="n">condition</span> <span class="o">==</span> <span class="n">AST_CONTROL_UNHOLD</span> <span class="o">||</span> <span class="n">_condition</span> <span class="o"><</span> <span class="mi">0</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4393</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/* Visible indication is cleared/stopped. */</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4393</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/* Visible indication is cleared/stopped. */</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4394</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">chan</span><span class="o">-></span><span class="n">visible_indication</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4394</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">chan</span><span class="o">-></span><span class="n">visible_indication</span> <span class="o">=</span> <span class="mi">0</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;">i am not sure if its a good idea to only look at the _condition and stop playtones.
AFAIK chan->tech->indicate looks mostly on the visible_indication which is here set to 0 if _condition is < 0.
this could means chan->tech->indicate is doing everything fine with indication 0 but you maybe do stop_playtones twice and i dont know if this could lead to other problems.
if this is a reproduceable problem you should try to see what happens in the used chan->tech->indicate function. maybe you can see some difference there between condition and visible_indication.
</pre>
</blockquote>
<p>On November 5th, 2011, 6:54 p.m., <b>may213</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;">Thank for review.
Neither channel driver look to visible_indication:
may@vl:~/asterisk/x/original/trunk> find channels/ -name '*.[ch]' | xargs grep visible_indication
may@vl:~/asterisk/x/original/trunk>
ast_playtones_stop just call ast_deactivate_generator that remove generator only if exists so
there will not problem even if it will called twice:
----
void ast_deactivate_generator(struct ast_channel *chan)
{
ast_channel_lock(chan);
if (chan->generatordata) {
....
}
ast_channel_unlock(chan);
----
br
</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;">The visible_indication is used by masquerade so it can maintain visible tones like ringback on the new channel. Channel drivers don't care about it since they are concerned with the current indication being processed.</pre>
<br />
<p>- rmudgett</p>
<br />
<p>On November 2nd, 2011, 2:47 a.m., may213 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 may213.</div>
<p style="color: grey;"><i>Updated Nov. 2, 2011, 2:47 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;">ast_indicate_data function must call ast_stop_playtones when condition arg is negative, but don't call ast_stop_playtones if indicate function of channel driver return zero result.
If channel->tech->indicate return zero then processing go to indicate_cleanup before processing negative condition arg, it's bug
because we must stop tone generator if condition arg is negative independent of returned from tech->indicate value.
The patch call ast_stop_playtones before processing indicate return value.</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;">app_queue work correctly with 'r' option on dahdi channel with patch.</pre>
</td>
</tr>
</table>
<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/18803">18803</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>branches/1.8/main/channel.c <span style="color: grey">(338895)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1559/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>