Revert of Compute tail time from Biquad coefficients

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

Reason for revert:
Breaks WebKit Linux Trusty MSAN: 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

[kjellander_chromium, 2017-05-31 13:16:26]

The CQ bit was checked by kjellander@chromium.org

[kjellander_chromium, 2017-05-31 13:16:26]

Created Revert of Compute tail time from Biquad coefficients

[commit-bot: I haz the power, 2017-05-31 13:16:39]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 13:16:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-31 13:16:58]

Failed to apply patch for
third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp:
While running git apply --index -3 -p1;
  error: patch failed:
third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp:31
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp: patch
does not apply

Patch:       third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
Index: third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
b/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
index
e45539e60f03b6d33cdcb838976eb8a5658f2f6a..e5384a5606c81133918d0a94324a91ec930483aa
100644
--- a/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
@@ -31,6 +31,13 @@
 
 namespace blink {
 
+// FIXME: As a recursive linear filter, depending on its parameters, a biquad
+// filter can have an infinite tailTime. In practice, Biquad filters do not
+// usually (except for very high resonance values) have a tailTime of longer
+// than approx. 200ms. This value could possibly be calculated based on the
+// settings of the Biquad.
+static const double kMaxBiquadDelayTime = 0.2;
+
 void BiquadDSPKernel::UpdateCoefficientsIfNecessary(int frames_to_process) {
   if (GetBiquadProcessor()->FilterCoefficientsDirty()) {
     float cutoff_frequency[AudioUtilities::kRenderQuantumFrames];
@@ -114,23 +121,6 @@
         break;
     }
   }
-
-  UpdateTailTime(number_of_frames - 1);
-}
-
-void BiquadDSPKernel::UpdateTailTime(int coef_index) {
-  // A reasonable upper limit for the tail time.  While it's easy to
-  // create biquad filters whose tail time can be much larger than
-  // this, limit the maximum to this value so that we don't keep such
-  // nodes alive "forever".
-  // TODO: What is a reasonable upper limit?
-  const double kMaxTailTime = 30;
-
-  double sample_rate = SampleRate();
-  double tail =
-      biquad_.TailFrame(coef_index, kMaxTailTime * sample_rate) / sample_rate;
-
-  tail_time_ = clampTo(tail, 0.0, kMaxTailTime);
 }
 
 void BiquadDSPKernel::Process(const float* source,
@@ -209,7 +199,7 @@
 }
 
 double BiquadDSPKernel::TailTime() const {
-  return tail_time_;
+  return kMaxBiquadDelayTime;
 }
 
 double BiquadDSPKernel::LatencyTime() const {

[kjellander_chromium, 2017-05-31 13:17:28]

Description was changed from

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

Reason for revert:
Breaks WebKit Linux Trusty MSAN:
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty...
WebKit Linux Trusty MSAN

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/+/0be695dcb495cade0ed5c4b70aba...

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
==========

to

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

Reason for revert:
Breaks WebKit Linux Trusty MSAN:
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty...

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/+/0be695dcb495cade0ed5c4b70aba...

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
==========

[kjellander_chromium, 2017-05-31 13:17:37]

The CQ bit was checked by kjellander@chromium.org

[commit-bot: I haz the power, 2017-05-31 13:17:54]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 13:18:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-31 13:18:09]

Failed to apply patch for
third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp:
While running git apply --index -3 -p1;
  error: patch failed:
third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp:31
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp: patch
does not apply

