1503950 - when more than two AudioParam events of the same type (e.g. setValueAtTime) are added at the same time, the latter events are ignored

Status: VERIFIED FIXED

(Core :: Web Audio, defect, P1)

Description

[RokaWar]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.77 Safari/537.36

Steps to reproduce:

Hello,

This bug is for me the reason of this problem https://bugzilla.mozilla.org/show_bug.cgi?id=1502748 that will be fixed in the planned patch (63.0.2)

For me, something is wrong on your WEBAudio / setValueAtTime

Test this only on Firefox beta 64.0b5 or Nightly 65.0a1

My test is made on a windows machine.

1 : Open https://www.asr-games.net/testing/BuildTest2018.1.3f1/ (on Firefox beta 64.0b5 or Nightly 65.0a1)
2 : Click "play sound" (you hear the sound)
3 : move the slider to the left (volume will be at 0)
4 : Click "play sound" (you do not hear any sound)
5 : This version use "WEBAudio.audioInstances[channelInstance].gain.gain.value = v;"
6 : Everything is correct

now,

1 : Open https://www.asr-games.net/testing/BuildTest2018.1.4f1/ (on Firefox beta 64.0b4 or Nightly 65.0a1)
2 : Click "play sound" (you hear the sound)
3 : move the slider to the left (volume will be at 0)
4 : Click "play sound" (you hear now a crackling sound event with the volume at 0)
5 : this version use WEBAudio.audioInstances[channelInstance].gain.gain.setValueAtTime(v, WEBAudio.audioContext.currentTime);
6 : This one is not correct

Thank you




Actual results:

When you play a sound with WebAudio (webgl) by using setValueAtTime you hear a crackling sound event with the volume set at 0


Expected results:

We should get the same result by using "WEBAudio.audioInstances[channelInstance].gain.gain.value = v;" or WEBAudio.audioInstances[channelInstance].gain.gain.setValueAtTime(v, WEBAudio.audioContext.currentTime);

Comment 1

[RokaWar]

Must be for lhansen and padenot according https://bugzilla.mozilla.org/show_bug.cgi?id=1502748

Thank you

Comment 2

[Lars T Hansen [:lth]]

Deferring to Paul here.

Comment 3

[Karl Tomlinson (:karlt)]

Bug 1502748 does not reproduce on 64.

What is the evidence that suggests that this is the problem behind bug 1502748?

Comment 4

[RokaWar]

(In reply to Karl Tomlinson (:karlt) from comment #3)
> Bug 1502748 does not reproduce on 64.
> 
> What is the evidence that suggests that this is the problem behind bug
> 1502748?

I'm not sure at 100% but Paul have landed a wrong patches on central.
If his "correct" patch would have been released instead of the wrong one, the same problem would happen on 63 too (instead of crashing).

Paul will have the answer ^^

Comment 5

[RokaWar]

(In reply to Karl Tomlinson (:karlt) from comment #3)
> Bug 1502748 does not reproduce on 64.
> 
> What is the evidence that suggests that this is the problem behind bug
> 1502748?

Also because i have patched one of my game (not using setValueAtTime) and the crash do not happen in 63 anymore.

Comment 6

[Karl Tomlinson (:karlt)]

Thanks.  Confirming that the change of behavior was triggered by one of these changesets:
https://hg.mozilla.org/integration/mozilla-inbound/log?rev=ancestors(f58ddafd8a65)-ancestors(765f2d8c3340)

That is the second set of landings for bug 1308436.

I also checked that 2b7a5180b902, the revision before the first set of landings, produced the expected silence,
and so the backout proposed for bug 1502748 should not trigger this change.

I don't understand why https://hg.mozilla.org/mozilla-central/rev/f58ddafd8a65 shows milestone 63.0a1, but https://hg.mozilla.org/mozilla-central/log?rev=ancestors(FIREFOX_BETA_63_BASE)-ancestors(f58ddafd8a65) is empty and so this bug should only affect 64.

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to Karl Tomlinson (:karlt) from comment #6)
> so this bug should only affect

64 and subsequent.

Comment 8

[Karl Tomlinson (:karlt)]

The change was triggered by https://hg.mozilla.org/integration/mozilla-inbound/rev/65901b920626

Comment 9

[Karl Tomlinson (:karlt)]

Google Chrome beta 71.0.3578.20 produces a crack on the first "play sound" on both https://www.asr-games.net/testing/BuildTest2018.1.3f1/ and https://www.asr-games.net/testing/BuildTest2018.1.4f1/ when step 2 is skipped.

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: MOZ_LOG=WebAudioAPI:4 output for BuildTest2018.1.4f1

I guess this may be related to having two events at the same time.
https://hg.mozilla.org/integration/mozilla-inbound/rev/65901b920626#l2.127

Comment 11

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=07791c79457420c1976e579b2c79f49ffd5b346b
We'll need a test for this too.

Comment 12

[Karl Tomlinson (:karlt)]

Attachment: Bug 1503950 remove obsolete expected subtest failures r?padenot

These have been lingering due to bug 1474463.

Comment 13

[Karl Tomlinson (:karlt)]

Attachment: Bug 1503950 test 3 events of the same type at the same time r?padenot

Depends on D10860

Comment 14

[Karl Tomlinson (:karlt)]

Attachment: Bug 1503950 insert new timeline events after existing events at the same time even when of the same type r?padenot

Depends on D10861

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de1d24ff32ae
remove obsolete expected subtest failures r=padenot
https://hg.mozilla.org/integration/autoland/rev/bf705d04c328
test 3 events of the same type at the same time r=padenot
https://hg.mozilla.org/integration/autoland/rev/ca875dd78b3c
insert new timeline events after existing events at the same time even when of the same type r=padenot

Comment 16

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13935 for changes under testing/web-platform/tests

Comment 17

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 18

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/de1d24ff32ae
https://hg.mozilla.org/mozilla-central/rev/bf705d04c328
https://hg.mozilla.org/mozilla-central/rev/ca875dd78b3c

Comment 19

[Karl Tomlinson (:karlt)]

Comment on attachment 9022507 [details]
Bug 1503950 insert new timeline events after existing events at the same time even when of the same type r?padenot

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1308436

User impact if declined: Games will produce sound when they should not.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Small logic change and affects only web audio.

String changes made/needed: none

Comment 20

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged

Comment 21

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9022507 [details]
Bug 1503950 insert new timeline events after existing events at the same time even when of the same type r?padenot

webaudio regression fix, approved for 64.0b9

Comment 22

[Razvan Maries]

https://hg.mozilla.org/releases/mozilla-beta/rev/42b57103a046

Comment 23

[Anca Soncutean, Desktop QA]

I reproduced this issue with Fx 65.0a1 (20181101100640) on Windows 10 x64. 
I can confirm the issue is fixed on latest Nightly Fx 65.0a1 (20181112220107) and latest Beta Fx 64.0b9 (20181112164519) under Windows 10 x64, macOS 10.13 and Ubuntu 18.06 x64.