New issue
Advanced search Search tips

Issue 612215 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Biquad filter cuts off after 200 ms?

Project Member Reported by rtoy@chromium.org, May 16 2016

Issue description

Run the following at hoch.github.io/canopy:

var src = context.createBufferSource()
var b = context.createBuffer(1,1,context.sampleRate);
b.getChannelData(0)[0] = 1;
src.buffer = b;

var f = context.createBiquadFilter();
f.Q.value = 25;
f.frequency.value = 100;

src.connect(f);
f.connect(context.destination);

src.start();

Examine the output waveform at around 200 ms.  The output suddenly cuts off.  This is unexpected.  The value of 200 ms is suspiciously close to the 200 ms tail time value we use for a biquad.  But I don't think we're doing anything special with tail time.
 

Comment 1 by rtoy@chromium.org, Apr 26 2017

To really emphasize the effect set Q to 100 instead of 25.

This happens because the tail time is set to 0.2 and propagatesSilence() returns true after that time, so it thinks the node now propagates silence and makes the output silent.

We need to compute the tail time from the filter coefficients so that this doesn't happen.
Project Member

Comment 2 by bugdroid1@chromium.org, May 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7615be681fe875bc5e6e5715e1455bbffa0a5eb3

commit 7615be681fe875bc5e6e5715e1455bbffa0a5eb3
Author: rtoy <rtoy@chromium.org>
Date: Thu May 18 16:50:57 2017

Compute the tail time for an IIRFilter from its coefficients

For an IIRFilterNode the tail time is currently set to infinity, which is
mathmatically true, but impractical because that keeps the IIRFilter
alive even if the output is very small.

Instead, compute a tail time based on the coefficients of the IIR
filter. The tail time is the estimated time where the impulse response
is sufficiently small that we can say that output is essentially
zero.

By doing so, an IIRFilterNode can disconnect itself from downstreams
nodes, reducing complexity of the graph.  Without this, downstream
nodes stay alive processing silence.

BUG= 612215 
TEST=iirfilter.html

