<div>Hi Jean-Marc,</div><div><br></div><div>Yes I confirm that we have done the same internal review on this patch.</div><div><br></div><div>For 1), I agree that an explicit unit test would be a good plus to cover the cases that "make check" cannot trigger. If you like, we may submit an unit test patch for code review.</div><div><br></div><div>Thanks,</div><div>Linfeng</div><div><br><div class="gmail_quote"><div>On Thu, Feb 9, 2017 at 4:48 PM Jean-Marc Valin <<a href="mailto:jmvalin@jmvalin.ca">jmvalin@jmvalin.ca</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Linfeng,<br class="gmail_msg">
<br class="gmail_msg">
Can you confirm that you the patch went through the same internal review<br class="gmail_msg">
(presumably from James) than the previous ones?<br class="gmail_msg">
<br class="gmail_msg">
I had a look and did some testing and it looked good to me. There's only<br class="gmail_msg">
two issues I'd like to resolve first -- none of which directly related<br class="gmail_msg">
to your code.<br class="gmail_msg">
<br class="gmail_msg">
1) The overflow condition is essentially untested because none of the<br class="gmail_msg">
tests in "make check" reliably triggers it. That may be a good case for<br class="gmail_msg">
an explicit unit test. IIRC, the case could be triggered by the<br class="gmail_msg">
following input vector:<br class="gmail_msg">
A_QA[] = { 46596096, -72118272, 78532608, -69447680, 52707328,<br class="gmail_msg">
-22073344, -19890176, 50507776, -54829056, 45518848, -33939456,<br class="gmail_msg">
21086208, -7127040, -4136960, 3993600, -1699840 }<br class="gmail_msg">
<br class="gmail_msg">
2) I'm not quite sure what to make of silk_LPC_inverse_pred_gain_Q24().<br class="gmail_msg">
It seems to never be called anywhere -- except from the MIPS code. Maybe<br class="gmail_msg">
it should just stay as it is (not renamed to _c()) but I need to check.<br class="gmail_msg">
Any thoughts?<br class="gmail_msg">
<br class="gmail_msg">
Cheers,<br class="gmail_msg">
<br class="gmail_msg">
Jean-Marc<br class="gmail_msg">
<br class="gmail_msg">
On 07/02/17 04:06 PM, Linfeng Zhang wrote:<br class="gmail_msg">
> Hi,<br class="gmail_msg">
><br class="gmail_msg">
> Attached is a patch with arm neon optimizations for<br class="gmail_msg">
> silk_LPC_inverse_pred_gain(). Please review.<br class="gmail_msg">
><br class="gmail_msg">
> Thanks,<br class="gmail_msg">
> Linfeng<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> _______________________________________________<br class="gmail_msg">
> opus mailing list<br class="gmail_msg">
> <a href="mailto:opus@xiph.org" class="gmail_msg" target="_blank">opus@xiph.org</a><br class="gmail_msg">
> <a href="http://lists.xiph.org/mailman/listinfo/opus" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.xiph.org/mailman/listinfo/opus</a><br class="gmail_msg">
><br class="gmail_msg">
</blockquote></div></div>