1688713 - Sawtooth wave form generated when joining Zoom Webclient in 86.0a1

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P2)

Description

[Mark Richards [:freshness]]

Ekr reached out this morning to report an audio buzzing (sawtooth wave form) being generated when connecting to Zoom webclient meetings from Nightly. We were able to reproduce the issue on Nightly 86.0a1, and confirmed that audio works fine on GA and Beta.

From the Zoom side, no updates to the Web Client were performed, according to their web page and our contacts within Zoom.

Looking for a mozregression to be performed to see what was the last stable version of Nightly with the Zoom Web Client.

FWIW, Google Meet does not have this issue.

Comment 1

[William Lachance (:wlach)]

I'm not sure what component this should be under, but mozregression ain't it. :) If you want someone to use mozregression to reproduce your bug, try using the regressionwindow-wanted keyword.

Comment 2

[Mark Richards [:freshness]]

Apologies, Thank you!

Comment 3

[William Lachance (:wlach)]

(In reply to Mark Richards [:freshness] from comment #2)

Apologies, Thank you!

No need to resolve the issue. I moved it to Firefox :: Untriaged (which hopefully means someone will look at it and put it in the right place)

Comment 4

[Daniel Bodea [:danibodea]]

I have been trying to reproduce the issue you reported on Windows 10, but I wasn't able to.
Could you answer some questions to clear out any misunderstandings?

1. Can you describe, or even record your buzzing?
2. Which operating system are you using?
3. Does it still reproduce while in safe mode?
   (https://support.mozilla.org/en-US/kb/troubleshoot-extensions-themes-to-fix-problems)
4. Alternatively, you could help us by investigating for the regressor using mozregression.
   (https://mozilla.github.io/mozregression/install.html)

Comment 5

[Daniel Bodea [:danibodea]]

P.S. mozregression does not work on Mac OS 11.

Comment 6

[Daniel Bodea [:danibodea]]

Attachment: audio recording of the sound the webclient on Mac OS does when unmuting the the android phone native app

I have found a constant crispy sound in the following situation:
I have an android phone connected using the Zoom native app to a Mac OS 11.1 using the web client on Nightly v87.0a1. This sound is totally unintended and annoying and it ONLY reproduces while the microphone on the android phone is unmuted and the sound is heard only on the Mac Web Client.
This sound is not audible on a Windows 10 connected through the web client on Nightly in the same call.

This being said, it could be an issue with the Zoom mobile native app or, more likely, the Mac OS Nightly with web client.

I have recorded the sound it makes because it is probably consistent throughout reproductions on different system combinations.
Mark, is it the sound you heard?

Comment 7

[Daniel Bodea [:danibodea]]

(In reply to Bodea Daniel [:danibodea] from comment #5)

P.S. mozregression does not work on Mac OS 11.

Furthermore, I have investigated for the first bad Nightly build by manually downloading and installing them and this is the result I got:
Name Firefox
Version 84.0a1
Build ID 20201027212052

More info:
Apparently, it does not reproduce when using Beta v86.0b1 or Release v84.0.2 or Release v85.0 instead of Nightly.

Comment 8

[Mark Richards [:freshness]]

That is the sound that I heard, (in the recording you shared it sounds like the echo cancellation is clamping down every ~4 seconds). When I reproduced the bug, the sound was consistent until I disconnected the webclient.

In my testing, the call was between the webclient and the Zoom desktop app. Only the webclient was hearing the consistent buzzing.

Comment 9

[Julien Cristau [:jcristau]]

Pushlog between 20201027212052 and the previous build:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3d6ed9f4cf34&tochange=ab66bd46c2d6874336d5fad4faceaa9b9448ff5f

Comment 10

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Zoom only uses WebRTC for transporting packets, it does use WebAssembly for decoding. The above push log has some WASM SIMD work, which seems like a it could be a good place to start looking.

Comment 11

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Lars, do you think that this may be related to your work in that range?

Comment 12

[Lars T Hansen [:lth]]

I know there's been some chatter about Zoom wanting to use SIMD but that's all I know about that. Clearly that pushlog shows a lot of SIMD optimization work landing, but as of yet I'm not aware of any regressions resulting from that. An obvious test to perform is to go into prefs to disable javascript.options.wasm_simd to see if the problem is still present. If it is not, I can try to dig a little deeper.

Comment 13

[Mark Richards [:freshness]]

I've disabled javascript.options.wasm_simd and the buzzing is no longer present.
Confirmed this behavior on both 86.0a1 and 87.0a1.
(buzz is still heard on 87.0a1 with javascript.options.wasm_simd set to true)

Comment 14

[Lars T Hansen [:lth]]

Thanks. I will make an attempt to bisect further with a local build and report back.

Comment 15

[Lars T Hansen [:lth]]

I can reproduce with wasm_simd enabled (cf comment 6):

• android client unmuted + running web client in 87 nightly on macOS 11.1 (Intel 2018 MBP)
• mac client unmuted + running web client in 87 nightly on Fedora 33 64-bit (my dual Xeon dev system)

I cannot reproduce with wasm_simd disabled or with the web client on Win64 10 (Xeon system).

The sound is more of a constant buzzing (or very rapid sawtooth, too fast for me to hear as other than continuous) than the sound described in comment 8, but it's clearly a related problem.

The win10 / unix disparity suggests an ABI problem since the unix ABI differs from the Win64 ABI in various ways. The problem could have been introduced by the patch set that landed ca Oct 27, or it could be preexisting and could be tickled by that patch set. The latter scenario is somewhat plausible since it did a lot of things to optimize register usage and didn't do anything with the ABI code.

(It could also be a C++ compiler bug since the C++ ABI is also different on this platform.)

Comment 16

[Lars T Hansen [:lth]]

It's only for ion, not for baseline. So if the bug is in the masm, it's in routines not used by baseline; more likely, the bug is in lowering/regalloc/codegen.

Reproduces on this morning's mozilla-central.
Reproduces with 83ffcdef59409078f58e9b90600c340f58aeea14 which was the last patch in the set of SIMD patches to land on Oct 27.
Does not reproduce with 4ae81cad3fea54d6c59b3a8d866572192d707d5d which was the last patch to land before that set.

Comment 17

[Lars T Hansen [:lth]]

Problem starts appearing with rev https://hg.mozilla.org/mozilla-central/rev/bb82bc52e536529530e13889173bc6b6dfecbd82, this introduces an optimization for load-and-widen. Not using this optimization removes the problem, also on the tip of mozilla-central (though m-c on Linux is very, very crashy on zoom). I think this can be consistent with symptoms. On unix, the ABI is different as comment 15 notes. This means that in some functions, parameters that arrive on the stack in win10 will arrive in registers on unix. This in turn means that the register allocator will use more higher-numbered registers on unix. These registers require an extra byte in the instruction encoding, and the patch that has problems introduces a new encoder. I remember that getting that code to work was a little tricky, and I expect the bug is that the encoder needs more work.

Comment 18

[Lars T Hansen [:lth]]

Attachment: Bug 1688713 - REX byte needs to consider all registers. r?nbp

This looks like a copy-paste problem: when I implemented the new
encoder for threeByteOp with Base+Index I copied code from an encoder
for Reg+offset, and thus my REX emitter considered only the base and
dest registers, not base, index, and dest. In the case where only the
index register would require the REX prefix to be present we'd get the
wrong code.

Comment 19

[Lars T Hansen [:lth]]

Gentle review ping.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ebbe2c2368e
REX byte needs to consider all registers. r=nbp

Comment 21

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/8ebbe2c2368e