<p>Patch set 32:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/11579">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/11579/32/apps/app_audiosocket.c">File apps/app_audiosocket.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/11579/32/apps/app_audiosocket.c">Patch Set #32:</a> </p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm still confused as to why we need the app when the dialplan could just Dial using the audiosocket channel. But OK.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/11579/32/apps/app_audiosocket.c@111">Patch Set #32, Line 111:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> if (ast_set_write_format(chan, ast_format_slin)) {<br> ast_log(LOG_ERROR, "Failed to set write format to SLINEAR for channel %s\n", chanName);<br> ao2_ref(writeFormat, -1);<br> ao2_ref(readFormat, -1);<br> return -1;<br> }<br> if (ast_set_read_format(chan, ast_format_slin)) {<br> ast_log(LOG_ERROR, "Failed to set read format to SLINEAR for channel %s\n", chanName);<br><br> /* Attempt to restore previous write format even though it is likely to<br> * fail, since setting the read format did.<br> */<br> if (ast_set_write_format(chan, writeFormat)) {<br> ast_log(LOG_ERROR, "Failed to restore write format for channel %s\n", chanName);<br> }<br> ao2_ref(writeFormat, -1);<br> ao2_ref(readFormat, -1);<br> return -1;<br> }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is the "calling" channel yes? If so is this going to cause a renegoation with the caller?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/11579/32/apps/app_audiosocket.c@177">Patch Set #32, Line 177:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> struct ast_channel *targetChan;<br> int ms = 0;<br> int outfd = 0;<br> struct ast_frame *f;<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Might be better to declare these outside the loop since this loop runs for every frame.<br>Just initialize ms to 0. outfd should probably be initialized to -1 at the top of the loop since 0 is a valid value.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/11579/32/res/res_audiosocket.exports.in">File res/res_audiosocket.exports.in:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/11579/32/res/res_audiosocket.exports.in@3">Patch Set #32, Line 3:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> LINKER_SYMBOL_PREFIXast_audiosocket_connect;<br> LINKER_SYMBOL_PREFIXast_audiosocket_init;<br> LINKER_SYMBOL_PREFIXast_audiosocket_send_frame;<br> LINKER_SYMBOL_PREFIX*ast_audiosocket_receive_frame;<br></pre></blockquote></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">You can condense this down to just<br> LINKER_SYMBOL_PREFIXast_audiosocket*<br>unless there are ast_audiosocket functions that you _don't_ want exported.</pre></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/11579">change 11579</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/11579"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ie866e6c4fa13178ec76f2a6971ad3590a3a588b5 </div>
<div style="display:none"> Gerrit-Change-Number: 11579 </div>
<div style="display:none"> Gerrit-PatchSet: 32 </div>
<div style="display:none"> Gerrit-Owner: Seán C. McCord <ulexus@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Seán C. McCord <ulexus@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 11 Dec 2019 16:33:12 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>