<p> Attention is currently required from: Kevin Harwell, Richard Mudgett. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/16231">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File funcs/func_scramble.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/+/16231/comment/7773510f_e2843ff0">Patch Set #6, Line 195:</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;">        ast_channel_lock(chan);<br>       if (!(datastore = ast_channel_datastore_find(chan, &scramble_datastore, NULL))) {<br>         ast_channel_unlock(chan);<br>             /* Allocate a new datastore to hold the reference to this audiohook information */<br>            if (!(datastore = ast_datastore_alloc(&scramble_datastore, NULL))) {<br>                      return 0;<br>             }<br>             if (!(ni = ast_calloc(1, sizeof(*ni)))) {<br>                     ast_datastore_free(datastore);<br>                        return 0;<br>             }<br>             ast_audiohook_init(&ni->audiohook, AST_AUDIOHOOK_TYPE_MANIPULATE, "Voice scrambler", AST_AUDIOHOOK_MANIPULATE_ALL_RATES);<br>            ni->audiohook.manipulate_callback = scramble_callback;<br>             datastore->data = ni;<br>              ast_channel_lock(chan);<br>               ast_channel_datastore_add(chan, datastore);<br>           ast_audiohook_attach(chan, &ni->audiohook);<br>    } else {<br>              ni = datastore->data;<br>      }<br>     ast_channel_unlock(chan);<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Leave the lock/unlock calls surrounding the "if" but remove the ones embedded within. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done - as a general rule of thumb, is this a best practice? In other existing functions setting up audiohooks, that seems to be the basic way they do it, i.e. the channel is not locked during the whole setup, e.g. func_pitchshift.c. So it's best to do it this way and this same issue would exist these other functions?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16231/comment/ecd7eda9_7d0e8411">Patch Set #6, Line 216:</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;">    ni->tx = tx;<br>       ni->rx = rx;<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Surround setting these variables too in the lock above otherwise two separate threads could attempt  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/16231">change 16231</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/+/16231"/><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: I01020769d91060a1f56a708eb405f87648d1a67e </div>
<div style="display:none"> Gerrit-Change-Number: 16231 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Attention: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Attention: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 11 Aug 2021 17:48:01 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>