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

Test SpeechSynthesisUtterance volume attribute #9963

Merged
merged 11 commits into from
May 8, 2019
Merged

Test SpeechSynthesisUtterance volume attribute #9963

merged 11 commits into from
May 8, 2019

Conversation

guest271314
Copy link
Contributor

@guest271314 guest271314 commented Mar 12, 2018

TODO: automate test to verify audio output of speechSynthesis.speak() using Web Audio API.

TODO: automate test to verify audio output of speechSynthesis.speak() using Web Audio API.
@ghost
Copy link

ghost commented Mar 12, 2018

Build ERRORED

Started: 2018-03-12 02:50:28
Finished: 2018-03-12 02:56:35

Failing Jobs

  • lint
  • chrome:dev
  • safari:11.0
  • MicrosoftEdge:16.16299

View more information about this build on:

@guest271314
Copy link
Contributor Author

@andrenatal @fleizach @gshires @jdsmith3000

What needs to be done to add this test to speech-api folder at this repository?

@guest271314
Copy link
Contributor Author

@foolip Can this test be merged into the speech-api tests directory?

@foolip
Copy link
Member

foolip commented May 31, 2018

Please have a look at https://web-platform-tests.org/writing-tests/manual.html for some tips for manual tests, in particular the test needs to make it clear to the person running it what is passing and not. For example, you might try to utter the "FAIL" with volume 0, then the word "PASS" with volume 1, and instructions say that the test passes if you only hear the PASS.

guest271314 added 4 commits March 20, 2019 02:58
Outputs expected result at Firefox. Crashes tab at Chromium/Chrome #15816
…diaelement-capturestream

Test HTMLMediaElement.captureStream()
…ce-htmlmediaelement-capturestream

Revert "Test HTMLMediaElement.captureStream()"
@foolip
Copy link
Member

foolip commented Mar 22, 2019

@guest271314 did you accidentally merge in more stuff to this PR? Did you have a look at https://web-platform-tests.org/writing-tests/manual.html? Sorry this has been sitting for so long, but it hasn't come up in my inbox because nothing happened until now.

@guest271314
Copy link
Contributor Author

@foolip It took a few days to get this PR here. Did not intentionally include the Web Speech API .volume test, again; as there is already an open PR for that test. Would probably still not be here without the help of @jdm #15816 (comment).

Will try to not to state the issues at GitHub to prevent being banned here as well people tend to get emotional when talking to them without kid gloves on re their "platform"; and have already been redirected to some page that states "abuse" of the site by simply searching, after the recent acquisition of the site.

Does the test need to be written according to the linked manual test page? And then, does this PR need to be closed and a new PR opened? Can you provide the complete steps necessary for this PR to be incorporated into wpt, to avoid eventually filing another issue for the purpose of navigating this site and git process to get to that point?

@guest271314
Copy link
Contributor Author

@foolip Does the manual test need to include buttons for both "PASS" and "FAIL"?

@foolip
Copy link
Member

foolip commented Apr 1, 2019

@guest271314 you don't need to have any buttons in the test itself, just instructions that are visible when opening the page that say what the user should do to judge whether it passes or not. Right now that's in JS comments, so putting the instructions in HTML would be good.

Other than that, you just need to get rid of the extra commits so that there's just one file remaining. If you don't want to deal with git rebase and would rather just edit the test in GitHub's UI, I can get rid of the extra file when merging instead.

@guest271314
Copy link
Contributor Author

@foolip Is this appropriate?

<!DOCTYPE html>
<html>
<head>
  <title>5.2.3 SpeechSynthesisUtterance volume attribute test - manual</title>
  <script>
var text = "hello universe";
var volumes = [0, 0.16666666666666666, 0.3333333333333333, 0.5, 0.6666666666666666, 0.8333333333333333, 0.9999999999999999];
 window.speechSynthesis.cancel();
 function handleVoicesChanged() {
  volumes.forEach(function(volume) {
    var utterance = new SpeechSynthesisUtterance();
    utterance.text = "hello universe";  
    utterance.volume = volume;
    window.speechSynthesis.speak(utterance);
  });
};
window.speechSynthesis.onvoiceschanged = handleVoicesChanged;
window.speechSynthesis.getVoices();
</script>
</head>
<body>
<p>
Specification

https://w3c.github.io/speech-api/speechapi.html#utterance-attributes

volume attribute

This attribute specifies the speaking volume for the utterance. 
It ranges between 0 and 1 inclusive, with 0 being the lowest volume and 1 the highest volume, with a default of 1. 
If SSML is used, this value will be overridden by prosody tags in the markup.
</p>
<p>
Test

