Support for Windows OneCore voices included in Windows 10. #7110

jcsteh · 2017-04-28T02:38:44Z

This uses a C++/CX dll to access the UWP SpeechSynthesizer class. There are other UWP APIs we might like to access in future (e.g. OCR), so rather than making this dll specific to OneCore speech, it's called nvdaHelperLocalWin10. The build system for this dll makes it easy to add other components in future.
In addition, this required code to generate balanced XML from an NVDA speech sequence. Although we use SSML for eSpeak, eSpeak happily accepts unbalanced (malformed) XML. OneCore speech does not. This code is in the speechXml module. This might eventually be reused to replace the ugly balanced XML code in the SAPI5 driver.
Fixes #6159.

This uses a C++/CX dll to access the UWP SpeechSynthesizer class. There are other UWP APIs we might like to access in future (e.g. OCR), so rather than making this dll specific to OneCore speech, it's called nvdaHelperLocalWin10. The build system for this dll makes it easy to add other components in future. In addition, this required code to generate balanced XML from an NVDA speech sequence. Although we use SSML for eSpeak, eSpeak happily accepts unbalanced (malformed) XML. OneCore speech does not. This code is in the speechXml module. This might eventually be reused to replace the ugly balanced XML code in the SAPI5 driver.

…eCommand.

michaelDCurran

Attribute values should be also escaped, which will require " in xml_escapes

michaelDCurran · 2017-05-02T03:59:45Z

source/synthDrivers/oneCore.py

