corrupted convolution from out-of-position nyquist DFT coefficient

RESOLVED FIXED in Firefox 43

Status

()

Product:
Core
Component:
Web Audio
Importance:
P1
normal
Rank:
5
Status:
RESOLVED FIXED
Reported:
3 years ago
Modified:
3 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

({regression})

Version:
42 Branch
Target:
mozilla45
Keywords:
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox43 fixed, firefox44 fixed, firefox45 fixed, b2g-v2.5 fixed)

Details

Attachments

(2 attachments)

MozReview Request: bug 1220037 pack and unpack Nyquist for MOZ_LIBAV_FFT r?padenot
40 bytes, text/x-review-board-request
padenot
: review+
sylvestre
: approval-mozilla-aurora+
sylvestre
: approval-mozilla-beta+
Details
MozReview Request: bug 1220037 test convolution r?padenot
40 bytes, text/x-review-board-request
padenot
: review+
Details
(Assignee)

Description

•
3 years ago 
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.
(Assignee)

Comment 1

•
3 years ago 
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.
Attachment #8681077 - Flags: review?(padenot)
(Assignee)

Comment 2

•
3 years ago 
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.
Attachment #8681078 - Flags: review?(padenot)

Comment 3

•
3 years ago 
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
Attachment #8681077 - Flags: review?(padenot) → review+
Attachment #8681078 - Flags: review?(padenot) → review+

Comment 4

•
3 years ago 
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 6

•
3 years ago
bugherder 
https://hg.mozilla.org/mozilla-central/rev/7fbc38f3b73c
https://hg.mozilla.org/mozilla-central/rev/9a7d7f86ff60
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
 (Assignee)

Updated

•
3 years ago
Flags: in-testsuite+
 (Assignee)

Comment 7

•
3 years ago 
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.
Attachment #8681077 - Flags: approval-mozilla-beta?
Attachment #8681077 - Flags: approval-mozilla-aurora?
status-firefox43: --- → affected
status-firefox44: --- → affected

Comment 8

•
3 years ago 
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.
Attachment #8681077 - Flags: approval-mozilla-beta?
Attachment #8681077 - Flags: approval-mozilla-beta+
Attachment #8681077 - Flags: approval-mozilla-aurora?
Attachment #8681077 - Flags: approval-mozilla-aurora+

Updated

•
3 years ago
Depends on: 1223520
You need to log in before you can comment on or make changes to this bug.