Does the volume of audio output change?
</p>
</body>
</html>

If so, what needs to be done? Place that HTML in own fork and make another PR? Or can the code in this PR be changed?

@foolip
Copy link
Member

foolip commented May 6, 2019

@guest271314 yes you can update the code in this PR by updating the master branch of your fork, since that's the branch used to create this PR. Or, if you find it simpler, you can just send another PR.

In the code you pasted, using the voiceschanged event looks unnecessary, can't the test just start calling speechSynthesis.speak right away?

@guest271314
Copy link
Contributor Author

@foolip Has been several OS's ago since dove into Web Speech API. speak() communicates with the local Speech Dispatcher server https://github.com/brailcom/speechd. At Chromium if no voices are loaded an empty array could be returned for getVoices(), see https://stackoverflow.com/q/46863170. Chrome is shipped with voices created by *oogle. Though the code base is Chromium source, Chromium is not shipped with the custom *oogle voices. Yes, would suggest to use onvoiceschanged event. It is not straightforward how the procedure works in the specification.

Have to get the code working again, locally, which may require setting up Speech Dispatcher server again. Been a while since mapped a flow chart of how to use Web Speech API.

Have not, in general, had success filing PR's. Something invariably is included or not included which should or should not have been included. Will try to make sure the code still works at Chromium and update this PR over the next couple days.

@guest271314
Copy link
Contributor Author

@foolip Another consideration that needs to be taken into account is that autoplay policies of browsers have changed since this PR was initially filed, e.g., guest271314/SpeechSynthesisRecorder#13

@guest271314
Copy link
Contributor Author

@foolip Updated the code at f580334. Tested at Firefox 66 and Chromium 73. Will try at Nightly 68 within a day or so. The volume does appear to change at Firefox, not at Chromium plnkr https://plnkr.co/edit/iV2DdU?p=preview.

@guest271314
Copy link
Contributor Author

@foolip Note, for the code to have audio output at Chromium at *nix espeak, espeak-ng or other TTS program needs to be installed and configured to be used with speech-dispatcher, and Chromium needs to be launched with --enable-speech-dispatcher flag.

guest271314 added 2 commits May 8, 2019 14:11
Delete testRecordingMediaFragmentsWithMediaSourceAndMediaRecorder.html from PR including Web Speech API test
@guest271314
Copy link
Contributor Author

@foolip Made the suggested changes to the code.

@foolip foolip merged commit d6c1c77 into web-platform-tests:master May 8, 2019
Copy link
Contributor Author

@guest271314 guest271314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@foolip There is a syntax error at closing </html> tag

@foolip
Copy link
Member

foolip commented May 8, 2019

@guest271314 I fixed that (and a blank line, and the filename) before merging.

@guest271314
Copy link
Contributor Author

@foolip const text = "hello universe" is not used.

@foolip
Copy link
Member

foolip commented May 9, 2019

@guest271314 can you send another PR to fix?

@guest271314
Copy link
Contributor Author

@foolip Still not versed enough yet with how to make a PR using the GitHub GUI. Tried for at least an hour yesterday to create a PR for the deleted (from this PR) MediaSource/HTMLMediaElement.captureStream() test. Every time the displayed output was, paraphrasing, "nothing to compare". Will continue to try.

@guest271314
Copy link
Contributor Author

@foolip Following the suggestion of @jdm at #15816 (comment) was helpful though led to multiple PR's being incorporated into this PR (which had to delete files from). master...guest271314:master is the link from that comment. How to "clean up" the multiple PR sources to both update this PR (the code) and separately file the PR which tests capturing <video> using .captureStream() where src attribute is set to MediaSource instance?

@foolip
Copy link
Member

foolip commented May 9, 2019

@guest271314, if you want to edit a single file and create a new PR, navigate to it from https://github.com/web-platform-tests/wpt?files=1 and then edit, you'll be at a URL like this:
https://github.com/web-platform-tests/wpt/edit/master/speech-api/SpeechSynthesisUtterance-volume-manual.html

Then just write a commit message and create the PR.

@guest271314
Copy link
Contributor Author

@foolip Filed updated PR. How to file the PR for the MediaSource/HTMLMediaElement.captureStream() test cc69d84 ?

@guest271314
Copy link
Contributor Author

@foolip These are the two commits that am trying to file a PR for each 4ec6753 and cc69d84. The former is for the mediacapture-fromelement directory the latter is for media-source directory.

@guest271314
Copy link
Contributor Author

@foolip This is the filed PR #16756. Probably needs some cleaning up, though not sure how to do so

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

Successfully merging this pull request may close these issues.

3 participants