Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream Blink OscillatorNode tests #25571

Open
youennf opened this issue Sep 16, 2020 · 7 comments
Open

Upstream Blink OscillatorNode tests #25571

youennf opened this issue Sep 16, 2020 · 7 comments
Labels

Comments

@youennf
Copy link
Contributor

@youennf youennf commented Sep 16, 2020

Blink has some useful OscillatorNode tests that WebKit recently imported directly in https://bugs.webkit.org/show_bug.cgi?id=216569.
It might be interesting to upstream them in WPT so that they can be kept in synced and shared with other platforms.

@gsnedders
Copy link
Contributor

@gsnedders gsnedders commented Sep 16, 2020

cc @rtoy

@rtoy
Copy link

@rtoy rtoy commented Sep 16, 2020

It's been a while since I looked, but I do think they work everywhere. My concern, IIRC, is that there are experimentally determined error thresholds that increased quite a bit on other browsers. (The current reference file was created long ago on a linux machine. If a different machine were used to create the reference, the thresholds would be different.)

And I didn't want to increase them so I could catch minor changes in the oscillator in Chrome. And I also didn't want to have two copies for these (one general browser-agnostic WPT one, and a duplicate one for Chrome with tighter thresholds).

I would not be opposed, though, if someone wanted to import these to WPT if they thought the tests were really useful. Chrome would keep its own copy, though.

@youennf
Copy link
Contributor Author

@youennf youennf commented Sep 18, 2020

So far, it seems the tests run in WebKit without having to change the thresholds.

@rtoy
Copy link

@rtoy rtoy commented Sep 18, 2020

@rtoy
Copy link

@rtoy rtoy commented Sep 18, 2020

I didn't check to which Blink oscillator tests were imported, but I just ran osc-sweep-snr-custom.html with Firefox 79.0. The expected SNR is at least 130.47, but Firefox got 30.96. I think this is why I didn't move these this test and the other sweep tests to WPT. (Long ago other tests that seemed to work well on Firefox were moved-just not these.)

I also see that the osc-440hz test fails on Firefox with an error threshold of 1.545e0, instead of something like 1.564e-3 and an SNR of 3.79 instead of 59. I need to look to see if this is a bug in the test or in Firefox.

@rtoy
Copy link

@rtoy rtoy commented Sep 22, 2020

FWIW, osc-440hz test fails on Firefox because it looks like a custom wave with a single component at 440 applied to an oscillator with frequency 1 is not the same as an oscillator with a component at 1 and a frequency of 440. Both of these should produce a sine wave with a frequency of 440 Hz.

The difference between the two is probably caused by an oscillator with a frequency of 1 Hz not being well interpolated. Chrome had this problem and a slightly better interpolation was added when the frequency was low.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.