1313058 - SetValueCurveAtTime interpolation is incorrect

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P2)

Description

[Dan Minor [:dminor]]

For instance, if the curve is [1.0, 0.0] the values are interpolated from 1.0 to 0.0 over the first half of the duration, and are 0.0 for the second half of the duration. This is likely related to the old behaviour of jumping to the next value once we are closer to it than the current value.

Comment 1

[Dan Minor [:dminor]]

Attachment: Bug 1313058 - Fix SetValueCurveAtTime interpolation;

This interpolates over aCurveLength - 1 steps rather than over
aCurveLength steps as was done before.

Previously we would reach the final value on the curve before
the end of the specified duration. For example, on the curve
[1.0, 0.0] with duration 1000, we would interpolate from 1.0 to
0.0 by time 500 rather than time 1000. With these changes, we
don't reach 0.0 until time 1000, as expected.

This also updates TestSpecExample in TestAudioEventTimeline.cpp
to match the curve in the latest spec.

Review commit: https://reviewboard.mozilla.org/r/88954/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/88954/

Comment 2

[Dan Minor [:dminor]]

Sorry I messed this up the first time around :/.

I also tried the [1.0, 0.5, 0.75, 0.25] curve on an 128 sample OfflineAudioContext buffer and compared results to Chrome to ensure that with the new changes we match what they are doing.

Comment 3

[Dan Minor [:dminor]]

Try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51f9819f0b9f8d52216345ff96758729f60ae63f

Comment 4

[Paul Adenot (:padenot)]

Comment on attachment 8805105 [details]
Bug 1313058 - Fix SetValueCurveAtTime interpolation;

https://reviewboard.mozilla.org/r/88954/#review89648

::: dom/media/webaudio/compiledtest/TestAudioEventTimeline.cpp:108
(Diff revision 1)
>    ErrorResultMock rv;
>  
> -  float curve[] = { -1.0f, 0.0f, 1.0f };
> +  uint32_t curveLength = 44100;
> +  float* curve = new float[curveLength];
> +  for (uint32_t i = 0; i < curveLength; ++i) {
> +    curve[i] = sin(3.14159265358979 * i / float(curveLength));

M_PI ?

Comment 5

[Dan Minor [:dminor]]

Comment on attachment 8805105 [details]
Bug 1313058 - Fix SetValueCurveAtTime interpolation;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/88954/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9bb46ea431c6
Fix SetValueCurveAtTime interpolation; r=padenot

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9bb46ea431c6