Patch:       third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
Index: third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
b/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
index
e45539e60f03b6d33cdcb838976eb8a5658f2f6a..e5384a5606c81133918d0a94324a91ec930483aa
100644
--- a/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
@@ -31,6 +31,13 @@
 
 namespace blink {
 
+// FIXME: As a recursive linear filter, depending on its parameters, a biquad
+// filter can have an infinite tailTime. In practice, Biquad filters do not
+// usually (except for very high resonance values) have a tailTime of longer
+// than approx. 200ms. This value could possibly be calculated based on the
+// settings of the Biquad.
+static const double kMaxBiquadDelayTime = 0.2;
+
 void BiquadDSPKernel::UpdateCoefficientsIfNecessary(int frames_to_process) {
   if (GetBiquadProcessor()->FilterCoefficientsDirty()) {
     float cutoff_frequency[AudioUtilities::kRenderQuantumFrames];
@@ -114,23 +121,6 @@
         break;
     }
   }
-
-  UpdateTailTime(number_of_frames - 1);
-}
-
-void BiquadDSPKernel::UpdateTailTime(int coef_index) {
-  // A reasonable upper limit for the tail time.  While it's easy to
-  // create biquad filters whose tail time can be much larger than
-  // this, limit the maximum to this value so that we don't keep such
-  // nodes alive "forever".
-  // TODO: What is a reasonable upper limit?
-  const double kMaxTailTime = 30;
-
-  double sample_rate = SampleRate();
-  double tail =
-      biquad_.TailFrame(coef_index, kMaxTailTime * sample_rate) / sample_rate;
-
-  tail_time_ = clampTo(tail, 0.0, kMaxTailTime);
 }
 
 void BiquadDSPKernel::Process(const float* source,
@@ -209,7 +199,7 @@
 }
 
 double BiquadDSPKernel::TailTime() const {
-  return tail_time_;
+  return kMaxBiquadDelayTime;
 }
 
 double BiquadDSPKernel::LatencyTime() const {

[kjellander_chromium, 2017-05-31 17:31:02]

On 2017/05/31 13:18:09, commit-bot: I haz the power wrote:
> Failed to apply patch for
> third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp:
> While running git apply --index -3 -p1;
>   error: patch failed:
> third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp:31
>   error: repository lacks the necessary blob to fall back on 3-way merge.
>   error: third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp: patch
> does not apply
> 
> Patch:       third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
> Index: third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
> diff --git a/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
> b/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
> index
>
e45539e60f03b6d33cdcb838976eb8a5658f2f6a..e5384a5606c81133918d0a94324a91ec930483aa
> 100644
> --- a/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
> +++ b/third_party/WebKit/Source/modules/webaudio/BiquadDSPKernel.cpp
> @@ -31,6 +31,13 @@
>  
>  namespace blink {
>  
> +// FIXME: As a recursive linear filter, depending on its parameters, a biquad
> +// filter can have an infinite tailTime. In practice, Biquad filters do not
> +// usually (except for very high resonance values) have a tailTime of longer
> +// than approx. 200ms. This value could possibly be calculated based on the
> +// settings of the Biquad.
> +static const double kMaxBiquadDelayTime = 0.2;
> +
>  void BiquadDSPKernel::UpdateCoefficientsIfNecessary(int frames_to_process) {
>    if (GetBiquadProcessor()->FilterCoefficientsDirty()) {
>      float cutoff_frequency[AudioUtilities::kRenderQuantumFrames];
> @@ -114,23 +121,6 @@
>          break;
>      }
>    }
> -
> -  UpdateTailTime(number_of_frames - 1);
> -}
> -
> -void BiquadDSPKernel::UpdateTailTime(int coef_index) {
> -  // A reasonable upper limit for the tail time.  While it's easy to
> -  // create biquad filters whose tail time can be much larger than
> -  // this, limit the maximum to this value so that we don't keep such
> -  // nodes alive "forever".
> -  // TODO: What is a reasonable upper limit?
> -  const double kMaxTailTime = 30;
> -
> -  double sample_rate = SampleRate();
> -  double tail =
> -      biquad_.TailFrame(coef_index, kMaxTailTime * sample_rate) /
sample_rate;
> -
> -  tail_time_ = clampTo(tail, 0.0, kMaxTailTime);
>  }
>  
>  void BiquadDSPKernel::Process(const float* source,
> @@ -209,7 +199,7 @@
>  }
>  
>  double BiquadDSPKernel::TailTime() const {
> -  return tail_time_;
> +  return kMaxBiquadDelayTime;
>  }
>  
>  double BiquadDSPKernel::LatencyTime() const {

Sorry about the spam, I didn't realize this was already reverted and that this
bot had 5 hour cycle times. I'll make sure to check more carefully next time.