Hi! I'm Yves, bit fiddler, hoping to roll a fresh version of libopus into chromium. This fix <https://chromium-review.googlesource.com/c/chromium/src/+/1072869/> never made it to opus repository. Was it intentional? If so, what is the rational? If not, could the fix be applied on the libopus master branch? Thanks for the amazing work! -- Warm regards, Yves -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xiph.org/pipermail/opus/attachments/20190910/ef501b45/attachment.html>
Hi Yves, At that time, the issue wasn't reproducible in opus master, but there had been some significant changes since the last tagged release that made it hard to pinpoint a single patch to cherry pick to Chromium, hence this local fix. Cheers, Felicia On Tue, Sep 10, 2019 at 7:56 AM Yves Gerey <yvesg at google.com> wrote:> Hi! I'm Yves, bit fiddler, hoping to roll a fresh version of libopus into > chromium. > > This fix > <https://chromium-review.googlesource.com/c/chromium/src/+/1072869/> never > made it to opus repository. > > Was it intentional? If so, what is the rational? > If not, could the fix be applied on the libopus master branch? > > Thanks for the amazing work! > > -- > Warm regards, > Yves >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xiph.org/pipermail/opus/attachments/20190910/20f279c9/attachment.html>
Felicia Lim wrote:> Hi Yves, > > At that time, the issue wasn't reproducible in opus master, but there > had been some significant changes since the last tagged release that > made it hard to pinpoint a single patch to cherry pick to Chromium, > hence this local fix.I'm not convinced that using a saturating add here is the correct thing to do, either (as opposed to, say, multiplying out by the coefficients individually). Either way, there are exact analogs of this code in NSQ.c, as well as all of the SIMD optimizations (x86/NSQ_sse4_1.c and x86/NSQ_del_dec_sse4_1.c, arm/NSQ_del_dec_neon_intr.c, and mips/NSQ_del_dec_mipsr1.h). If it is a possible issue in master, the fix looks incomplete to me, at least.