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

Order available voicingManager voices by priority #1282

Closed
chrisklus opened this issue Sep 16, 2021 · 10 comments
Closed

Order available voicingManager voices by priority #1282

chrisklus opened this issue Sep 16, 2021 · 10 comments

Comments

@chrisklus
Copy link
Contributor

I'm using the voicingManager for the SpeechSynthesisButton in Number Play, and recently noticed that the current voice being used on my machine (MacOS + Chrome) was different (and less desirable) than a few weeks ago.

I looked at the available voices in voicingManager.voices and found that all the same ones that I remember were there, they were just in a different order (Google's voices used to be at the top of the list, now they appear to be at the bottom). And voicingManager, as well as the locale-specific voice choosing code I'm using in Number Play, just selects the first one in the list. So it was unclear to me why the order changed (browser update, maybe?), but it seems like we should be giving preference to the voices we like best so that when they do exist on a particular browser/machine, they are intentionally chosen instead of relying on the order of the list.

@jessegreenberg and I made some comparisons, both on MacOS + Chrome, and found that while we have a lot of the same voices, they were not in the same order. This seems like another case where, if we both have the same list, running a sim should use the same voice so our experience is consistent (unless we need to take into consideration people choosing a specific voice for their machine @jessegreenberg? do you know if that affects what voice the browser chooses?).

After figuring this out, @jessegreenberg recommended a couple things we could do to prioritize what voices are used - first, we could order the list of all voices by putting the types we like at the top, like Google's voices. This would be general support for all locales, so that once we filter down to, say Spanish, if a Google voice exists for that locale, it will be chosen. However, this only gets us so far because desirable types of voices may not exist in all browsers (non-Chrome browsers, for the example above). So, we could also create an explicit list of ordered voices for English, so we can get specific with the voices by their name, which might look more like getBestSoundingEnglishVoice. Something like this would ensure that two Mac users would hear the same thing when in english.

@jessegreenberg offered to try this out, thank you!

@jessegreenberg
Copy link
Contributor

unless we need to take into consideration people choosing a specific voice for their machine @jessegreenberg? do you know if that affects what voice the browser chooses?).

@chrisklus helped me look into this. There is a default property on Voices, and according to the spec it behaves like

"This attribute is true for at most one voice per language. There may be a different default for each language. It is user agent dependent how default voices are determined."

We tested out changing the default OS voice and seeing how it impacted the default voice for the browser. On my Windows 10 machine changing the OS voice had no impact on the default voice in Chrome, even when I fully restarted Chrome. But on @chrisklus Mac changing the default system voice did change the default Chrome voice. So we are hesitant to force a different default voice when the original default voice may have been purposely selected by the user.

But we did decide to add a function like getPrioritizedVoiceList that would return the list of voices but with the best sounding ones at the top (google, microsoft, at the top. then others). This could be used in the number-play case if that is best. I am not sure whether to use this for Voicing yet.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Sep 16, 2021

A basic function was done in the above commit to prioritize "Google" voices.

I asked the Voicing team on slack whether something like this should be used for Voicing and listed the tradeoffs as

a) For those that are less familiar with AT, presenting a more friendly sounding voice by default could make the feature more appealing. I remember seeing lots of feedback in the surveys commenting on the robotic sounding voice, and using something other than Fred might resolve some of that.
b) The default voice for Mac Chrome is determined by operating system settings. If the user specifically chooses a default voice for other speech synthesis and we don't use the system default we could potentially override their setting.

@chrisklus will this function work for number-play for now?

@chrisklus
Copy link
Contributor Author

chrisklus commented Sep 27, 2021

Thanks @jessegreenberg!

I noticed that the existing sort was alphabetizing the remaining voices after prioritizing Google voices, so @jessegreenberg and I chatted on Slack about a fix. I pushed a commit above that appears to be fulfilling the behavior we would like. If a browser doesn't have Google voices but does have the system default first in the list, it will preserve that order.

I noticed that Safari just provides the list of ordered voices, without putting the current system voice first, like Chrome does. Do we know if Safari defaults to the current system voice a different way (not by ordering the list of voices?). I'd be curious to how we can best support prioritizing voices for all browsers.

Back to @jessegreenberg to double check my commit is working as expected.

@jessegreenberg
Copy link
Contributor

Thanks looks good, thanks @chrisklus. I tested in Chrome to verify that Google voices come first, then tested in Firefox and Edge to verify that order of voices stayed the same when there are no Google voices.

Do we know if Safari defaults to the current system voice a different way (not by ordering the list of voices?)

I don't see anything in the W3C spec about the order of the voices list (https://wicg.github.io/speech-api/#dom-speechsynthesis-getvoices). I don't think we can assume that the first in the list is the voice with the default attribute. And in #1282 (comment) we found that the voice with the default attribute may or may not adhere to operating system preferences set by the user.

Over slack I asked Voicing team if we should prioritize voices even though that could go against user settings and @terracoda said

I agree that presenting the best voices we know of would be a good idea. [...] if someone really loves Fred, they can always choose him, and eventually someday we may have a way of saving ones preferences, right?

For now, I am going to change the sorting to be "google first, Fred last" because "Fred" is by far the most robotic sounding voice.

jessegreenberg added a commit that referenced this issue Sep 27, 2021
…oval of duplicate voices to populateVoices, see #1282
jessegreenberg added a commit to phetsims/joist that referenced this issue Sep 27, 2021
jessegreenberg added a commit that referenced this issue Sep 27, 2021
@jessegreenberg
Copy link
Contributor

