Skip to content

Add verbose debug log for SAPI4#17774

Merged
SaschaCowley merged 5 commits into
nvaccess:masterfrom
gexgd0419:sapi4-log
Mar 6, 2025
Merged

Add verbose debug log for SAPI4#17774
SaschaCowley merged 5 commits into
nvaccess:masterfrom
gexgd0419:sapi4-log

Conversation

@gexgd0419

@gexgd0419 gexgd0419 commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

Link to issue number:

None

Summary of the issue:

In #17718, I changed the SAPI4 synthesizer code, so that SAPI4 can now use WASAPI WavePlayer for audio output. This was done by creating a custom audio output destination class, and let the SAPI4 engines use the custom destination instead of the built-in MMAudioDest to output audio.

The engines interact with the audio destination object directly, and each engine may have its own way to use the audio destination object. So while the current implementation works with some of the SAPI4 engines, it may not work well with some other engines. As different engines have different behaviors, it may be impossible to replicate the same issue with another voice.

It would be better if I can get access to the exact same voice and study its behavior. But many of those voices are commercial products, which require purchasing and activating, so I cannot easily get the voices.

The second option is to get the log file from the user and study the logs. While I am using logs to debug on my side, the logs may be too chatty for regular users, because every audio chunk would be logged, so I decided to leave the logging part out. But now I find it difficult to diagnose SAPI4-related problems without logs when I can't get access to the voice.

Description of user facing changes

When the debug log category synthDriver in the advanced settings page is enabled, additional verbose SAPI4 logs will be enabled, which includes most of the interactions between the SAPI4 engine and the custom audio destination object, including every audio data write.

This can log many lines when using SAPI4 voices, so it's recommended to keep the log disabled when not necessary.

Description of development approach

Added logging in the SAPI4 module, which can be enabled or disabled by the synthDriver debug log category.

Testing strategy:

Tested manually.

Known issues with pull request:

Code Review Checklist:

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

@coderabbitai summary

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Mar 4, 2025
Comment thread source/config/configSpec.py Outdated
@gexgd0419 gexgd0419 marked this pull request as ready for review March 5, 2025 06:53
@gexgd0419 gexgd0419 requested a review from a team as a code owner March 5, 2025 06:53
@gexgd0419 gexgd0419 requested a review from SaschaCowley March 5, 2025 06:53

@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.

thanks @gexgd0419, can you change the format strings to f-strings?

Comment thread source/synthDrivers/sapi4.py Outdated
@gexgd0419 gexgd0419 marked this pull request as draft March 6, 2025 03:34
@gexgd0419 gexgd0419 marked this pull request as ready for review March 6, 2025 15:49
@SaschaCowley SaschaCowley merged commit 9186b78 into nvaccess:master Mar 6, 2025
@github-actions github-actions Bot added this to the 2025.1 milestone Mar 6, 2025
@gexgd0419 gexgd0419 deleted the sapi4-log branch March 7, 2025 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants