Update NVDA Controller client to support speaking SSML sequences#15734
Conversation
690511c to
cb0bf83
Compare
cb0bf83 to
1b0ff0a
Compare
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
| - Providing the symbol level. | ||
| - Providing the priority of speech to be spoken. | ||
| - Speaking both synchronously (blocking) and asynchronously (instant return). | ||
| - The exported ``_nvdaController_onSsmlMarkReached`` pointer can be assigned to a callback function that is called in synchronous mode for every ``<mark />`` tag encountered in the SSML sequence. |
There was a problem hiding this comment.
It is very likely my lack of understanding of how RPC rather than a real problem, but looking at this I cannot figure out how two applications wanting to provide their own callback for onSsmlMarkReached can do so. Since it seems to be necessary to assign the callback to the global pointer, it appears that, to be on the safe side, application developer should assign the old value of onSsmlMarkReached to a temporary variable before passing any speech to NVDA, assign their own callback, speak whatever has to be spoken, and restore the old value afterwards. Would it be possible to pass pointer to onSsmlMarkReached as an additional parameter to nvdaController_speakSsml instead of using the single global callback?
There was a problem hiding this comment.
it appears that, to be on the safe side, application developer should assign the old value of
onSsmlMarkReachedto a temporary variable before passing any speech to NVDA, assign their own callback, speak whatever has to be spoken, and restore the old value afterwards.
That's not necessary. The old value is nullptr anyway and the internal nvdaController_onSsmlMarkReached only calls the exported pointer when it is set correctly.
Would it be possible to pass pointer to
onSsmlMarkReachedas an additional parameter tonvdaController_speakSsmlinstead of using the single global callback?
That would be a lot nicer indeed, though I don't think we'd be able to pass the function pointer over RPC.
I'll have a look into this.
There was a problem hiding this comment.
it appears that, to be on the safe side, application developer should assign the old value of
onSsmlMarkReachedto a temporary variable before passing any speech to NVDA, assign their own callback, speak whatever has to be spoken, and restore the old value afterwards.That's not necessary. The old value is nullptr anyway and the internal nvdaController_onSsmlMarkReached only calls the exported pointer when it is set correctly.
Backing up and restoring the old function pointer seems to still be necessary, without doing so I see no way in which two applications can provide their own callback. Without backing up and restoring, the last assigned callback will always be used.
There was a problem hiding this comment.
Every application can provide its own callback because the callback is stored in the user space of the client. The server, NVDA in this case, doesn't know anything about what the callback does. A callback is just RPC in the opposite direction.
Is this caused by issues in eSpeak, or our driver for it. In other words should this be reported against NVDA (if so is there an existing issue), or eSpeak (same question)?
Would it be possible to create a system test bundling version 1.0 of controller client, which ensures that the current NVDA can indeed speak through it?
I assume you mend version 1.0 here? |
I recall that @jcsteh bumped into issues with Espeak indexing during speech refactor, but I"m not sure. I'm pretty sure it is an issue in ESpeak though.
I'm not very comfortable with writing system tests honestly. I guess that this can be a unit test though.
Correct, sorry. |
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Doing this as an unit test seems tricky as you need to have |
|
I found out a nice way to avoid the API breakage. I will update the description, changelog and docs accoordingly later this week. |
See test results for failed build of commit 420e50f977 |
See test results for failed build of commit fd7a608ac8 |
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
LeonarddeR
left a comment
There was a problem hiding this comment.
I'm sorry, I had pending comments I forgot to post yesterday.
|
Thanks for approving @seanbudd. Is this still blocked by something? |
|
|
||
| prefixSpeechCommand = None | ||
| markCallable = None | ||
| if not asynchronous: |
There was a problem hiding this comment.
Am I write in understanding that if the function is called with asynchronous as True, the mark callback will not be ever fired?
There was a problem hiding this comment.
This is correct. As per the docs about callbacks:
The callback function ... lets the server query the client for information in the context of the original call. Callbacks are special cases of remote calls that execute as part of a single thread. A callback is issued in the context of a remote call. Any remote procedure defined as part of the same interface as the static callback function can call the callback function.
So in other words, a callback can only be called during a call. Calling the callback as part of an async call that was already completed wouldn't work.
Link to issue number:
Fixes #11028
Fixes #5638
Summary of the issue:
The NVDA Controller client has been stable for a long time, but it lacked support for modern speech features, such as priority and callbacks.
Description of user facing changes
None.
Description of development approach
Added the following functions to the controller client:
nvdaController_getProcessId: To get the process id (PID) of the current instance of NVDA the controller client is using.nvdaController_speakSsml: To instruct NVDA to speak according to the given SSML. This function also supports:nvdaController_setOnSsmlMarkReachedCallback: To register a callback of typeonSsmlMarkReachedFuncTypethat is called in synchronous mode for every<mark />tag encountered in the SSML sequence provided tonvdaController_speakSsml.Testing strategy:
Updated python example. Note that this doesn't work very well with Espeak, but that's more of espeaks fault. This is best tested with OneCore.
Known issues with pull request:
Code Review Checklist: