Skip to content

Fix compilation of espeak dictionary for language zhy#12370

Merged
feerrenrut merged 8 commits into
betafrom
fixCantoneseEspeak
May 28, 2021
Merged

Fix compilation of espeak dictionary for language zhy#12370
feerrenrut merged 8 commits into
betafrom
fixCantoneseEspeak

Conversation

@feerrenrut

@feerrenrut feerrenrut commented May 6, 2021

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes: #10418

Summary of the issue:

Compiling the zhy dictionary has failed for a long time, it was excluded because the cause was unknown.
It was suspected that there was an error in the format of the files.
Looking into this I found the issue was caused by trying to set the voice to "zhy" by calling espeak_SetVoiceByProperties.
The result was 2 (ENS_VOICE_NOT_FOUND)
Compilation was based on using glob to find the *_rules files, and splitting the filename to get the language to use for the voice.

Description of how this pull request fixes the issue:

After getting advice from espeak-ng devs source files for compiling dictionaries in espeak have been updated.

This change makes the compilation of espeak dictionaries more explicit.
An explicit listing of the dictionaries NVDA expects (rather than using glob), allows us to be aware of the introduction or removal of languages.

Links that might help reviewers:

Testing strategy:

Tested building and running with espeak voice set to Chinese(Cantonese)
Ask alpha users more familiar with this language to test.

Known issues with pull request:

None

Change log entries:

Bug fixes
- eSpeak Cantonese and Mandarin voices are now available again.

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

@AppVeyorBot

This comment has been minimized.

@dpy013

dpy013 commented May 6, 2021

Copy link
Copy Markdown
Contributor

cc @kara-louise

@feerrenrut

Copy link
Copy Markdown
Contributor Author

Rather than determining the names of languages from files, it is probably better to explicitly list the expected languages, input files, and output files and any other settings that may be required. As a next step on this PR I'll update espeak and add an explicit mapping.

@feerrenrut feerrenrut added this to the 2021.1 milestone May 17, 2021
When compiling dicts for espeak, use an explicit list rather than a
glob.
Using glob makes it hard to track added / or removed languages
@feerrenrut feerrenrut force-pushed the fixCantoneseEspeak branch from 43500cf to c63cc7a Compare May 25, 2021 11:05
@feerrenrut feerrenrut marked this pull request as ready for review May 25, 2021 11:40
@feerrenrut feerrenrut requested a review from a team as a code owner May 25, 2021 11:40
@feerrenrut feerrenrut requested a review from seanbudd May 25, 2021 11:40
@feerrenrut feerrenrut mentioned this pull request May 26, 2021
7 tasks
seanbudd
seanbudd previously approved these changes May 26, 2021

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've confirmed that Mandarin and Cantonese are included and work as voices for eSpeak. The change of strategy looks good and makes the code a lot easier to understand, but I left some minor suggestions.

Comment thread nvdaHelper/espeak/sconscript Outdated
Comment thread nvdaHelper/espeak/sconscript Outdated
Comment thread nvdaHelper/espeak/sconscript Outdated
@feerrenrut

Copy link
Copy Markdown
Contributor Author

@seanbudd I have made those changes, also added a note about dictionary compilation to the espeak docs.

Looking at this again, it might be a mistake that we don't list all inputs to the dictionary compilation. As it currently stands, SCons will only recompile the dictionary if the *_rules file changes, however most languages also depend on a *_list file, and some also on a *_listx.

@feerrenrut feerrenrut requested a review from seanbudd May 26, 2021 06:42
@seanbudd

Copy link
Copy Markdown
Member

Looking at this again, it might be a mistake that we don't list all inputs to the dictionary compilation. As it currently stands, SCons will only recompile the dictionary if the *_rules file changes, however most languages also depend on a *_list file, and some also on a *_listx.

Is this still a pending issue then? Otherwise, the changes look good.

@feerrenrut

Copy link
Copy Markdown
Contributor Author

I think it is best if I update it further.

@seanbudd seanbudd dismissed their stale review May 26, 2021 07:49

changes required

Comment thread nvdaHelper/espeak/sconscript
seanbudd
seanbudd previously approved these changes May 27, 2021

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes look good, pending your thoughts on how to generate the dictionary

@feerrenrut feerrenrut changed the base branch from master to beta May 28, 2021 02:35
@feerrenrut feerrenrut dismissed seanbudd’s stale review May 28, 2021 02:35

The base branch was changed.

@feerrenrut feerrenrut merged commit fca3c2d into beta May 28, 2021
@feerrenrut feerrenrut deleted the fixCantoneseEspeak branch May 28, 2021 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The Chinese (Cantonese) voice is broken

4 participants