Commit a3c8a81f authored by Raymond Toy's avatar Raymond Toy Committed by Commit Bot

Refine condition when setTargetAtTime has converged

The function HasSetTargetConverged was too restrictive and would
continue to say the setTargetAtTime event was not converged even
though the increment was so small as make no change in the value.

Thus, adjust the criteria so that if the increment is too small to
affect the value, consider the event has having converged.

There's also a bug in HandleAllEventsInThePast where setTargetAtTime
has converged and we've updated the default value but the timeline
would return the old default value instead of the new converged
default value.

A couple of tests needed to be updated due to the change in the
convergence criterion.

Ran the test https://jsfiddle.net/8vo0gbav/52/ and the CPU now no
longer goes to 100% as it did without this change.

Bug: 813504
Change-Id: I506b31289b5b40380147d231d8b2ea41785d6600
Reviewed-on: https://chromium-review.googlesource.com/940273Reviewed-by: default avatarHongchan Choi <hongchan@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539900}
parent d91a592a
......@@ -23,10 +23,10 @@
// Find the time where setTargetAtTime is close enough to the limit.
// Since we're approaching 1, use a value of eps smaller than
// kSetTargetThreshold (5e-7) in AudioParamTimeline.cpp. This is to
// kSetTargetThreshold (1.5e-6) in AudioParamTimeline.cpp. This is to
// account for round-off in the actual implementation (which uses a
// filter and not the formula.)
let limitThreshold = 1e-7;
let limitThreshold = 1e-6;
runTest(should, {
sampleRate: sampleRate,
......@@ -35,7 +35,8 @@
timeConstant: timeConstant,
eps: limitThreshold,
// Experimentally determined
threshold: 2.4e-5
threshold: 2.4e-5,
tailThreshold: {relativeThreshold: 9.8234e-7}
}).then(() => task.done());
})
......@@ -70,7 +71,8 @@
timeConstant: timeConstant,
eps: limitThreshold,
// Experimentally determined
threshold: 1.3e-7
threshold: 1.3e-7,
tailThreshold: {absoluteThreshold: 9.7950e-22}
}).then(() => task.done());
});
......@@ -148,7 +150,7 @@
{absoluteThreshold: options.threshold});
should(actual.slice(tailFrame), 'Tail output for ' + message)
.containValues([options.v1]);
.beCloseToArray(expected.slice(tailFrame), options.tailThreshold);
});
}
......
......@@ -27,7 +27,7 @@
// Max allowed difference between the rendered data and the expected
// result.
let maxAllowedError = 1.6953e-6;
let maxAllowedError = 6.5683e-4
// The AudioGainNode starts with this value instead of the default value.
let initialValue = 100;
......
......@@ -1147,14 +1147,38 @@ void AudioParamTimeline::ClampNewEventsToCurrentTime(double current_time) {
// Test that for a SetTarget event, the current value is close enough
// to the target value that we can consider the event to have
// converged to the target.
static bool HasSetTargetConverged(float value, float target) {
return fabs(value - target) < kSetTargetThreshold * fabs(target) ||
static bool HasSetTargetConverged(float value,
float target,
float discrete_time_constant) {
// Let c = |discrete_time_constant|. Then SetTarget computes
//
// new value = value + (target - value) * c
// = value * (1 + (target - value)*c/value)
//
// We consider the value converged if (target - value) * c is
// sufficiently small so as not to change value. This happens if
// (target-value)*c/value is a small value, say, eps. Thus, we've converged
// if
//
// |(target-value)*c/value| < eps
// or
// |target-value|*c < eps*|value|
//
// However, if target is zero, we need to be careful:
//
// new value = value + (0 - value) * c
// = value * (1 - c)
//
// So the new value is sufficiently close to zero if |value|*(1-c)
// is close enough to zero.
return fabs(target - value) * discrete_time_constant <
kSetTargetThreshold * fabs(value) ||
(target == 0 && fabs(value) < kSetTargetZeroThreshold);
}
bool AudioParamTimeline::HandleAllEventsInThePast(double current_time,
double sample_rate,
float default_value,
float& default_value,
unsigned number_of_values,
float* values) {
// Optimize the case where the last event is in the past.
......@@ -1174,7 +1198,11 @@ bool AudioParamTimeline::HandleAllEventsInThePast(double current_time,
// we're at least 5 time constants past the start of the event. If not, we
// have to continue processing it.
if (last_event_type == ParamEvent::kSetTarget) {
if (HasSetTargetConverged(default_value, last_event->Value()) &&
float discrete_time_constant =
static_cast<float>(AudioUtilities::DiscreteTimeConstantForSampleRate(
last_event->TimeConstant(), sample_rate));
if (HasSetTargetConverged(default_value, last_event->Value(),
discrete_time_constant) &&
current_time > last_event_time + 5 * last_event->TimeConstant()) {
// We've converged. Slam the default value with the target value.
default_value = last_event->Value();
......@@ -1543,7 +1571,7 @@ std::tuple<size_t, float, unsigned> AudioParamTimeline::ProcessSetTarget(
// If the value is close enough to the target, just fill in the data
// with the target value.
if (HasSetTargetConverged(value, target)) {
if (HasSetTargetConverged(value, target, discrete_time_constant)) {
for (; write_index < fill_to_frame; ++write_index)
values[write_index] = target;
} else {
......
......@@ -358,9 +358,10 @@ class AudioParamTimeline {
// Handle the case where the last event in the timeline is in the
// past. Returns false if any event is not in the past. Otherwise,
// return true and also fill in |values| with |defaultValue|.
// |defaultValue| may be updated with a new value.
bool HandleAllEventsInThePast(double current_time,
double sample_rate,
float default_value,
float& default_value,
unsigned number_of_values,
float* values);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment