opus - May 2015 - [RFC PATCH v1 0/8] Ne10 fft fixed and previous

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.

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.

Viswanath Puttagunta wrote:
> 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?
Yes, let's do that for now.

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
>