Skip to content

Implement BreakCommand for SAPI4, fix eSpeak#13875

Merged
seanbudd merged 2 commits into
masterfrom
fixBreakCommand
Jul 12, 2022
Merged

Implement BreakCommand for SAPI4, fix eSpeak#13875
seanbudd merged 2 commits into
masterfrom
fixBreakCommand

Conversation

@seanbudd

@seanbudd seanbudd commented Jul 7, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Link to issue number:

None

Summary of the issue:

The BreakCommand is not implemented with SAPI4.
There is also a known issue with eSpeak not supporting the break SSML tag correctly.

Description of user facing changes

Fix pauses being announced correctly when using MathPlayer and SAPI4 or eSpeak.

Description of development approach

For SAPI5 the Pau tag is used to implement the pause.

There is a known issue with eSpeak and the break SSML tag, which has been fixed with a workaround, adding the strength attribute, while we wait for espeak-ng/espeak-ng#1232 to be fixed.

Testing strategy:

Manual testing

Running the following code in the Python console using

  • SAPI5
  • eSpeak
  • OneCore
  • SAPI 4
import core
from synthDriverHandler import getSynth
from speech.commands import BreakCommand
# returning None suppresses NVDA from announcing the return object CallLater
# which makes it harder to confirm the timed delay
s = lambda x: None if core.callLater(1, getSynth().speak, [BreakCommand(x), "test"]) else None
s(100)  # 100ms
# wait and test the next value
s(1000) # 1s
# wait and test the next value
s(5000) # 5s

Known issues with pull request:

None

Change log entries:

New features
Changes
- Improve eSpeak and SAPI 4 pause lengths when encountering commas and breaks using MathPlayer. 
Bug fixes
For Developers
- The SAPI 4 Synth Driver now supports ``BreakCommand``.

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English

@seanbudd seanbudd requested a review from a team as a code owner July 7, 2022 07:38
@seanbudd seanbudd requested a review from feerrenrut July 7, 2022 07:38
@seanbudd seanbudd mentioned this pull request Jul 7, 2022
Merged
5 tasks
@codeofdusk

This comment was marked as resolved.

Sign in to view
@seanbudd seanbudd self-assigned this Jul 8, 2022
@seanbudd

seanbudd commented Jul 11, 2022

Copy link
Copy Markdown
Member Author

@feerrenrut - this has now been tested with SAPI4 using the steps in #12152

feerrenrut
feerrenrut approved these changes Jul 11, 2022
View reviewed changes

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good.

Comment thread source/synthDrivers/espeak.py Outdated
import os
from collections import OrderedDict
from typing import Optional, Set
from typing import Dict, List, Optional, Set

@feerrenrut feerrenrut Jul 11, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please split onto multiple lines.

@seanbudd seanbudd merged commit c32855c into master Jul 12, 2022
@seanbudd seanbudd deleted the fixBreakCommand branch July 12, 2022 02:08
@nvaccessAuto nvaccessAuto added this to the 2022.3 milestone Jul 12, 2022
@djphoenix

djphoenix commented Dec 10, 2022 •
edited
Loading

Copy link
Copy Markdown

@seanbudd @feerrenrut hi!
espeak-ng/espeak-ng#1232 is fixed now, so I think that "strength" workaround should be removed with next update of espeak.

@feerrenrut

feerrenrut commented Dec 12, 2022

Copy link
Copy Markdown
Contributor

@djphoenix Could you please open a new issue to track this?

@seanbudd

seanbudd commented Dec 12, 2022

Copy link
Copy Markdown
Member Author

Note, this will need to happen after/when updating eSpeak

This was referenced Dec 22, 2022
Closed
Merged
seanbudd added a commit that referenced this pull request Jan 4, 2023
@seanbudd
Update eSpeak to a51235aa commit (#14483)
3183fce 
Closes #14281
Closes #14241
Addresses #13875 (comment)

Summary of the issue:
Janitorial update of eSpeak.
Removes a workaround added to handle a bug with eSpeaks BreakCommand implementation, tracked in espeak-ng/espeak-ng#1232, #13875 (comment)

Description of user facing changes
eSpeak is updated.
Fixes pronunciation of large numbers (#14241).

Description of development approach
Dropped strength for eSpeak BreakCommand attribute per espeak-ng/espeak-ng#1232 being fixed in eSpeak.
@seanbudd seanbudd mentioned this pull request Jan 5, 2023
Merged
6 tasks
seanbudd added a commit that referenced this pull request Jan 6, 2023
@seanbudd
Update eSpeak to a51235aa commit (#14517)
6dd9002 
#14483 was reverted via #14516 due to an issue with rate boost.
eSpeak changed the compilation flag, and that change wasn't in the Makefile.am diff, so it was missed.
This is fixed in 7e558da

Link to issue number:
Closes #14281
Closes #14241
Addresses #13875 (comment)

Summary of the issue:
Janitorial update of eSpeak.
Removes a workaround added to handle a bug with eSpeaks BreakCommand implementation, tracked in espeak-ng/espeak-ng#1232, #13875 (comment)

Description of user facing changes
eSpeak is updated.
Fixes pronunciation of large numbers (#14241).

Description of development approach
Dropped strength for eSpeak BreakCommand attribute per espeak-ng/espeak-ng#1232 being fixed in eSpeak.

Janitorial update
Followed instructions in include/espeak.md.
Checked the following diffs.

cd include/espeak
git diff a51235aa b17ed2d6 src/windows/config.h
git diff a51235aa b17ed2d6 Makefile.am
Addressed the following eSpeak changes:

Introduction of langopts.c: espeak-ng/espeak-ng@4a28902
Fix up of code standards: change DINCLUDE to DUSE: espeak-ng/espeak-ng@ca1f590
Note that eSpeak has changed MBROLA compilation, does not affect our build: espeak-ng/espeak-ng@78ac6c4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@feerrenrut feerrenrut feerrenrut approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@seanbudd seanbudd

Labels

None yet

Projects

None yet

Milestone

2022.3

Development

Successfully merging this pull request may close these issues.

5 participants

@seanbudd @codeofdusk @djphoenix @feerrenrut @nvaccessAuto