opus - Oct 2015 - [RFC V3 7/8] armv7, armv8: Optimize fixed point fft using NE10 library

I'm trying to get these cleaned up and landed, but I'm running into  
some trouble with this patch. Using commit a08b29d88e3c (July 21) of  
Ne10, I'm seeing test failures for 60-point FFTs:

nfft=60 inverse=0,snr = -3.312408
** poor snr: -3.312408 **
nfft=60 inverse=1,snr = -16.079597
** poor snr: -16.079597 **

All other sizes tested appear to work fine (84 to 140 dB of SNR). This  
doesn't match the results you reported on this list in May, where the  
FFTs appeared to pass tests, but the MDCTs did not (the latter of  
which I presume was fixed by the May 12 commits to Ne10). I don't see  
any commits since then that I would expect to have broken this. Any  
ideas?

Hello Timothy,

Great to hear from you!

Fired up my hardware today and this issue looks like a regression in
Ne10 library.

The commit in Ne10 [1] that I tested to be working successfully back in May
5b63074db45000f9688460990ee3f5e147d93782
which is the Patch Phil at ARM added to fix the overflow issue in nfft=60 case.

After git-bisect, looks like the culprit patch in Ne10 [1] is
cf33c0d51a445bbe6ad7e21c2af875acee07b838

Phil,
Please fix the regression at the earliest and let me know if you need any help.

Regards,
Vish

[1]: https://github.com/projectNe10/Ne10.git

On 5 October 2015 at 20:21, Timothy B. Terriberry
<tterriberry at mozilla.com> wrote:
>
> I'm trying to get these cleaned up and landed, but I'm running into
some trouble with this patch. Using commit a08b29d88e3c (July 21) of Ne10,
I'm seeing test failures for 60-point FFTs:
>
> nfft=60 inverse=0,snr = -3.312408
> ** poor snr: -3.312408 **
> nfft=60 inverse=1,snr = -16.079597
> ** poor snr: -16.079597 **
>
> All other sizes tested appear to work fine (84 to 140 dB of SNR). This
doesn't match the results you reported on this list in May, where the FFTs
appeared to pass tests, but the MDCTs did not (the latter of which I presume was
fixed by the May 12 commits to Ne10). I don't see any commits since then
that I would expect to have broken this. Any ideas?

Viswanath Puttagunta <viswanath.puttagunta at linaro.org>
wrote:
> After git-bisect, looks like the culprit patch in Ne10 [1] is
> cf33c0d51a445bbe6ad7e21c2af875acee07b838
Thanks! Testing using the prior commit seems to work fine.

Hi Timothy,

Sorry for late reply. I have upstreamed the patch to fix the regression here:
 
https://github.com/projectNe10/Ne10/commit/ee5d856cd9cb8c4a15ace567df4239f4e788d043

I have tested it with Vish's branch:
 
http://git.linaro.org/people/viswanath.puttagunta/opus.git/shortlog/refs/heads/rfcv3_fft_fixed)

Both unit test dft and unit test mdct passed on ARM v7/v8, floating point/fixed
point, with or without Ne10.

Best regards,
Phil Wang

-----Original Message-----
From: Viswanath Puttagunta [mailto:viswanath.puttagunta at linaro.org]
Sent: Tuesday, October 06, 2015 11:40 PM
To: Timothy B. Terriberry
Cc: Phil Wang; opus at xiph.org; Viswanath Puttagunta; Tom Gall
Subject: Re: [opus] [RFC V3 7/8] armv7, armv8: Optimize fixed point fft using
NE10 library

Hello Timothy,

Great to hear from you!

Fired up my hardware today and this issue looks like a regression in
Ne10 library.

The commit in Ne10 [1] that I tested to be working successfully back in May
5b63074db45000f9688460990ee3f5e147d93782
which is the Patch Phil at ARM added to fix the overflow issue in nfft=60 case.

After git-bisect, looks like the culprit patch in Ne10 [1] is
cf33c0d51a445bbe6ad7e21c2af875acee07b838

Phil,
Please fix the regression at the earliest and let me know if you need any help.

Regards,
Vish

[1]: https://github.com/projectNe10/Ne10.git

On 5 October 2015 at 20:21, Timothy B. Terriberry <tterriberry at
mozilla.com> wrote:
>
> I'm trying to get these cleaned up and landed, but I'm running into
some trouble with this patch. Using commit a08b29d88e3c (July 21) of Ne10,
I'm seeing test failures for 60-point FFTs:
>
> nfft=60 inverse=0,snr = -3.312408
> ** poor snr: -3.312408 **
> nfft=60 inverse=1,snr = -16.079597
> ** poor snr: -16.079597 **
>
> All other sizes tested appear to work fine (84 to 140 dB of SNR). This
doesn't match the results you reported on this list in May, where the FFTs
appeared to pass tests, but the MDCTs did not (the latter of which I presume was
fixed by the May 12 commits to Ne10). I don't see any commits since then
that I would expect to have broken this. Any ideas?

-- IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered
in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
Registered in England & Wales, Company No:  2548782