Try next synth if oneCore consistently fails by seanbudd · Pull Request #12647 · nvaccess/nvda

seanbudd · 2021-07-15T02:03:38Z

Link to issue number:

Summary of the issue:

#10451
OneCore voices silently fail when attempting to speak an alphabet they don't support (eg an English voice reading Cyrillic characters). This makes installing NVDA for the first time for the first time inaccessible if someone is using a system with a language that isn't supported by their OneCore installed voices.

#11544
An issue was raised on #11543 where oneCore failed to initialise, causing all speech to fail.
If no speech succeeds, the _player is never set and so a subsequent error is thrown in _processQueue.

Description of how this pull request fixes the issue:

#10451
When picking a synthesizer automatically (using "auto", ie not overriden by a user), fail if the current NVDA language and current system language is not supported by installed OneCore voices.

#11544
If oneCore fails to speak more than N=5 times in a row, try the next synth on the synth priority list.

Testing strategy:

#10451
Manual confirmation that this fixes the issue by installing NVDA:

on a fresh config, and
with a system language set that OneCore voices doesn't support, or is not installed.
And having espeak set and read the install dialogs.

Manual confirmation that setting/changing synthesizers works and is respected across restarts.

#11544
Hard to test as it requires a failure external to NVDA, which is unknown how to reproduce. Ensure oneCore works as a synthesizer normally otherwise.

Known issues with pull request:

None

Change log entries:

Bug fixes