In my initial commit I noticed that 'Fred' was not at the bottom on macOS Chrome. To understand why I found examples at https://stackoverflow.com/questions/29829205/sort-an-array-so-that-null-values-always-come-last which demonstrate when you want to make sure something as at the front/end of a list using sort you have to specify the behavior for both values in the comparator function. So after e2619a5 the sorting is as I would expect, with "Google" voices first and "Fred" last.

I am also using this function in the joist/PreferencesDialog now so that voice options use this priority.

@chrisklus can you please review this change?

chrisklus added a commit that referenced this issue Sep 28, 2021
@chrisklus
Copy link
Contributor Author

Nice @jessegreenberg! Everything looks great and is working as expected. I tested on Mac Chrome/Safari/Firefox. Your additional commits also clarified to me how sort works, thanks!

For now, I am going to change the sorting to be "google first, Fred last" because "Fred" is by far the most robotic sounding voice.

Agreed that this is a nice change.

Passing back to you to close if we're all set.

@samreid
Copy link
Member

samreid commented Oct 6, 2021

I opened this side issue for the CT problem that is occurring: #1293

@samreid
Copy link
Member

samreid commented Oct 6, 2021

I found this issue from the CT error described in #1293. But when I saw the sorting algorithm, it seemed asymmetrical:

    return this.voices.slice().sort( ( a, b ) => {
      return a.name.includes( 'Fred' ) ? 1 : // a includes 'Fred', put b before a so 'Fred' is at the bottom
             b.name.includes( 'Fred' ) ? -1 : // b includes 'Fred', put a before b so 'Fred' is at the bottom
             a.name.includes( 'Google' ) ? -1 : // a includes 'Google', put a before b so 'Google' is at the top
             b.name.includes( 'Google' ) ? 1 : // b includes 'Google, 'put b before a so 'Google' is at the top
             0; // otherwise all voices are considered equal
    } );

This is saying any name with "Fred" goes after any other name. This means Fred1 goes after Fred2 and also Fred2 goes after Fred1, which is contradictory. This means sorting could be unstable or different on different browsers. I tested this code:

const names = [ 'Fred', 'Larry', 'George', 'Harold', 'Fred', 'Google', 'Tim', 'Fred', 'Bob (Google)', 'Alice (Google)' ];

const sorter = ( a, b ) => {
  return a.includes( 'Fred' ) ? 1 : // a includes 'Fred', put b before a so 'Fred' is at the bottom
         b.includes( 'Fred' ) ? -1 : // b includes 'Fred', put a before b so 'Fred' is at the bottom
         a.includes( 'Google' ) ? -1 : // a includes 'Google', put a before b so 'Google' is at the top
         b.includes( 'Google' ) ? 1 : // b includes 'Google, 'put b before a so 'Google' is at the top
         0; // otherwise all voices are considered equal
};

const x = names.sort( sorter );
console.log( x );

and saw that on macOS/Chrome it listed this order:

['Alice (Google)', 'Bob (Google)', 'Google', 'Larry', 'George', 'Harold', 'Tim', 'Fred', 'Fred', 'Fred']

Whereas on macOS/Firefox, it listed a completely different order:

[ "Google", "Bob (Google)", "Alice (Google)", "Larry", "George", "Harold", "Tim", "Fred", "Fred", "Fred" ]

Note the examples in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort that it is idiomatic for the sorting function to look at both arguments, not short circuit and return after only looking at one argument. Would it be difficult to change the sorting rule to say "if (both a and b contain 'Fred' or both contain 'Google') or (neither a nor b contain fred or google), then use the indices from indexOf in the original list", etc?

Also, do we need to guard against substrings, like what if google adds a name like Google Fredrickson which also contains "Fred"?

@samreid
Copy link
Member

samreid commented Oct 6, 2021

Something like this seems like a step in the right direction:

  const getIndex = name =>
    name.includes( 'Google' ) ? -1 : // Google should move toward the front
    name.includes( 'Fred' ) ? names.length : // Fred should move toward the back
    names.indexOf( name ); // Otherwise preserve ordering

  const sorter = ( a, b ) => getIndex( a ) - getIndex( b );

It's behaving correctly in my testing, and has reproducible results across Chrome and Firefox. I'm not sure why it doesn't need a clause like "if they both have Google or both have Fred, take a difference of their indices". Maybe that is covered by sort stability in ES2019+ ? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#sort_stability But we could add that clause for clarity if we want.

Here's a more verbose "take stability into our own hands" implementation (not sure if it is necessary):

  const getIndex = name =>
    name.includes( 'Google' ) ? -1 : // Google should move toward the front
    name.includes( 'Fred' ) ? names.length : // Fred should move toward the back
    names.indexOf( name ); // Otherwise preserve ordering

  const sorter = ( a, b ) => {
    if ( ( a.includes( 'Google' ) && b.includes( 'Google' ) ) || ( a.includes( 'Fred' ) && b.includes( 'Fred' ) ) ) {
      return names.indexOf( a ) - names.indexOf( b );
    }
    else {
      return getIndex( a ) - getIndex( b );
    }
  };

@chrisklus
Copy link
Contributor Author

@jessegreenberg and I returned to this issue today while investigating phetsims/number-play#134 and we tested the first method from #1282 (comment) and found to work well on all browsers. Thank you @samreid! We are ready to close.

jessegreenberg added a commit to phetsims/utterance-queue that referenced this issue Feb 7, 2022
jessegreenberg added a commit to phetsims/utterance-queue that referenced this issue Feb 7, 2022
jessegreenberg added a commit to phetsims/utterance-queue that referenced this issue Feb 7, 2022
jessegreenberg added a commit to phetsims/utterance-queue that referenced this issue Feb 7, 2022
jessegreenberg pushed a commit to phetsims/utterance-queue that referenced this issue Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants