<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/2070/">https://reviewboard.asterisk.org/r/2070/</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;">I don't like the way that this is implemented because essentially, chan_local will claim that it can handle T.38 faxes even if the local channel is proxying a channel that cannot or should not handle T.38 faxes.
I think your comment in the diff is the right idea, but the execution is off. Essentially what chan_local needs to do is relay the indication to the channel that it is proxying and return the result of that indication. The tricky part of this is going to be getting the locking correct. When local_indicate is called, the "ast" channel passed in will already be locked. You will need to lock the other channel (you can determine which channel is the other channel using the IS_OUTBOUND macro) before calling ast_indicate_data() on it. However, I don't see a good reason to keep ast locked while calling ast_indicate_data() on the other channel.
Approximately how this would look:
if (condition == AST_CONTROL_T38) {
ast_channel *other_channel = IS_OUTBOUND(ast, p) ? p->owner : p->chan;
if (!other_channel) {
return -1;
}
ao2_unlock(ast);
ao2_lock(other_channel);
res = ast_indicate_data(other_channel, condition, data, datalen);
ao2_unlock(other_channel);
ao2_lock(ast);
return res;
}
There may be other things you need to do (like unreffing p before returning) but this is only meant to be approximate.</pre>
<br />
<p>- Mark</p>
<br />
<p>On August 14th, 2012, 10:46 a.m., wdoekes 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 wdoekes.</div>
<p style="color: grey;"><i>Updated Aug. 14, 2012, 10:46 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;">See bug report. For this dialplan, faxing to 's' doesn't work, but faxing to 'fax' does. The patch fixes so it works. But I'm not familiar enough with T38 nor the indications to know if this is the right fix.
[default]
exten => s,1,Dial(Local/fax@default)
exten => fax,1,NoOp()
same => n,ReceiveFAX(/tmp/abc.tiff)
same => n,Hangup()
</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;">On 1.6.2.x faxing works without the patch.
On 1.8.x faxing works on the 's' extension only after applying the patch.
I used the patched sipp from https://code.osso.nl/projects/sipp with the sendfax.xml and sendfax.pcap to replay a (lengthy) fax.
</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/ASTERISK-20229">ASTERISK-20229</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/channels/chan_local.c <span style="color: grey">(371243)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2070/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>