1220037 - corrupted convolution from out-of-position nyquist DFT coefficient

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[Karl Tomlinson (back July 13 :karlt)]

FFTBlock was not written for packed Nyquist format as used by libav FFT.

BufferComplexMultiply knows nothing about this format and so ends up corrupting the DC coefficient when Nyquists are multiplied.

Comment 1

[Karl Tomlinson (back July 13 :karlt)]

Attachment: MozReview Request: bug 1220037 pack and unpack Nyquist for MOZ_LIBAV_FFT r?padenot

bug 1220037 pack and unpack Nyquist for MOZ_LIBAV_FFT r?padenot

BufferComplexMultiply knows nothing about this format and so ends up
corrupting the DC coefficient if packed Nyquists are multiplied.

Comment 2

[Karl Tomlinson (back July 13 :karlt)]

Attachment: MozReview Request: bug 1220037 test convolution r?padenot

bug 1220037 test convolution r?padenot

This is in the mochitest suite so that Android and B2G tests can run it, but
designed so that it can be moved to web-platform-tests when they run on all
platforms.

Comment 3

[Paul Adenot (:padenot)]

Comment on attachment 8681077 [details]
MozReview Request: bug 1220037 pack and unpack Nyquist for MOZ_LIBAV_FFT r?padenot

https://reviewboard.mozilla.org/r/23747/#review21497

Comment 4

[Paul Adenot (:padenot)]

Comment on attachment 8681078 [details]
MozReview Request: bug 1220037 test convolution r?padenot

https://reviewboard.mozilla.org/r/23749/#review21495

::: dom/media/webaudio/test/test_convolverNodeDelay.html:64
(Diff revision 1)
> +// The 5/4 ratio rovides sampling across a range of delays and offsets within

s/rovides/provides/

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7fbc38f3b73c
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a7d7f86ff60

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7fbc38f3b73c
https://hg.mozilla.org/mozilla-central/rev/9a7d7f86ff60

Comment 7

[Karl Tomlinson (back July 13 :karlt)]

Comment on attachment 8681077 [details]
MozReview Request: bug 1220037 pack and unpack Nyquist for MOZ_LIBAV_FFT r?padenot

Approval Request Comment
[Feature/regressing bug #]: regression from bug 1157768
[User impact if declined]: distorted audio on desktop platforms when some reverb effects
[Describe test coverage new/current, TreeHerder]: new test.
[Risks and why]: small patch and test give low risk and confidence.
[String/UUID change made/needed]: none.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8681077 [details]
MozReview Request: bug 1220037 pack and unpack Nyquist for MOZ_LIBAV_FFT r?padenot

Let's take it.
Should be in 43 beta 2.
Please land also the test.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dfdf94bfd152
https://hg.mozilla.org/releases/mozilla-aurora/rev/779eec6a0b9e

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ab27c2db7e68
https://hg.mozilla.org/releases/mozilla-beta/rev/d3b021968c63

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/dfdf94bfd152
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/779eec6a0b9e