Review-Url: https://codereview.chromium.org/2851873003
Cr-Commit-Position: refs/heads/master@{#472845}

[add] https://crrev.com/7615be681fe875bc5e6e5715e1455bbffa0a5eb3/third_party/WebKit/LayoutTests/webaudio/IIRFilter/iir-tail-time.html
[modify] https://crrev.com/7615be681fe875bc5e6e5715e1455bbffa0a5eb3/third_party/WebKit/LayoutTests/webaudio/IIRFilter/iirfilter.html
[modify] https://crrev.com/7615be681fe875bc5e6e5715e1455bbffa0a5eb3/third_party/WebKit/Source/modules/webaudio/IIRDSPKernel.cpp
[modify] https://crrev.com/7615be681fe875bc5e6e5715e1455bbffa0a5eb3/third_party/WebKit/Source/modules/webaudio/IIRDSPKernel.h
[modify] https://crrev.com/7615be681fe875bc5e6e5715e1455bbffa0a5eb3/third_party/WebKit/Source/platform/audio/IIRFilter.cpp
[modify] https://crrev.com/7615be681fe875bc5e6e5715e1455bbffa0a5eb3/third_party/WebKit/Source/platform/audio/IIRFilter.h

Project Member

Comment 3 by bugdroid1@chromium.org, May 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0be695dcb495cade0ed5c4b70aba95eb3dce49bd

commit 0be695dcb495cade0ed5c4b70aba95eb3dce49bd
Author: rtoy <rtoy@chromium.org>
Date: Tue May 30 19:56:03 2017

Compute tail time from Biquad coefficients

Instead of using a fixed tail time of 200 ms for all biquad filters,
we compute the actual tail time from filter coefficients.  Many
typical filters in applications have a tail time of 10 ms or less.
This allows biquads and downstream nodes to stop processing much sooner
than what currently happens, reducing CPU usage and allowing GC to
collect nodes sooner.

BUG= 612215 
TEST=

Review-Url: https://codereview.chromium.org/2862373002
Cr-Commit-Position: refs/heads/master@{#475632}

[add] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-allpass.html
[add] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-bandpass.html
[add] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-highpass.html
[add] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-highshelf.html
[add] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-lowpass.html
[add] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-lowshelf.html
[add] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-notch.html
[add] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-peaking.html
[add] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/test-tail-time.js
[modify] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/LayoutTests/webaudio/resources/audit-util.js
[modify] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
[modify] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.h
[modify] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/Source/platform/audio/Biquad.cpp
[modify] https://crrev.com/0be695dcb495cade0ed5c4b70aba95eb3dce49bd/third_party/WebKit/Source/platform/audio/Biquad.h

Project Member

Comment 4 by bugdroid1@chromium.org, May 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd4b3bdb8a5e9194b8410df13921b16248dc7009

commit cd4b3bdb8a5e9194b8410df13921b16248dc7009
Author: tyoshino <tyoshino@chromium.org>
Date: Wed May 31 06:05:35 2017

Revert of Compute tail time from Biquad coefficients (patchset #7 id:120001 of https://codereview.chromium.org/2862373002/ )

Reason for revert:
The added layout tests are failing on "WebKit Linux Trusty MSAN" bot.
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20MSAN/builds/1242

Original issue's description:
> Compute tail time from Biquad coefficients
>
> Instead of using a fixed tail time of 200 ms for all biquad filters,
> we compute the actual tail time from filter coefficients.  Many
> typical filters in applications have a tail time of 10 ms or less.
> This allows biquads and downstream nodes to stop processing much sooner
> than what currently happens, reducing CPU usage and allowing GC to
> collect nodes sooner.
>
> BUG= 612215 
> TEST=
>
> Review-Url: https://codereview.chromium.org/2862373002
> Cr-Commit-Position: refs/heads/master@{#475632}
> Committed: https://chromium.googlesource.com/chromium/src/+/0be695dcb495cade0ed5c4b70aba95eb3dce49bd

TBR=hongchan@chromium.org,rtoy@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 612215 

Review-Url: https://codereview.chromium.org/2912293002
Cr-Commit-Position: refs/heads/master@{#475822}

[delete] https://crrev.com/65ef1a1a01feffd43c073cc60fb75de97a567ad2/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-allpass.html
[delete] https://crrev.com/65ef1a1a01feffd43c073cc60fb75de97a567ad2/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-bandpass.html
[delete] https://crrev.com/65ef1a1a01feffd43c073cc60fb75de97a567ad2/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-highpass.html
[delete] https://crrev.com/65ef1a1a01feffd43c073cc60fb75de97a567ad2/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-highshelf.html
[delete] https://crrev.com/65ef1a1a01feffd43c073cc60fb75de97a567ad2/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-lowpass.html
[delete] https://crrev.com/65ef1a1a01feffd43c073cc60fb75de97a567ad2/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-lowshelf.html
[delete] https://crrev.com/65ef1a1a01feffd43c073cc60fb75de97a567ad2/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-notch.html
[delete] https://crrev.com/65ef1a1a01feffd43c073cc60fb75de97a567ad2/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-peaking.html
[delete] https://crrev.com/65ef1a1a01feffd43c073cc60fb75de97a567ad2/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/test-tail-time.js
[modify] https://crrev.com/cd4b3bdb8a5e9194b8410df13921b16248dc7009/third_party/WebKit/LayoutTests/webaudio/resources/audit-util.js
[modify] https://crrev.com/cd4b3bdb8a5e9194b8410df13921b16248dc7009/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
[modify] https://crrev.com/cd4b3bdb8a5e9194b8410df13921b16248dc7009/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.h
[modify] https://crrev.com/cd4b3bdb8a5e9194b8410df13921b16248dc7009/third_party/WebKit/Source/platform/audio/Biquad.cpp
[modify] https://crrev.com/cd4b3bdb8a5e9194b8410df13921b16248dc7009/third_party/WebKit/Source/platform/audio/Biquad.h

Comment 5 by rtoy@chromium.org, May 31 2017

Owner: rtoy@chromium.org
Status: Started (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea0745f9549a61349c37d5e853f8337f4ee79d5e

commit ea0745f9549a61349c37d5e853f8337f4ee79d5e
Author: rtoy <rtoy@chromium.org>
Date: Thu Jun 01 00:41:16 2017

Compute tail time from Biquad coefficients

Instead of using a fixed tail time of 200 ms for all biquad filters,
we compute the actual tail time from filter coefficients.  Many
typical filters in applications have a tail time of 10 ms or less.
This allows biquads and downstream nodes to stop processing much sooner
than what currently happens, reducing CPU usage and allowing GC to
collect nodes sooner.

BUG= 612215 
TEST=

Review-Url: https://codereview.chromium.org/2862373002
Cr-Original-Commit-Position: refs/heads/master@{#475632}
Committed: https://chromium.googlesource.com/chromium/src/+/0be695dcb495cade0ed5c4b70aba95eb3dce49bd
Review-Url: https://codereview.chromium.org/2862373002
Cr-Commit-Position: refs/heads/master@{#476113}

[add] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-allpass.html
[add] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-bandpass.html
[add] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-highpass.html
[add] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-highshelf.html
[add] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-lowpass.html
[add] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-lowshelf.html
[add] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-notch.html
[add] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/tail-time-peaking.html
[add] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/BiquadFilter/test-tail-time.js
[modify] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/LayoutTests/webaudio/resources/audit-util.js
[modify] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
[modify] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.h
[modify] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/Source/platform/audio/Biquad.cpp
[modify] https://crrev.com/ea0745f9549a61349c37d5e853f8337f4ee79d5e/third_party/WebKit/Source/platform/audio/Biquad.h

Comment 7 by rtoy@chromium.org, Jun 2 2017

Status: Fixed (was: Started)

Sign in to add a comment