1281382 - setTargetAtTime uses incorrect starting value when earlier event is skipped

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[Karl Tomlinson (:karlt)]

Attachment: testcase

AudioEventTimeline assumes that the current value will be fetched for all
ticks, but this does not happen for processing blocks where the AudioParam is
not required.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: actual screenshot

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: double-audioparam-event-graph-3-expected.png

Comment 3

[Dan Minor [:dminor]]

Attachment: Patch to update mLastComputedEvent when cleaning up "old" events.

This depends upon the fix for Bug 1113634.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: testcase with two setTarget events

Comment 5

[Karl Tomlinson (:karlt)]

Comment on attachment 8780647 [details] [diff] [review]
Patch to update mLastComputedEvent when cleaning up "old" events.

See attachment 8781012 [details] for a problem with this.

mLastComputedValue should contain the value from the curve of the previous event at the time of the current event.  It need only be calculated when the current event is a SetTarget.

Comment 6

[Dan Minor [:dminor]]

Attachment: Update mLastComputedEvent when cleaning up "old" events.

This only updates mLastComputedValue if the next event is a SetTarget, and uses the time of the SetTarget event.

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8781109 [details] [diff] [review]
Update mLastComputedEvent when cleaning up "old" events.

>+        mLastComputedValue = GetValuesAtTimeHelperInternal(mEvents[1].template Time<TimeType>(), &mEvents[0], &mEvents[1]);

Wrap to keep within 80 columns.

I suggest passing nullptr for aNext to make it clear that mEvents[1] has no other effect, but what you have still works, if you prefer.

Comment 8

[Dan Minor [:dminor]]

Try job here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=821e962ac79d18593987c7a9a0e44f69a2cde8bc

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/272a2ab0d60d
Fix setTargetAtTime using incorrect starting value when earlier event is skipped; r=karlt

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/272a2ab0d60d