<p> Attention is currently required from: Sean Bright, Kevin Harwell. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/18689">View Change</a></p><p>5 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Commit Message:</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/+/18689/comment/d91df499_6462e80b">Patch Set #1, Line 16:</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;"><br>This fixes my immediate issue, however may re-introduce the issue. Is<br>there a reliable test case for the crash which can be used to validate<br>that this doesn't re-introduce the problem?<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Unfortunately I don't think there is a testsuite test for this. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">This patch only affects frames going through the JB, and the JB can only create extra frames from JB_INTERP, which (as far as I can determine) can ONLY happen once voicefmt has been established. So if this issue is also fixed for the non-JB case (which it should be), then we are having the wrong discussion. If it's not fixed, we need to address subclass.format being NULL in those locations where they could cause problems.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</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/+/18689?tab=comments">Patch Set #1:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Lack of indications. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File channels/chan_iax2.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/+/18689/comment/b606d097_d7a8c376">Patch Set #1, Line 4152:</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 (ms >= (next = jb_next(pvt->jb))) {<br> voicefmt = ast_format_compatibility_bitfield2format(pvt->voiceformat);<br> ret = jb_get(pvt->jb, &frame, ms, voicefmt ? ast_format_get_default_ms(voicefmt) : 20);<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I believe this was initially changed to bypass having to do multiple NULL checks on voicefmt, and po […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18689/comment/1fadca7e_88f11fea">Patch Set #1, Line 4168:</a> <code style="font-family:monospace,monospace"> af.subclass.format = voicefmt;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">if af.subclass. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Looking at the JB code, it's impossible for voicefmt to be NULL here as we can only get JB_INTERP frames once we've had at least one voice frame, at which point voicefmt will be set to a non-NULL value since the voice format will have been negotiated.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18689/comment/2f99b24a_a1bf4f5c">Patch Set #1, Line 4169:</a> <code style="font-family:monospace,monospace"> af.samples = frame.ms * (ast_format_get_sample_rate(voicefmt) / 1000);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">With the above revert this will also require a NULL check and a default used.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">See above explanation. I don't think this is the case.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18689">change 18689</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/+/18689"/><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: I80f9c1af4b2cd8281dbd74cb7cbc01b8deccdf96 </div>
<div style="display:none"> Gerrit-Change-Number: 18689 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Jaco Kroon <jaco@uls.co.za> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <default.enum@gmail.com> </div>
<div style="display:none"> Gerrit-CC: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-Attention: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-Attention: Kevin Harwell <default.enum@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 15 Dec 2022 09:12:28 +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: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Comment-In-Reply-To: Kevin Harwell <default.enum@gmail.com> </div>
<div style="display:none"> Comment-In-Reply-To: Jaco Kroon <jaco@uls.co.za> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>