Hello Jean-Marc, Below are the results that show test_unit_dft passes, but test_unit_mdct fails (only for nfft=480, 960, 1920) Note: Tested on BeagleboneBlack(Cortex-A8) fixed point on branch [1] ./test_unit_dft nfft=32 inverse=0,snr = 88.394372 nfft=32 inverse=1,snr = 93.896470 nfft=128 inverse=0,snr = 89.185895 nfft=128 inverse=1,snr = 93.537021 nfft=256 inverse=0,snr = 88.353151 nfft=256 inverse=1,snr = 90.826012 nfft=36 inverse=0,snr = 87.883324 nfft=36 inverse=1,snr = 88.161852 nfft=50 inverse=0,snr = 84.841953 nfft=50 inverse=1,snr = 86.312280 nfft=60 inverse=0,snr = 142.086642 nfft=60 inverse=1,snr = 140.223589 nfft=120 inverse=0,snr = 139.717566 nfft=120 inverse=1,snr = 135.677978 nfft=240 inverse=0,snr = 137.783374 nfft=240 inverse=1,snr = 131.742062 nfft=480 inverse=0,snr = 137.329389 nfft=480 inverse=1,snr = 125.394139 Testing test_unit_mdct nfft=32 inverse=0,snr = 85.342660 nfft=32 inverse=1,snr = 96.283780 nfft=256 inverse=0,snr = 86.729952 nfft=256 inverse=1,snr = 87.326203 nfft=512 inverse=0,snr = 82.575740 nfft=512 inverse=1,snr = 87.088448 nfft=1024 inverse=0,snr = 84.944481 nfft=1024 inverse=1,snr = 86.939423 nfft=2048 inverse=0,snr = 84.754290 nfft=2048 inverse=1,snr = 86.859531 nfft=36 inverse=0,snr = 84.423668 nfft=36 inverse=1,snr = 86.638850 nfft=40 inverse=0,snr = 83.808312 nfft=40 inverse=1,snr = 87.403083 nfft=60 inverse=0,snr = 84.983050 nfft=60 inverse=1,snr = 88.736219 nfft=120 inverse=0,snr = 82.342631 nfft=120 inverse=1,snr = 84.941462 nfft=240 inverse=0,snr = 88.011018 nfft=240 inverse=1,snr = 88.410825 nfft=480 inverse=0,snr = 9.016824 ** poor snr: 9.016824 ** nfft=480 inverse=1,snr = 87.773827 nfft=960 inverse=0,snr = 11.813777 ** poor snr: 11.813777 ** nfft=960 inverse=1,snr = 88.283572 nfft=1920 inverse=0,snr = 7.652986 ** poor snr: 7.652986 ** nfft=1920 inverse=1,snr = 87.762925 Thanks, Vish [1]: https://git.linaro.org/people/viswanath.puttagunta/opus.git/shortlog/refs/heads/rfcv2_fft_fixed_exp On 8 May 2015 at 12:06, Jean-Marc Valin <jmvalin at jmvalin.ca> wrote:> Is the actual output of the test posted somewhere? One thing I would > suspect is that the test might use values that trigger overflows not > generally triggered with audio. > > Jean-Marc > > On 08/05/15 11:08 AM, Viswanath Puttagunta wrote: >> Hello Timothy, >> >> Just FYI, Phil at ARM is still looking into why mdct is failing.. will >> keep you posted. In the mean time, do you want me to disable NE10 for >> mdct_forward and re-submit the patchset so we may make progress? >> >> Regards, >> Vish >> >> On 30 April 2015 at 09:33, Viswanath Puttagunta >> <viswanath.puttagunta at linaro.org> wrote: >>> >>> On 29 April 2015 at 17:22, Timothy B. Terriberry <tterribe at xiph.org> wrote: >>>> >>>> Viswanath Puttagunta wrote: >>>>> >>>>> This patch series is follow up on work I posted on [1]. >>>>> In addition to what was posted on [1], this patch series mainly >>>>> integrates Fixed point FFT implementations in NE10 library into opus. >>>>> You can view my opus wip code at [2]. >>>> >>>> >>>> Thanks, and I apologize for being slow here. >>>> >>>>> - This was surprising to me because test_unit_dft passes for all >>>>> nfft including 60, 120, 240, 480. May be there are some data >>>>> corner cases that need further investigation. >>>> >>>> >>>> Yes, that seems concerning. I'd like to at least understand the cause of the failures. "Audio is clearly audible" is not a very strong test for encoder quality regressions, but in the worst case we could just disable NE10 for mdct_forward. >>> >>> Yes, agree we need to root cause this.. I only mentioned audio is >>> audible clearly to point out that NE10 fft is not completely messed >>> up, but does have some corner cases where it is failing. ARM folks are >>> actively looking into this issue, and expecting some resolution next >>> week. >>> >>> Also, this NE10 fixed fft issue only impacts patches >>> armv7,armv8: Optimize fixed point fft using NE10 library >>> armv7,armv8: Extend fixed fft NE10 optimizations to mdct >>> >>> In the mean time, do you want me (as you suggested) disable NE10 fixed >>> fft for mdct_forward? That way when it gets fixed in NE10, it will >>> just be a one line change patch that will follow. >>> >>> Either way, request that we make progress on review/merge rest of the patches. >> _______________________________________________ >> opus mailing list >> opus at xiph.org >> http://lists.xiph.org/mailman/listinfo/opus >>
Hi, Can you apply this change to the MDCT test and run it again. See if more (all) sizes pass. Given the results, I strongly suspect an overflow. Jean-Marc On 08/05/15 01:21 PM, Viswanath Puttagunta wrote:> Hello Jean-Marc, > > Below are the results that show test_unit_dft passes, but > test_unit_mdct fails (only for nfft=480, 960, 1920) > Note: Tested on BeagleboneBlack(Cortex-A8) fixed point on branch [1] > > ./test_unit_dft > nfft=32 inverse=0,snr = 88.394372 > nfft=32 inverse=1,snr = 93.896470 > nfft=128 inverse=0,snr = 89.185895 > nfft=128 inverse=1,snr = 93.537021 > nfft=256 inverse=0,snr = 88.353151 > nfft=256 inverse=1,snr = 90.826012 > nfft=36 inverse=0,snr = 87.883324 > nfft=36 inverse=1,snr = 88.161852 > nfft=50 inverse=0,snr = 84.841953 > nfft=50 inverse=1,snr = 86.312280 > nfft=60 inverse=0,snr = 142.086642 > nfft=60 inverse=1,snr = 140.223589 > nfft=120 inverse=0,snr = 139.717566 > nfft=120 inverse=1,snr = 135.677978 > nfft=240 inverse=0,snr = 137.783374 > nfft=240 inverse=1,snr = 131.742062 > nfft=480 inverse=0,snr = 137.329389 > nfft=480 inverse=1,snr = 125.394139 > > > Testing test_unit_mdct > > nfft=32 inverse=0,snr = 85.342660 > nfft=32 inverse=1,snr = 96.283780 > nfft=256 inverse=0,snr = 86.729952 > nfft=256 inverse=1,snr = 87.326203 > nfft=512 inverse=0,snr = 82.575740 > nfft=512 inverse=1,snr = 87.088448 > nfft=1024 inverse=0,snr = 84.944481 > nfft=1024 inverse=1,snr = 86.939423 > nfft=2048 inverse=0,snr = 84.754290 > nfft=2048 inverse=1,snr = 86.859531 > nfft=36 inverse=0,snr = 84.423668 > nfft=36 inverse=1,snr = 86.638850 > nfft=40 inverse=0,snr = 83.808312 > nfft=40 inverse=1,snr = 87.403083 > nfft=60 inverse=0,snr = 84.983050 > nfft=60 inverse=1,snr = 88.736219 > nfft=120 inverse=0,snr = 82.342631 > nfft=120 inverse=1,snr = 84.941462 > nfft=240 inverse=0,snr = 88.011018 > nfft=240 inverse=1,snr = 88.410825 > > nfft=480 inverse=0,snr = 9.016824 > ** poor snr: 9.016824 ** > > nfft=480 inverse=1,snr = 87.773827 > > nfft=960 inverse=0,snr = 11.813777 > ** poor snr: 11.813777 ** > > nfft=960 inverse=1,snr = 88.283572 > > nfft=1920 inverse=0,snr = 7.652986 > ** poor snr: 7.652986 ** > > nfft=1920 inverse=1,snr = 87.762925 > > > Thanks, > Vish > > [1]: https://git.linaro.org/people/viswanath.puttagunta/opus.git/shortlog/refs/heads/rfcv2_fft_fixed_exp > > > On 8 May 2015 at 12:06, Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: >> Is the actual output of the test posted somewhere? One thing I would >> suspect is that the test might use values that trigger overflows not >> generally triggered with audio. >> >> Jean-Marc >> >> On 08/05/15 11:08 AM, Viswanath Puttagunta wrote: >>> Hello Timothy, >>> >>> Just FYI, Phil at ARM is still looking into why mdct is failing.. will >>> keep you posted. In the mean time, do you want me to disable NE10 for >>> mdct_forward and re-submit the patchset so we may make progress? >>> >>> Regards, >>> Vish >>> >>> On 30 April 2015 at 09:33, Viswanath Puttagunta >>> <viswanath.puttagunta at linaro.org> wrote: >>>> >>>> On 29 April 2015 at 17:22, Timothy B. Terriberry <tterribe at xiph.org> wrote: >>>>> >>>>> Viswanath Puttagunta wrote: >>>>>> >>>>>> This patch series is follow up on work I posted on [1]. >>>>>> In addition to what was posted on [1], this patch series mainly >>>>>> integrates Fixed point FFT implementations in NE10 library into opus. >>>>>> You can view my opus wip code at [2]. >>>>> >>>>> >>>>> Thanks, and I apologize for being slow here. >>>>> >>>>>> - This was surprising to me because test_unit_dft passes for all >>>>>> nfft including 60, 120, 240, 480. May be there are some data >>>>>> corner cases that need further investigation. >>>>> >>>>> >>>>> Yes, that seems concerning. I'd like to at least understand the cause of the failures. "Audio is clearly audible" is not a very strong test for encoder quality regressions, but in the worst case we could just disable NE10 for mdct_forward. >>>> >>>> Yes, agree we need to root cause this.. I only mentioned audio is >>>> audible clearly to point out that NE10 fft is not completely messed >>>> up, but does have some corner cases where it is failing. ARM folks are >>>> actively looking into this issue, and expecting some resolution next >>>> week. >>>> >>>> Also, this NE10 fixed fft issue only impacts patches >>>> armv7,armv8: Optimize fixed point fft using NE10 library >>>> armv7,armv8: Extend fixed fft NE10 optimizations to mdct >>>> >>>> In the mean time, do you want me (as you suggested) disable NE10 fixed >>>> fft for mdct_forward? That way when it gets fixed in NE10, it will >>>> just be a one line change patch that will follow. >>>> >>>> Either way, request that we make progress on review/merge rest of the patches. >>> _______________________________________________ >>> opus mailing list >>> opus at xiph.org >>> http://lists.xiph.org/mailman/listinfo/opus >>>-------------- next part -------------- A non-text attachment was scrubbed... Name: test_mdct_scale.patch Type: text/x-patch Size: 364 bytes Desc: not available Url : http://lists.xiph.org/pipermail/opus/attachments/20150508/f4898eaf/attachment.bin
Hello Jean-Marc, Yep, that was it.. with your patch, test_unit_mdct passes for all nfft. So, what you do you suggest the next step here is? Regards, Vish On 8 May 2015 at 12:30, Jean-Marc Valin <jmvalin at jmvalin.ca> wrote:> Hi, > > Can you apply this change to the MDCT test and run it again. See if more > (all) sizes pass. Given the results, I strongly suspect an overflow. > > Jean-Marc > > On 08/05/15 01:21 PM, Viswanath Puttagunta wrote: >> Hello Jean-Marc, >> >> Below are the results that show test_unit_dft passes, but >> test_unit_mdct fails (only for nfft=480, 960, 1920) >> Note: Tested on BeagleboneBlack(Cortex-A8) fixed point on branch [1] >> >> ./test_unit_dft >> nfft=32 inverse=0,snr = 88.394372 >> nfft=32 inverse=1,snr = 93.896470 >> nfft=128 inverse=0,snr = 89.185895 >> nfft=128 inverse=1,snr = 93.537021 >> nfft=256 inverse=0,snr = 88.353151 >> nfft=256 inverse=1,snr = 90.826012 >> nfft=36 inverse=0,snr = 87.883324 >> nfft=36 inverse=1,snr = 88.161852 >> nfft=50 inverse=0,snr = 84.841953 >> nfft=50 inverse=1,snr = 86.312280 >> nfft=60 inverse=0,snr = 142.086642 >> nfft=60 inverse=1,snr = 140.223589 >> nfft=120 inverse=0,snr = 139.717566 >> nfft=120 inverse=1,snr = 135.677978 >> nfft=240 inverse=0,snr = 137.783374 >> nfft=240 inverse=1,snr = 131.742062 >> nfft=480 inverse=0,snr = 137.329389 >> nfft=480 inverse=1,snr = 125.394139 >> >> >> Testing test_unit_mdct >> >> nfft=32 inverse=0,snr = 85.342660 >> nfft=32 inverse=1,snr = 96.283780 >> nfft=256 inverse=0,snr = 86.729952 >> nfft=256 inverse=1,snr = 87.326203 >> nfft=512 inverse=0,snr = 82.575740 >> nfft=512 inverse=1,snr = 87.088448 >> nfft=1024 inverse=0,snr = 84.944481 >> nfft=1024 inverse=1,snr = 86.939423 >> nfft=2048 inverse=0,snr = 84.754290 >> nfft=2048 inverse=1,snr = 86.859531 >> nfft=36 inverse=0,snr = 84.423668 >> nfft=36 inverse=1,snr = 86.638850 >> nfft=40 inverse=0,snr = 83.808312 >> nfft=40 inverse=1,snr = 87.403083 >> nfft=60 inverse=0,snr = 84.983050 >> nfft=60 inverse=1,snr = 88.736219 >> nfft=120 inverse=0,snr = 82.342631 >> nfft=120 inverse=1,snr = 84.941462 >> nfft=240 inverse=0,snr = 88.011018 >> nfft=240 inverse=1,snr = 88.410825 >> >> nfft=480 inverse=0,snr = 9.016824 >> ** poor snr: 9.016824 ** >> >> nfft=480 inverse=1,snr = 87.773827 >> >> nfft=960 inverse=0,snr = 11.813777 >> ** poor snr: 11.813777 ** >> >> nfft=960 inverse=1,snr = 88.283572 >> >> nfft=1920 inverse=0,snr = 7.652986 >> ** poor snr: 7.652986 ** >> >> nfft=1920 inverse=1,snr = 87.762925 >> >> >> Thanks, >> Vish >> >> [1]: https://git.linaro.org/people/viswanath.puttagunta/opus.git/shortlog/refs/heads/rfcv2_fft_fixed_exp >> >> >> On 8 May 2015 at 12:06, Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: >>> Is the actual output of the test posted somewhere? One thing I would >>> suspect is that the test might use values that trigger overflows not >>> generally triggered with audio. >>> >>> Jean-Marc >>> >>> On 08/05/15 11:08 AM, Viswanath Puttagunta wrote: >>>> Hello Timothy, >>>> >>>> Just FYI, Phil at ARM is still looking into why mdct is failing.. will >>>> keep you posted. In the mean time, do you want me to disable NE10 for >>>> mdct_forward and re-submit the patchset so we may make progress? >>>> >>>> Regards, >>>> Vish >>>> >>>> On 30 April 2015 at 09:33, Viswanath Puttagunta >>>> <viswanath.puttagunta at linaro.org> wrote: >>>>> >>>>> On 29 April 2015 at 17:22, Timothy B. Terriberry <tterribe at xiph.org> wrote: >>>>>> >>>>>> Viswanath Puttagunta wrote: >>>>>>> >>>>>>> This patch series is follow up on work I posted on [1]. >>>>>>> In addition to what was posted on [1], this patch series mainly >>>>>>> integrates Fixed point FFT implementations in NE10 library into opus. >>>>>>> You can view my opus wip code at [2]. >>>>>> >>>>>> >>>>>> Thanks, and I apologize for being slow here. >>>>>> >>>>>>> - This was surprising to me because test_unit_dft passes for all >>>>>>> nfft including 60, 120, 240, 480. May be there are some data >>>>>>> corner cases that need further investigation. >>>>>> >>>>>> >>>>>> Yes, that seems concerning. I'd like to at least understand the cause of the failures. "Audio is clearly audible" is not a very strong test for encoder quality regressions, but in the worst case we could just disable NE10 for mdct_forward. >>>>> >>>>> Yes, agree we need to root cause this.. I only mentioned audio is >>>>> audible clearly to point out that NE10 fft is not completely messed >>>>> up, but does have some corner cases where it is failing. ARM folks are >>>>> actively looking into this issue, and expecting some resolution next >>>>> week. >>>>> >>>>> Also, this NE10 fixed fft issue only impacts patches >>>>> armv7,armv8: Optimize fixed point fft using NE10 library >>>>> armv7,armv8: Extend fixed fft NE10 optimizations to mdct >>>>> >>>>> In the mean time, do you want me (as you suggested) disable NE10 fixed >>>>> fft for mdct_forward? That way when it gets fixed in NE10, it will >>>>> just be a one line change patch that will follow. >>>>> >>>>> Either way, request that we make progress on review/merge rest of the patches. >>>> _______________________________________________ >>>> opus mailing list >>>> opus at xiph.org >>>> http://lists.xiph.org/mailman/listinfo/opus >>>>