-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
Implement Basic Text to Speech #1143
Conversation
I wasn't able to implement PAUSE, RESUME, and volume. PAUSE/RESUME: Qt relies on the underlying system's audio engines to perform these operations and Qt has quite some buggy behavior on those engines. I tried for a long time to find solutions but none is compatible with all engines and Qt versions. This leads to the compromise of simply having Speak & STOP. Volume: From what I tested, the Linux "speech" engines' audio doesn't change during the reading, which in my opinion makes it useless. Considering we are shipping cross-platform, it is unreliable to have features that work on one but not on another. If we can find a way to package Qt with a working speech engine, we might be able to implement PAUSE/RESUME. However, I have no idea how to do that. @veloman-yunkan @mgautierfr Do you by any chance have an idea of how hard it would be to package an external library like Flite with Kiwix-Desktop? If it's a huge hurdle and we only get the PAUSE/RESUME, then I wouldn't say the return on the effort is great. The priority for this PR would be to have Kiwix-Desktop be compatible with Qt6.4+, CI builds, and a satisfactory UI/UX given the existing feature. |
@ShaopengLin Thank you very much for your PR. For the moment I'm not able to compile it on my Ubuntu 22.04 but should move soon to 24.04 and hope this will allow me to have Qt 6.4. In the meantime we need to secure the backward compatibility with older version of Qt (both version 5 and 6). Please fix the compilation script and the code to secure that this feature is only activated if Qt6.4 or higher is available, otherwise just deactive the feature in a way both compilation and run are fine. |
e06256d
to
ba921f6
Compare
@kelson42 The compilation is now for Qt 6.4 +. |
@ShaopengLin I have moved to 24.04. Would you be able please to rebase on |
ba921f6
to
f4ad9f8
Compare
@kelson42 Done. From my own testing, TTS from Qt is rather unstable and sometimes crashes the application when used for no apparent reason/pattern at all. I have been trying my best to find out if there is a solution to the core problem, though little information really floats around the internet on this topic. I honestly hope it's just my own computer's issue... Please try it on your machine and see if any crashes or misbehaviors exist. If I cannot figure out why those things occur, rather than relying on Qt's flawed implementation, the goal should be improving screen reader compatibilities. When you test, the dfki packages(if exists) are likely not going to work so select the other ones. |
@kelson42 I have found the root cause to the crashes. Will put up a fixup commit soon... |
6937c65
to
1b9265c
Compare
@kelson42 Should be ready for testing now. |
@ShaopengLin I see the menu entry but nothing happens if I click on it: no sound not bottom bar visible. I use Ubuntu 24.04 wieth Qt 5.15.13. |
@ShaopengLin Then the option should not be available (or grayed) |
@ShaopengLin For me, does not compile with Qt 6.4.2:
|
1b9265c
to
5ce8df9
Compare
@sgourdas Can you please fix compilation regression we have now with Qt6? This is blocking review of this PR. |
@kelson42 was this not fixed with #1212 by @ShaopengLin? |
Actually I don't have Qt6.7+ on my Ubuntu 24.04. How should i test this? @ShaopengLin @sgourdas Do you use a PPA to get the most recent version? |
If I remember correctly I had built it from the git source. Would you like me to test something? |
52fac5b
to
c62418d
Compare
What does that technically means "does not have texttospeech"? It means no boice is available? |
@kelson42 The Qt TextToSpeech module is missing in that CI. |
@ShaopengLin The problem is not really the CI but what will happen if for some reason we have Qt5 without all the necessary. My best guess is that we should be prepared to such a scenario and hide or desactivate the feature if this lack is detected at start. Or is that depending to a certain version of Qt5? |
@kelson42 Don't believe it should depend on a certain version of Qt5, but I suppose having safeguards against this would be ideal. I will change this shortly. |
7b629ff
to
45a1139
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering TTS as an accessibility feature for people with vision problems, it should be primarily accessible via keyboard. I know that kiwix-desktop's accessibility in that regard is quite poor, however introducing TTS without a way of controlling it via keyboard doesn't seem wise.
if (m_speech.state() == QTextToSpeech::Speaking) | ||
speak(m_text); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, that won't be good UX. If possible (given the limitation of pause/resuming functionality of QTextToSpeech
) it would be better to pause the speech, change the voice and resume the speech.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first thought as well, unfortunately, QTextToSpeech's pause and resume barely work.
Basic Reading of Articles or Selected Texts.
Added the bar at bottom of webpage. Intoduced the stop speaking button.
Add shortcut on keyboard to stop reading for accessibility.
Bar is default closed and shown on any speech activation.
The line edit should not be edit/highlight/selectable.
Rather than only the arrow, clicking any place on the combobox should popup the selector
Tabs are by default Zim's Locale
Keep track of the voices so user do not need to reconfigure.
Show the TTS bar and popup the respective selector on shortcut for accessibility.
Some systems or QT versions might not have or the module is not available.
45a1139
to
e3106ea
Compare
Added a new set of shortcuts for TTS listed on the PR description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Didn't test.
Fix #44
Changes:
Shortcut: