1221833 - remove direct convolution stage; instead use largest initial FFT stage possible

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[Karl Tomlinson (:karlt)]

Replacing direct convolution with an fft stage of the same size reduces
rendering thread cpu usage by 6% on attachment 8683409 [details].

Removing the first two half-block-size stages reduces rendering thread cpu usage by a further 2.5%.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1221833 reduce FFTConvolver latency by one block r?padenot

bug 1221833 reduce FFTConvolver latency by one block r?padenot

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1221833 replace initial direct convolution stage with fft r?padenot

bug 1221833 replace initial direct convolution stage with fft r?padenot

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1221833 remove first two half-block-size convolver stages r?padenot

bug 1221833 remove first two half-block-size convolver stages r?padenot

Efficiency is proportional to stage size, so start with the largest size
possible.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1221833 remove now-unused direct convolver r?padenot

bug 1221833 remove now-unused direct convolver r?padenot

Comment 5

[Paul Adenot (:padenot)]

(In reply to Karl Tomlinson (ni?:karlt) from comment #2)
> Created attachment 8683446 [details]
> MozReview Request: bug 1221833 replace initial direct convolution stage with
> fft r?padenot
> 
> bug 1221833 replace initial direct convolution stage with fft r?padenot

Why do you think it got initially written using direct computation? The fft size always has been big enough so that doing it in the frequency domain should have been a win, no?

Comment 6

[Paul Adenot (:padenot)]

Comment on attachment 8683445 [details]
MozReview Request: bug 1221833 reduce FFTConvolver latency by one block r?padenot

https://reviewboard.mozilla.org/r/24351/#review21867

Comment 7

[Paul Adenot (:padenot)]

Comment on attachment 8683446 [details]
MozReview Request: bug 1221833 replace initial direct convolution stage with fft r?padenot

https://reviewboard.mozilla.org/r/24353/#review21869

Comment 8

[Paul Adenot (:padenot)]

Comment on attachment 8683447 [details]
MozReview Request: bug 1221833 remove first two half-block-size convolver stages r?padenot

https://reviewboard.mozilla.org/r/24355/#review21871

Comment 9

[Paul Adenot (:padenot)]

Comment on attachment 8683448 [details]
MozReview Request: bug 1221833 remove now-unused direct convolver r?padenot

https://reviewboard.mozilla.org/r/24357/#review21873

Comment 10

[Karl Tomlinson (:karlt)]

(In reply to Paul Adenot (:padenot) from comment #5)
> Why do you think it got initially written using direct computation? The fft
> size always has been big enough so that doing it in the frequency domain
> should have been a win, no?

The first FFT stage size was the same size as the direct stage, so, yes, I don't think the size of the stage was the reason for choosing to use the direct computation.

I suspect the reason for using the direct stage was just that the FFT stage implementation didn't support zero latency at any stage sizes.

I haven't looked at the code history but the existence of the Reverb/ReverbConvolver::latencyFrames() methods (to be removed in bug 1221831) and the reverbTotalLatency variable in the ReverbConvolver constructor might indicate that these were once non-zero.  Perhaps, the FFTConvolver implementation may have been designed for a more general situation with variable block sizes.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/26eaa1b7e878
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe2266a55ca1
https://hg.mozilla.org/integration/mozilla-inbound/rev/b69c165bf3c0
https://hg.mozilla.org/integration/mozilla-inbound/rev/e66105937eef

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/26eaa1b7e878
https://hg.mozilla.org/mozilla-central/rev/fe2266a55ca1
https://hg.mozilla.org/mozilla-central/rev/b69c165bf3c0
https://hg.mozilla.org/mozilla-central/rev/e66105937eef