Split speech processing commands and commands for synth

feerrenrut commented 3 years ago

Is your feature request related to a problem? Please describe.

Speech commands for synthesizers and speech commands for internal speech processing are not well separated.
The SpeechManager class is the boundary between internal speech processing and the synthesizer, however it isn't entirely clear which commands may exist on either side of that boundary.
SynthDrivers have a list of supported commands, however these aren't actually considered anywhere.

Describe the solution you'd like

Make a new base class for SynthCommands. Commands like PitchCommand should inherit from there.
Either through design (better), or documentation (worse) make the separation of intents for commands for synth and internal speech commands clear .
Enforce SynthDriver.supportedCommands from speech manager (with reasonable defaults if the driver provides and empty list).

Reasonable defaults is probably:

    supportedCommands = {
        IndexCommand,
        CharacterModeCommand,
        LangChangeCommand,
        BreakCommand,
        PitchCommand,
        RateCommand,
        VolumeCommand,
        PhonemeCommand,
    }

Describe alternatives you've considered

None

Additional context

While working on #12710 the conflated purposes of speech commands became clear.

lukaszgo1 commented 3 years ago

* Enforce `SynthDriver.supportedCommands` from speech manager (with reasonable defaults if the driver provides and empty list).

Wouldn't it be better to refuse to use drivers providing an empty list?

Reasonable defaults is probably:


  supportedCommands = {
      IndexCommand,
      CharacterModeCommand,
      LangChangeCommand,

I'm not sure about this one - now we're assuming that every engine out there supports more than one language.

feerrenrut commented 3 years ago

The defaults are based on what we currently do.

Wouldn't it be better to refuse to use drivers providing an empty list?

This would break synths unnecessarily. Instead we give them a chance to override the value, if they don't provide a sensible list (empty is not sensible), we use the defaults as before.

now we're assuming that every engine out there supports more than one language.

No, only that the synth driver handles the LangChangeCommand, it doesn't have to pass it on, but it would be better for that synth to override the supported commands to exclude it.

I'd be willing to consider dropping LangChangeCommand from the defaults, but this would be a compat breaking change.

Adriani90 commented 3 days ago

I think this would improve alot the flexibility of NVDA in terms of changing voice or the entire synth in different situations automatically (e.g. different syntethizers for different languages when automatic language change is enabled).

cc: @CyrilleB79

nvaccess / nvda