<div dir="ltr">Hi Jean-Marc,<div><br></div><div>The original celt_fir() is a little bit messy. It has 2 branches chosen by #ifdef SMALL_FOOTPRINT.<br></div><div>For floating-point, the 2 branches are identical (except the operation sequence of accumulating x[i] to sum, which is not a big deal).</div><div>For fixed-point, the 2 branches are different. I separate them into 2 functions: the new celt_fir(), and celt_fir_permit_overflow() which is the SMALL_FOOTPRINT branch.</div><div>The only difference for fixed-point is:</div><div>celt_fir(): the sum is truncated first and then accumulated to x[i] and saturated.</div><div><div>celt_fir_permit_overflow(): x[i] is accumulated to the sum first and then truncated saturated.</div></div><div><br></div><div>Maybe this is the reason why silk_LPC_analysis_filter() switched the FIR from celt_fir() to celt_fir_permit_overflow() half a year ago.</div><div><br></div><div>Because of silk_LPC_analysis_filter(), celt_fir_permit_overflow() must behave the same for both floating-point and fixed-point, and this is why we defined ADD32_FIXED(), ..., PSHR32_FIXED() etc.</div><div><br></div><div>It's still a messy.</div><div><br></div><div>For the NEON optimization part, the previous celt_fir() optimization calls xcorr_kernel(). We tested and found that calling the xcorr_kernel() optimization didn't help too much here. The optimization in the patch is about 1% faster than simply calling xcorr_kernel() for the whole encoder. Considering the really small size of the new optimization, it's better to not call xcorr_kernel() to get 1% faster.</div><div><br></div><div>Thanks,</div><div>Linfeng</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 15, 2017 at 12:06 PM, Jean-Marc Valin <span dir="ltr"><<a href="mailto:jmvalin@jmvalin.ca" target="_blank">jmvalin@jmvalin.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Linfeng,<br>
<br>
Can you give me a bit more details about the purpose of this patchset.<br>
It seems to me like it's mostly duplicating the celt_fir()<br>
optimizations? Did I miss anything?<br>
<br>
Cheers,<br>
<br>
        Jean-Marc<br>
<span class=""><br>
On 15/02/17 02:22 PM, Linfeng Zhang wrote:<br>
</span><div><div class="h5">> Hi,<br>
><br>
> Attached are two patches. Patch 1 refactors silk_LPC_analysis_filter().<br>
> And Patch 2 optimizes the new function celt_fir_permit_overflow() for<br>
> ARM NEON.<br>
><br>
> Please recommend a better function name.<br>
><br>
> We did the same internal code review and testing already.<br>
><br>
> Thanks,<br>
> Linfeng<br>
><br>
><br>
><br>
</div></div>> ______________________________<wbr>_________________<br>
> opus mailing list<br>
> <a href="mailto:opus@xiph.org">opus@xiph.org</a><br>
> <a href="http://lists.xiph.org/mailman/listinfo/opus" rel="noreferrer" target="_blank">http://lists.xiph.org/mailman/<wbr>listinfo/opus</a><br>
><br>
</blockquote></div><br></div>