- NVDA will default to eSpeak if no installed OneCore voices support the NVDA preferred language. (#10451)
- If OneCore voices consistently fail to speak, revert to eSpeak as a synthesizer. (#11544)

Code Review Checklist:

Pull Request description is up to date.
Unit tests.
System (end to end) tests.
Manual testing.
User Documentation.
Change log entry.
Context sensitive help for GUI changes.
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers

LeonarddeR · 2021-07-15T07:01:36Z

How is this related to #12646?

LeonarddeR · 2021-07-15T07:17:22Z

+			self._consecutiveSpeechFailures += 1
+			if self._consecutiveSpeechFailures >= self.MAX_CONSECUTIVE_SPEECH_FAILURES:
+				log.debugWarning("Too many consecutive speech failures, changing synth")
+				setNextSynth(self.name)


I'm a bit worried about this implementation, i.e. a synth driver calling a synthDriverHandler function. Could you consider something like the braille handler does, i.e. in _writeCells, it tries to write content to a display. If that fails, it calls handleDisplayUnavailable which performs the fallback.

I think it might be best to use a new exception type for this, e.g. ConsecutiveSPeechFailureException. The OneCore synth can throw that exception in its speak method when it notices the self._consecutiveSpeechFailures limit has been exceeded. Then, the speech manager can catch the exception and perform the fallback.

Handling failures here is quite tricky. I've opted to queue a synth change instead here

ERROR - unhandled exception (11:22:58.596) - Dummy-3 (20092): Traceback (most recent call last): File "_ctypes/callbacks.c", line 232, in 'calling callback function' File "synthDrivers\oneCore.py", line 373, in _callback raise AssertionError() AssertionError

AppVeyorBot · 2021-07-19T08:18:05Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 7214a0ed52

seanbudd · 2021-07-20T01:56:25Z

 	Fetches the locale name of the user's configured language in Windows.
 	"""
-	windowsLCID=ctypes.windll.kernel32.GetUserDefaultUILanguage()
+	windowsLCID = ctypes.windll.kernel32.GetUserDefaultLCID()


ctypes.windll.kernel32.GetUserDefaultUILanguage() returned 'en_GB' when all of my language preferences (including voices) were set to 'en_AU'.

Please add a comment here to explain the choice of GetUserDefaultLCID over GetUserDefaultUILanguage.

Have you checked the history of this to see if it was previously GetUserDefaultLCID?

This was changed in a2f0b75, to fix #353. I believe #353 should still be fixed by this change still, or at least improved, as when we try to set the language for the translation service, we now attempt to just use the language code if the country code is not supported.

@LeonarddeR has also made a suggestion to change this to GetUserPreferredUILanguages in #7999, which is another move to consider and probably fix #353 in a more meaningful way. That way we could try multiple preferred languages (with and without country code) and give better internationalisation support.

I think I could revert this change and attempt a fix for that after 2021.2 beta is out

feerrenrut

Please add docs to these tests to explain the situations they are testing.

codeofdusk · 2021-07-29T08:24:28Z

+			self.assertEqual(synthName, synthDriverHandler.getSynth().name)
+			self.assertEqual(self._oldSynthConfig, config.conf["speech"]["synth"])
+
+	def test_setSynth_oneCoreSupportsDefaultLangauge(self):


Suggested change

def test_setSynth_oneCoreSupportsDefaultLangauge(self):

def test_setSynth_oneCoreSupportsDefaultLanguage(self):

…oneCore is not supported (#12702) Some unit tests introduced as part of #12647 were failing under versions of Windows where oneCore was unsupported. Tests which require oneCore being supported are now being skipped on versions of Windows on which oneCore is not supported.

seanbudd requested a review from a team as a code owner July 15, 2021 02:03

seanbudd requested a review from feerrenrut July 15, 2021 02:03

seanbudd changed the title ~~move to next synth if oneCore consistently fails~~ Try next synth if oneCore consistently fails Jul 15, 2021

LeonarddeR reviewed Jul 15, 2021

View reviewed changes

seanbudd mentioned this pull request Jul 16, 2021

Fallback to eSpeak if OneCore voices are not installed #12646

Closed

11 tasks

seanbudd marked this pull request as draft July 16, 2021 02:52

seanbudd added 3 commits July 19, 2021 18:47

improve typing and logging

33de152

prevent oneCore setting a language that doesn't match system

d2a3929

handle onecore failure

ac1c4ad

lukaszgo1 reviewed Jul 19, 2021

View reviewed changes

Comment thread source/synthDrivers/oneCore.py

seanbudd added 2 commits July 20, 2021 11:00

add another fallback language to try

a498ee0

improve default locale handling

03f2e23

seanbudd force-pushed the fix-11544 branch from 0cf4891 to 03f2e23 Compare July 20, 2021 01:54

seanbudd added 2 commits July 21, 2021 10:54

only fail to set onecore if synth == auto

b91479b

always normalise windows locale

9a405f6

seanbudd commented Jul 21, 2021

View reviewed changes

seanbudd marked this pull request as ready for review July 21, 2021 01:24

seanbudd added 3 commits July 21, 2021 11:59

fix lang normalization

00ec6b6

reverse default

fb23821

only log warning on onecore

2a4d21c

seanbudd added this to the 2021.3 milestone Jul 21, 2021

seanbudd requested a review from michaelDCurran July 23, 2021 01:01

feerrenrut reviewed Jul 23, 2021

View reviewed changes

Comment thread source/speech/manager.py

feerrenrut reviewed Jul 23, 2021

View reviewed changes

Comment thread source/synthDriverHandler.py

seanbudd added 3 commits July 29, 2021 16:51

add unit tests for synthDriver handler

31d4314

revert to GetUserDefaultUILanguage

b28ecfb

add test for oneCore support

b720aae

seanbudd requested a review from feerrenrut July 29, 2021 07:47

feerrenrut suggested changes Jul 29, 2021

View reviewed changes

Comment thread tests/unit/test_synthDriverHandler.py Outdated

codeofdusk reviewed Jul 29, 2021

View reviewed changes

improve testing

d94dbb9

seanbudd requested a review from feerrenrut July 29, 2021 08:43

feerrenrut approved these changes Jul 29, 2021

View reviewed changes

seanbudd added 2 commits July 29, 2021 21:14

Merge branch 'master' into fix-11544

9dd4674

update changes

873f246

seanbudd merged commit 8c8e2bb into master Jul 29, 2021

seanbudd deleted the fix-11544 branch July 29, 2021 11:50

lukaszgo1 mentioned this pull request Jul 30, 2021

Make unit tests for oneCore pass under the versions of Windows where oneCore is not supported #12702

Merged

8 tasks

seanbudd mentioned this pull request Aug 15, 2021

Problem with ESpeak on NVDA2021.1 #12732

Closed

lukaszgo1 mentioned this pull request Aug 23, 2021

Use GetUserPreferredUILanguages for determining Windows Languages to use #12707

Open

seanbudd mentioned this pull request Feb 17, 2023

Error with Windows Core Voices on Windows 11 #14576

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Try next synth if oneCore consistently fails#12647

Try next synth if oneCore consistently fails#12647
seanbudd merged 16 commits into
masterfrom
fix-11544

seanbudd commented Jul 15, 2021 •

edited

Loading

Uh oh!

LeonarddeR commented Jul 15, 2021

Uh oh!

LeonarddeR Jul 15, 2021

Uh oh!

seanbudd Jul 21, 2021 •

edited

Loading

Uh oh!

AppVeyorBot commented Jul 19, 2021

Uh oh!

Uh oh!

seanbudd Jul 20, 2021

Uh oh!

feerrenrut Jul 23, 2021

Uh oh!

seanbudd Jul 28, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feerrenrut left a comment

Uh oh!

Uh oh!

codeofdusk Jul 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	def test_setSynth_oneCoreSupportsDefaultLangauge(self):
	def test_setSynth_oneCoreSupportsDefaultLanguage(self):

Uh oh!

Conversation

seanbudd commented Jul 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing strategy:

Known issues with pull request:

Change log entries:

Code Review Checklist:

Uh oh!

LeonarddeR commented Jul 15, 2021

Uh oh!

LeonarddeR Jul 15, 2021

Choose a reason for hiding this comment

Uh oh!

seanbudd Jul 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AppVeyorBot commented Jul 19, 2021

Uh oh!

Uh oh!

seanbudd Jul 20, 2021

Choose a reason for hiding this comment

Uh oh!

feerrenrut Jul 23, 2021

Choose a reason for hiding this comment

Uh oh!

seanbudd Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feerrenrut left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codeofdusk Jul 29, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

seanbudd commented Jul 15, 2021 •

edited

Loading

seanbudd Jul 21, 2021 •

edited

Loading

seanbudd Jul 28, 2021 •

edited

Loading