Skip to content
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

Add support for the Speech dispatcher socket #5272

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

tyrylu
Copy link

@tyrylu tyrylu commented Jan 31, 2023

This PR allows accessing a Flatpak app the socket used for communication with the Speech dispatcher daemon. The issue of this socket not being accessible came when testing a possibility of switching Firefox to the Flatpak on Fedora.

common/flatpak-run.c Outdated Show resolved Hide resolved
@Erick555
Copy link
Contributor

The potential problem with the new mount options is they seem to not work simultaneously with currently used --filesystem options, i.e. --filesystem=xdg-run/speech-dispatcher:ro.

Older flatpak clients won't know about new option which means you need to wait few years until it become usable.

@smcv
Copy link
Collaborator

smcv commented Feb 6, 2023

The potential problem with the new mount options is they seem to #5095 simultaneously with currently used --filesystem options, i.e. --filesystem=xdg-run/speech-dispatcher:ro.

They should work, as long as they're implemented correctly (see #5286). At the risk of stating the obvious, Flatpak should never segfault, even if given incompatible options, so if it does then that's a bug.

@xhorak
Copy link

xhorak commented Jun 8, 2023

Are we still interested in this one? We most likely need that for the https://bugzilla.mozilla.org/show_bug.cgi?id=1638150 . If so, could you @tyrylu rebase the PR that it can be merged?

@Erick555
Copy link
Contributor

Erick555 commented Jun 8, 2023

This is mostly wrapping --filesystem=xdg-run/speech-dispatcher:ro under new name. Similarly to gpg-socket it would take long time before new option will be supported by most clients. I'm not sure if it even make sense. You may just use --filesystem= like everyone else does.

@tyrylu
Copy link
Author

tyrylu commented Jun 8, 2023

Not exactly, at least this handles the case the Speech dispatcher socket is moved to a different location than the default.

@Erick555
Copy link
Contributor

Erick555 commented Jun 8, 2023

yeah but that's relevant for 1% users and most of them will still miss this feature. Even if this lands tomorrow all apps still need to have --filesystem=xdg-run/speech-dispatcher:ro

@tyrylu
Copy link
Author

tyrylu commented Jun 8, 2023

Yeah, likely, they do. So, we're ignoring the users or teaching them about permission overrides, then? And, of course, by forcing usage of the paths, you're exposing implementation details, which the developers now must learn about, even they don't have to care when using Speech dispatcher, as libspeechd abstracts them from it.

@Erick555
Copy link
Contributor

Erick555 commented Jun 8, 2023

The point is this change is irrelevant for near future same as gpg-socket is. It could be merged sure, no problem with that yet for current ff users this doesn't change anything and it still need to use filesystem permission for years to come.

I would like to set expectation for this change right as there may be false impression that after this lands ff could add --socket=speech-dispatcher and fix their issue.

@tyrylu
Copy link
Author

tyrylu commented Jun 9, 2023

I know that. But we can at least respond much simply to others if the came across the same issue and are creating a new app and have the right Flatpak version.

@grulja
Copy link
Contributor

grulja commented Jan 17, 2024

Hi @tyrylu, can you rebase your change please?

* License along with this library. If not, see <http:https://www.gnu.org/licenses/>.
*
* Authors:
* Alexander Larsson <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

You can put yourself as an author. Same applies to other files. Also the year can be updated.

tyrylu and others added 5 commits January 17, 2024 17:32
This commit allows accessing a Flatpak app the socket used for communication with the Speech dispatcher daemon.
@securegh
Copy link

Is there any updates regarding this issue?
I have a problem with two flatpak applications that can't access speech-dispatcher, Brave Browser and Kodoo reader.
Also, adding --filesystem=xdg-run/speech-dispatcher:ro Does not seem to work, I hope the solution proposed here fixes the the problem.
Thank you!

@Erick555
Copy link
Contributor

Erick555 commented Apr 28, 2024

Also, adding --filesystem=xdg-run/speech-dispatcher:ro Does not seem to work, I hope the solution proposed here fixes the the problem.

You need to figure out why it doesn't work because solution proposed here works almost identical to filesystem option.

@tyrylu
Copy link
Author

tyrylu commented Apr 29, 2024

I am interested myself, yes, the approach of this PR is basically the same.

@securegh
Copy link

You need to figure our why it doesn't work because solution proposed here works almostidentical to filesystem option.

This has been a hard problem to troubleshoot and there is not much help online since this seems to be niche edge case.

So far, I was able to make the Appimage version of Koodo Reader to work with speech-dispatcher by adding cmd flag argument --enable-speech-dispatcher and I am not sure if those are compatible with flatpak.
This didn't work for the flatpak version, I tried with this cmd:
flatpak run --filesystem=xdg-run/speech-dispatcher:ro --command=koodo io.github.troyeguo.koodo-reader --enable-speech-dispatcher

It's a similar issue with Brave browser.

I hope you can point me to the right direction to solve this problem.

Thank you!

ps. I am running flatpak 1.15.4 on fedora 38 kernel 6.7

@Erick555
Copy link
Contributor

Try running speech-dispatcher -s -t0 in terminal then flatpak run --filesystem=xdg-run/speech-dispatcher:ro --command=koodo io.github.troyeguo.koodo-reader --enable-speech-dispatcher in another.

@securegh
Copy link

Try running speech-dispatcher -s -t0 in terminal then flatpak run --filesystem=xdg-run/speech-dispatcher:ro --command=koodo io.github.troyeguo.koodo-reader --enable-speech-dispatcher in another.

Thank you for your suggestion, unfortunately it didn't work.

@tyrylu
Copy link
Author

tyrylu commented Apr 30, 2024

Where did it fail? Any console error messages?

@securegh
Copy link

securegh commented May 1, 2024

Where did it fail? Any console error messages?

Nothing insightful, it's just I can't use TTS inside the the application.

Here is the log when running the app from the terminal:

flatpak run --filesystem=xdg-run/speech-dispatcher:ro --command=koodo io.github.troyeguo.koodo-reader --enable-speech-dispatcher
Gtk-Message: 12:17:48.011: Failed to load module "canberra-gtk-module"
Gtk-Message: 12:17:48.012: Failed to load module "pk-gtk-module"
Gtk-Message: 12:17:48.013: Failed to load module "canberra-gtk-module"
Gtk-Message: 12:17:48.013: Failed to load module "pk-gtk-module"
[2:0501/121748.023471:ERROR:bus.cc(392)] Failed to connect to the bus: Failed to connect to socket /run/dbus/system_bus_socket: No such file or directory
[2:0501/121748.027059:ERROR:address_tracker_linux.cc(214)] Could not bind NETLINK socket: Address already in use (98)
libva error: /usr/lib/x86_64-linux-gnu/dri/intel-vaapi-driver/iHD_drv_video.so init failed
[42:0501/121748.150069:ERROR:sandbox_linux.cc(374)] InitializeSandbox() called with multiple threads in process gpu-process.
[2:0501/121749.131368:ERROR:nss_util.cc(169)] Error initializing NSS with a persistent database (sql:/home/gh/.pki/nssdb): NSS error code: -8126

The issue here seems to me is more related to the cmd arg --enable-speech-dispatcher not being passed or interpreted by the application somehow. I don't how I can troubleshoot that. I think the same is the case for Brave Browser.

Can you kindly, test that speech-dispatcher is working with the flatpak version of Brave Browser?

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants