1695161 - Firefox truncates web audio convolver effects

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P2)

Description

[Tomas Eriksson]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:85.0) Gecko/20100101 Firefox/85.0

Steps to reproduce:

Tested multiple sites, choosed a long reverb convolver effect

For example https://drumbit.app/ - Effect Large Hall

Actual results:

The convolver sound is truncated, fading out quickly

Expected results:

As other browsers, seconds long reverb effects should played as they are

Comment 1

[Tomas Eriksson]

Simple example, play, turn up reverb to 1, stop. My Firefox has a much shorter reverb than Chrome.

https://middleearmedia.com/demos/webaudio/convolver.html

Comment 2

[Hani Yacoub, Desktop QA]

Managed to reproduce this issue on Windows 10 x64, ubuntu 20.04 x64 and on macOS 10.15.

Comment 3

[Karl Tomlinson (:karlt)]

Thank you for reporting. This is clearly demonstrated with attachment 8990853 [details].

Comment 4

[Karl Tomlinson (:karlt)]

This was being tested, but the expected results were changed in https://hg.mozilla.org/integration/autoland/rev/ef5ccdbf1ccf#l3.3

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1695161 update m_writeIndex even when it would not wrap to buffer start r?Gankra

Comment 6

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/jobs?repo=try&revision=7dbbd4c59576be693ead180b6113ae9e048ce895

Comment 7

[Mathew Hodson]

(In reply to Karl Tomlinson (:karlt) from comment #4)

This was being tested, but the expected results were changed in https://hg.mozilla.org/integration/autoland/rev/ef5ccdbf1ccf#l3.3

That bug just updated the test, so I don't think it's correct to mark it in the Regressed by field. It looks like the issue was caused by the change in bug 1682938 in Firefox 86.

Comment 8

[Karl Tomlinson (:karlt)]

(In reply to Mathew Hodson from comment #7)

It looks like the issue was caused by the change in bug 1682938 in Firefox 86.

Yes, that change caused the problem, but that change would have been backed out if the test result showing this bug hadn't been explicitly ignored. i.e. both changes were required for this bug to ship.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/194fe6d098dd
update m_writeIndex even when it would not wrap to buffer start r=Gankra

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/194fe6d098dd

Comment 11

[Karl Tomlinson (:karlt)]

Comment on attachment 9207668 [details]
Bug 1695161 update m_writeIndex even when it would not wrap to buffer start r?Gankra

Beta/Release Uplift Approval Request

• User impact if declined: Regression in reverb effects, which can demonstrate as an abrupt truncation of sound.
  A difference can be observed with these STR:

1. Load http://reverbjs.org/
2. Click on AbernyteGrainSilo "(synth)"

• 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 code change restoring previous behavior.
• String changes made/needed: None.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9207668 [details]
Bug 1695161 update m_writeIndex even when it would not wrap to buffer start r?Gankra

IMO we should also take this on ESR since it's a recently-introduced regression. I've already verified that it grafts cleanly and realtime-conv.html passes.

Comment 13

[Karl Tomlinson (:karlt)]

Yes, I agree we want to fix this on esr78.

There we should revert https://hg.mozilla.org/releases/mozilla-esr78/rev/abb664ecc9fd instead of removing realtime-conv.html.ini, because https://hg.mozilla.org/mozilla-central/rev/0194a0d1befc is not on 78, so linux32 tests will not pass.

Comment 14

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

Comment on attachment 9207668 [details]
Bug 1695161 update m_writeIndex even when it would not wrap to buffer start r?Gankra

approved for 87.0b9

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9207668 [details]
Bug 1695161 update m_writeIndex even when it would not wrap to buffer start r?Gankra

Approved for 78.9esr.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/53999ac8db91

Comment 17

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

https://hg.mozilla.org/releases/mozilla-beta/rev/8bc93ab4e461