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

Move voice dict files #7666

Merged
merged 5 commits into from Nov 1, 2017
Merged

Move voice dict files #7666

merged 5 commits into from Nov 1, 2017

Conversation

feerrenrut
Copy link
Contributor

Link to issue number:

#7592

Summary of the issue:

Espeak voice names have changed, voice dictionaries use the voice name in their filename, now NVDA is not able to find the dictionaries with the new voice name.

Description of how this pull request fixes the issue:

This PR uses a mapping of the old to new espeak voice names to rename the dictionary files. Old files are backed up to preserve the old name in case something goes wrong. The files are put into a versioned directory to make future updates to the names possible.

Testing performed:

Created a default dictionary, and a voice dictionary for each of the synths on my system (espeak, SAPI5, OneCore) using NVDA 2017.3.
Copied these into the branch, and ensured that the dictionaries can be loaded.

Known issues with pull request:

This does not use the voice ID, so future regressions due to name changes of synth voices are possible. This was done because synths such as SAPI5 and OneCore use registry paths as their ID's. These are not friendly to end users, who should be able to manually identify and manage their dictionary files. These long ID's also run the risk of creating file paths that exceed maximum length.

Any users on master or next builds who have created new dictionary entries, or renamed old dictionaries to work with espeak will have to manually fix their dictionary files.

Change log entry:

In Changes:
- Voice dictionary files are now versioned and have been moved to the 'speechDicts/voiceDicts.v1' directory. (#7592)

Backup and move voice dict files, introduce a versioning scheme.
Espeak voice dictionaries are renamed to use the new name for voices to
avoide "lost" dictionaries.
Fixes #7592
@Brian1Gaff
Copy link

Brian1Gaff commented Oct 17, 2017 via email

@@ -0,0 +1,10 @@
# -*- coding: UTF-8 -*-
#A part of NonVisual Desktop Access (NVDA)
#Copyright (C) 2017 NVDA Contributors <http://www.nvda-project.org/>
Copy link
Member

Choose a reason for hiding this comment

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

Copyright should be NV Access Limited now.

@feerrenrut
Copy link
Contributor Author

@Brian1Gaff , after this change you will need to manually fix your dictionary files again. This could be by reverting the name change you made, or by manually moving the files to the new folder.

While this issue did not affect onecore or SAPI5, its best if all voice dictionaries follow the same conventions.

Update copyright and some comments
feerrenrut added a commit that referenced this pull request Oct 18, 2017
Merge branch 'i7592-fixEspeakDictNames' into next
@Brian1Gaff
Copy link

Brian1Gaff commented Oct 18, 2017 via email

@feerrenrut
Copy link
Contributor Author

@Brian1Gaff we use the "name" property of a voice in the file name. Espeak updated the names of their voices, adding capital letters, and using more specific descriptions for the voices. This meant that the files needed the new name in order to "match up".

I just tested this again, and it seems I misspoke. As long as you only have one copy of the voice dictionary then this will automatically move the dictionary to the new location. Regardless of whether the file uses the old name "espeak-english.dic" or the new name "espeak-English (Great Britain).dic". If there are both are present, you might not get the result you want, however all files are backed up to userconfig/speechDicts/voiceDictsBackup.v0 so you will be able to manually fix it.

The files are moved to userconfig/speechDicts/voiceDicts.v1/espeak/

I hope this clears it up for you.

@Brian1Gaff
Copy link

Brian1Gaff commented Oct 19, 2017 via email

When running from the launcher, dont upgrade NVDA config or move espeak
voice dictionaries. Fixes #7688
@feerrenrut
Copy link
Contributor Author

Mick, when you get a chance, can you review the latest changes? This should address the concerns in #7688

@@ -316,6 +316,10 @@ def __init__(self):
self._shouldHandleProfileSwitch = True
self._pendingHandleProfileSwitch = False
self._suspendedTriggers = None
# Never save the config if running securely or if running from the launcher.
# When running from the launcher we dont save settings becuase the user may decide not to install this version,
Copy link
Member

Choose a reason for hiding this comment

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

typo

@@ -316,6 +316,10 @@ def __init__(self):
self._shouldHandleProfileSwitch = True
self._pendingHandleProfileSwitch = False
self._suspendedTriggers = None
# Never save the config if running securely or if running from the launcher.
# When running from the launcher we dont save settings becuase the user may decide not to install this version,
# and these settings may not be compatible with with the already installed version. See #7688
Copy link
Member

Choose a reason for hiding this comment

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

typo

if globalVars.appArgs.secure:
# Never save the config if running securely.
if not self._shouldWriteProfile:
log.info("Not writting profile, either --secure or --launcher args present")
Copy link
Member

Choose a reason for hiding this comment

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

typo

""" Do any upgrades required for the synth passed in.
"""
if globalVars.appArgs.launcher:
# When running from the launcher we dont upgrade dicts becuase the user may decide not to install this version,
Copy link
Member

Choose a reason for hiding this comment

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

typo

"""
if globalVars.appArgs.launcher:
# When running from the launcher we dont upgrade dicts becuase the user may decide not to install this version,
# and the dict location may not be compatible with with the already installed version. See #7688
Copy link
Member

Choose a reason for hiding this comment

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

typo

@@ -12,19 +12,22 @@

SCHEMA_VERSION_KEY = "schemaVersion"

def upgrade(profile, validator):
def upgrade(profile, validator, writeProfileToFileFunc, shouldWriteProfileToFile):
Copy link
Member

Choose a reason for hiding this comment

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

Why not just support profileFileFunc being None and therefore saving won't happen, rather than requring the extra boolean?

feerrenrut added a commit that referenced this pull request Oct 25, 2017
Merge remote-tracking branch 'origin/pr/7666' into next
@feerrenrut
Copy link
Contributor Author

I had to make a change to fix the unit tests, I will incubate this again.

feerrenrut added a commit that referenced this pull request Oct 25, 2017
Merge remote-tracking branch 'origin/pr/7666' into next
@Brian1Gaff
Copy link

Brian1Gaff commented Oct 26, 2017 via email

@feerrenrut
Copy link
Contributor Author

@Brian1Gaff This change hasn't made it to master yet, neither has the new espeak version. These are likely to be merged later this week. Swapping back and forth between master and next is likely to cause complications when there are changes incubating that to affect user config (including the dictionaries).

@Brian1Gaff
Copy link

Brian1Gaff commented Oct 30, 2017 via email

@feerrenrut
Copy link
Contributor Author

I'm not sure if it fixes the pronunciations, this version is currently in use on the next branch. If you have specific examples, I would suggest that you create a new issue on the espeak-ng repository.

@Brian1Gaff
Copy link

Brian1Gaff commented Oct 31, 2017 via email

@feerrenrut feerrenrut merged commit d63f4ed into master Nov 1, 2017
@nvaccessAuto nvaccessAuto added this to the 2017.4 milestone Nov 1, 2017
feerrenrut added a commit that referenced this pull request Nov 1, 2017
- Voice dictionary files are now versioned and have been moved to the
  'speechDicts/voiceDicts.v1' directory. (#7592)
- Versioned files (user configuration, voice dictionaries)
  modifications are no longer saved when NVDA is run from the launcher.
  (#7688)
@feerrenrut feerrenrut deleted the i7592-fixEspeakDictNames branch January 17, 2020 09:00
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.

None yet

4 participants