+	"""
+	# comtypes.BSTR.from_address seems to cause a crash for some reason. Not sure why.
+	# Just access the string ourselves.
+	val = ctypes.wstring_at(address)


Would there ever be the case where a NULL char might be in that string? probably not, but you could use SysStringLen to get the length and pass it to wstring_at.

michaelDCurran · 2017-05-02T04:05:56Z

source/synthDrivers/oneCore.py

+		super(SynthDriver, self).terminate()
+		# Drop the ctypes function instance for the callback,
+		# as it is holding a reference to an instance method, which causes a reference cycle.
+		self._dll.ocSpeech_terminate(self._handle)


Perhaps move this line above the comment as the comment is only about the callback.

michaelDCurran · 2017-05-02T04:23:05Z

nvdaHelper/localWin10/oneCoreSpeech.cpp

+	ComPtr<IBufferByteAccess> bufferByteAccess;
+	insp.As(&bufferByteAccess);
+	byte* bytes = nullptr;
+	bufferByteAccess->Buffer(&bytes);


Can inst.as fail? I.e. that interface is not supported? and perhaps then the pointer would be NULL?

…7110 (issue #6159).

jcsteh · 2017-05-15T02:50:02Z

@feerrenrut, would you mind reviewing 3b43394 and c02a788? These fix a reported issue where invalid characters break the OneCore Speech driver. @michaelDCurran originally reviewed this, but he's away for a week or so now and it'd be good to get this in ASAP.

The first commit ensures that errors get logged and don't break the driver for all subsequent speech. Previously, there was no logging and speech just went silent. The second fixes the actual problem with invalid characters.

feerrenrut

As discussed, the changes look good.

…etc. Incubates #7110 (issue #6159).

…eeded for OneCore Speech.

jcsteh · 2017-05-17T04:50:18Z

@feerrenrut, would you mind reviewing ba62552? It updates the build system to bundle some required redistributable VC2015 components that aren't installed on all systems.

Two users confirmed that this fixes the problem for them; see this nvda-devel thread.

…led. Incubates #7110 (issue #6159).

…indows, as otherwise, OneCore throws an error.

…indows 10 (unless running from source).

jcsteh · 2017-05-24T12:09:35Z

@feerrenrut, would you mind reviewing three more commits? Sorry. :(

f1a34ce deals with Provide support for new Windows 10 One Core voices #6159 (comment): a document which specifies an invalid language code (or one unknown to Windows) causes an exception in the driver. We detect such language codes and ignore them. We don't do this more generally (in core speech code) because we can't assume that some other synth won't know about a language that Windows doesn't; e.g. Windows doesn't know about the Aragonese language, but NVDA and eSpeak do.
1f9f90a adds a User Guide section which I neglected to add earlier.
032e65e excludes this driver from available synths for binary copies if not Windows 10. The driver only works on Windows 10. We don't exclude for source copies because source copies can't detect Windows 10 due to Python's manifest. That's okay, though, because developers can be expected to understand such things and only developers would run from source. Note that the synth will still fail gracefully if the user attempts to use it in this case.

feerrenrut · 2017-05-25T09:26:25Z

source/synthDrivers/oneCore.py

@@ -81,6 +84,13 @@ def convertCharacterModeCommand(self, command):
 		# Therefore, we don't use it.
 		return None

+	def convertLangChangeCommand(self, command):
+		lcid = languageHandler.localeNameToWindowsLCID(command.lang)
+		if lcid in (0, languageHandler.LOCALE_CUSTOM_UNSPECIFIED):


How do you get a value of zero and what does it represent? Is there a test for this case?

Honestly, I'm not sure. I did this because the other place we use that function checks for 0. It seems we can return 0 on Windows XP (where we implement all of that function ourselves), but that doesn't apply here, since this is always Windows 10. On Vista and later, we use the Windows LocaleNameToLCID function. That function says it can return 0 for invalid parameter values, but I don't know how to trigger this.

What do you think? On one hand, it might be better to remove the 0 so users notice potential unknown problems and report them. On the other hand, if we do get 0, there's probably nothing we can do and we'll end up just ignoring the language anyway.

In that case I think its best to keep checking for it and logging. It might be worth adding the return value to the log message. I guess we could name the zero something like INVALID_PARAM_VALUES or include some of what you said here as a comment to save the next person from having to look up what zero means.

jcsteh · 2017-05-26T12:14:37Z

@feerrenrut, as discussed, I've simplified localeNameToWindowsLCID so it returns 0 in all failure cases (with a constant of LCID_NONE). I'd prefer to just use Python's None, but I'm erring on the side of backwards compat caution. I also got rid of the oneCore synth driver tests and instead added tests for localeNameToWindowsLCID, since the method I was testing in oneCore is really just a thin wrapper around the latter.

feerrenrut

I think these comments really help those who aren't familiar with this area.

feerrenrut · 2017-05-29T03:15:42Z

tests/unit/synthDrivers/test_oneCore.py

@@ -1,34 +0,0 @@
-#tests/unit/synthDrivers/test_oneCore.py


I guess this was removed by accident?

No; it was intentional. I got rid of the oneCore synth driver tests and instead added tests for localeNameToWindowsLCID , since the method I was testing in oneCore is really just a thin wrapper around the latter.

feerrenrut · 2017-05-29T03:16:06Z

source/languageHandler.py

-	# #6259: LOCALE_CUSTOM_UNSPECIFIED denotes custom locale in Windows 10, thus returns "unknown language" or an odd description (observed for Aragonese).
-	# See https://msdn.microsoft.com/en-us/library/system.globalization.cultureinfo.lcid(v=vs.110).aspx.
-	if LCID not in (0, LOCALE_CUSTOM_UNSPECIFIED):
+	if LCID is not LCID_NONE:


I think this is much clearer

feerrenrut · 2017-05-30T03:54:56Z

As we discussed yesterday, don't forget to update the readme. I had to ensure that I installed Tools 1.4.1 and Windows 10 SDK 10.0.14393 with visual studio community edition to get this to build.

…on the Windows 10 Tools and SDK.

jcsteh · 2017-05-30T05:34:26Z

Readme updated.

jcsteh requested a review from michaelDCurran April 28, 2017 02:38

Fix broken CharacterModeCommand(False) for SSML. Test this and Phonem…

ddec1b4

…eCommand.

michaelDCurran requested changes May 2, 2017

View reviewed changes

jcsteh added 2 commits May 3, 2017 16:08

speechXml: Escape attribute values.

5973c2e

Review actions.

3632020

jcsteh requested a review from michaelDCurran May 3, 2017 11:58

michaelDCurran approved these changes May 3, 2017

View reviewed changes

jcsteh added a commit that referenced this pull request May 3, 2017

Support for Windows OneCore voices included in Windows 10. Incubates #…

350bdb7

…7110 (issue #6159).

nvaccessAuto added the incubating label May 3, 2017

nvaccessAuto assigned jcsteh May 3, 2017

mohdshara mentioned this pull request May 8, 2017

Automatic language detection based on unicode ranges #2990

Open

LeonarddeR mentioned this pull request May 11, 2017

Add support for MS SAPI Mobile voices, and remove the lag at end of sapi5 speech #6166

Closed

jcsteh added 2 commits May 15, 2017 10:12

Don't break subsequent output if a speech call fails asynchronously.

3b43394

speechXml: Don't ever output invalid XML characters.

c02a788

jcsteh requested a review from feerrenrut May 15, 2017 02:50

Review actions.

1cf30e9

feerrenrut approved these changes May 15, 2017

View reviewed changes

jcsteh added a commit that referenced this pull request May 15, 2017

Fix OneCore Voices driver breakage when handling control characters, …

9b3f5a9

…etc. Incubates #7110 (issue #6159).

Bundle the VC 2015 runtime, as some systems don't have it and it is n…

ba62552

…eeded for OneCore Speech.

jcsteh requested a review from feerrenrut May 17, 2017 04:53

feerrenrut approved these changes May 17, 2017

View reviewed changes

jcsteh added a commit that referenced this pull request May 18, 2017

OneCore Speech: Fix breakage on systems without VC2015 runtime instal…

e102a7e

…led. Incubates #7110 (issue #6159).

jcsteh added 3 commits May 24, 2017 16:16

OneCore synth driver: Ignore invalid languages/languages unknown to W…

f1a34ce

…indows, as otherwise, OneCore throws an error.

Add User Guide section for Windows OneCore Voices.

1f9f90a

OneCore synth driver: Exclude from available synths if earlier than W…

032e65e

…indows 10 (unless running from source).

jcsteh requested a review from feerrenrut May 24, 2017 12:09

feerrenrut reviewed May 25, 2017

View reviewed changes

Review actions.

1cb911f

jcsteh requested a review from feerrenrut May 26, 2017 12:09

feerrenrut reviewed May 29, 2017

View reviewed changes

readme: Update Visual Studio dependencies to note the new dependency …

d07bc1d

…on the Windows 10 Tools and SDK.

jcsteh requested a review from feerrenrut May 30, 2017 05:34

feerrenrut approved these changes May 30, 2017

View reviewed changes

jcsteh added a commit that referenced this pull request May 30, 2017

Various fixes to OneCore Voices driver. Incubates #7110 (issue #6159).

9f90389

jcsteh merged commit 2ee954f into master Jun 13, 2017

nvaccessAuto removed the incubating label Jun 13, 2017

nvaccessAuto added this to the 2017.3 milestone Jun 13, 2017

jcsteh deleted the oneCoreSpeech branch June 13, 2017 04:30

jcsteh mentioned this pull request Jul 4, 2017

Fix build failure for nvdaHelperLocalWin10 on 32 bit Windows. #7351

Merged

feerrenrut mentioned this pull request Jun 8, 2022

Error with OneCore voice and automatic language switching in NVDA2022.1 #13732

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Windows OneCore voices included in Windows 10. #7110

Support for Windows OneCore voices included in Windows 10. #7110

jcsteh commented Apr 28, 2017

michaelDCurran left a comment

michaelDCurran May 2, 2017

michaelDCurran May 2, 2017

michaelDCurran May 2, 2017

jcsteh commented May 15, 2017

feerrenrut left a comment

jcsteh commented May 17, 2017 •

edited

jcsteh commented May 24, 2017

feerrenrut May 25, 2017

jcsteh May 25, 2017

feerrenrut May 25, 2017

jcsteh commented May 26, 2017

feerrenrut left a comment

feerrenrut May 29, 2017

jcsteh May 29, 2017

feerrenrut May 29, 2017

feerrenrut commented May 30, 2017

jcsteh commented May 30, 2017

Support for Windows OneCore voices included in Windows 10. #7110

Support for Windows OneCore voices included in Windows 10. #7110

Conversation

jcsteh commented Apr 28, 2017

michaelDCurran left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcsteh commented May 15, 2017

feerrenrut left a comment

Choose a reason for hiding this comment

jcsteh commented May 17, 2017 • edited

jcsteh commented May 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcsteh commented May 26, 2017

feerrenrut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feerrenrut commented May 30, 2017

jcsteh commented May 30, 2017

jcsteh commented May 17, 2017 •

edited