flathub / com.slack.Slack

https://flathub.org/apps/details/com.slack.Slack
36 stars 36 forks source link

Build with pipewire support to enable wayland screen sharing #118

Closed vchernin closed 3 years ago

vchernin commented 3 years ago

Add pipewire screen sharing support following chromium flatpak's example. Pipewire is necessary to get screen sharing with chromium on wayland, and works correctly now in flatpak with these changes. The feature works fairly well, aside from some relatively minor bugs. The core screen sharing functionality works and is stable.

This PR builds the pipewire module with the flatpak, and also adds a needed chromium pipewire flag to the .desktop file. This feature works out of the box with these fixes, and no manual configuration or flags are needed from end users. This addition does not regress more traditional screen sharing in x11, it should only improve the experience for wayland users (tested on fedora 34 and ubuntu 20.04, on wayland and x11).

Fixes #101

Current issues with this implementation:

Keep in mind the alternative on wayland with slack flatpak is to get no screen sharing at all. This is likely a "good enough" experience for most users. These bugs will likely get resolved as chromium, electron, slack and pipewire release updates.

This work is based off chromium flatpak, given that slack is already built with pipewire support (if slack wasn't built with pipewire support none of this would work). The only thing missing at buildtime were the correct flatpak modules and a runtime flag.

Also note this PR does not and is not intended to enable wayland by default for slack. Enabling wayland by default may need a seperate PR or upstream effort.

This PR uses pipewire 0.2.x instead of the newer 0.3.x for the reasons listed here. In short there is no obvious benefit to using 0.3.x for now, but this could definitely change in the future.

See here for what seems to work for chromium to solve their screen sharing on wayland issue.

flathubbot commented 3 years ago

Started test build 47054

flathubbot commented 3 years ago

Build 47054 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/45251/com.slack.Slack.flatpakref
vchernin commented 3 years ago

I have tested this build and I haven't spotted any obvious problems. I am able to sign in and send messages and whatnot just fine. Calling seems to work, but amusingly, I am not actually able to test screen sharing since slack requires a paid plan for screen sharing, and I don't have one.

So if anyone who has a paid slack plan wants to test this out install the test build of the flatpak provided by flathubbot.

When testing screen sharing you must open slack with: (you can safely ignore this now)

flatpak run com.slack.Slack --enable-features=WebRTCPipeWireCapturer

If it's been 5 days since the last build summon the bot again with the command listed here.

vchernin commented 3 years ago

I managed to get a free trial of slack's premium plan, and yes flatpak wayland screen sharing works with this PR!

Pipewire working with slack flatpak Left 2 windows are my second user, middle 2 windows are the slack app (where I'm sharing from), on the right is the window I am sharing.

A few notes:

vchernin commented 3 years ago

I believe its best to keep the 0.2.7 version of pipewire (the latest pipewire in the 0.2 series).

0.3.x was only recently introduced in chromium, and requires an additional flag. I'm not even sure if slack is on the latest electron version (v12), so it's possible 0.3.x won't work at all until slack updates to the latest electron.

Moreover, pipewire 0.2.x works in systems like rhel 7 and debian 10, where 0.3.x is not present out of the box. So I don't see an explict benefit to shipping pipewire 0.3.x with the slack flatpak for the time being. The issues that are present (like with confusing dialogs) are likely chromium issues and not pipewire issues, so using a more up to date pipewire likely won't benefit slack much.

flathubbot commented 3 years ago

Started test build 47086

flathubbot commented 3 years ago

Build 47086 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/45283/com.slack.Slack.flatpakref
flathubbot commented 3 years ago

Started test build 47087

flathubbot commented 3 years ago

Build 47087 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/45284/com.slack.Slack.flatpakref
flathubbot commented 3 years ago

Started test build 47088

flathubbot commented 3 years ago

Build 47088 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/45285/com.slack.Slack.flatpakref
vchernin commented 3 years ago

Please test the latest and best build from above (47088). It should not require you to do anything special, you only need to install it and launch slack. No command line options are needed now.

a-lohmann commented 3 years ago

Hi @vchernin,

I've succesfully tested build 47054 on Fedora 33 with Gnome. To not be dependant on the command line I've changed the exec line in ~/.local/share/flatpak/exports/share/applications/com.slack.Slack.desktop to:

Exec=/usr/bin/flatpak run --branch=test --arch=x86_64 --command=slack --file-forwarding com.slack.Slack --enable-features=WebRTCPipeWireCapturer @@u %U @@

With this option enabled screen sharing works fine in both X.org and Wayland. Great job!

I'll try the latest build as well, just to be sure.

vchernin commented 3 years ago

Thank you for the test @a-lohmann!

If anyone is running a distro that doesn’t ship pipewire support by default (like ubuntu != 21.04), could you test this PR out on x11 and wayland? It’s possible the results will be different than on a very up to date distro like fedora.

Please try the latest build, 47088. Wayland screen sharing shouldn’t work on a distro without proper pipewire but x11 screen sharing should work.

If behaviour is as expected on an older distro version I think this PR is tested enough to be merged.

vchernin commented 3 years ago

I have tested build 47088 on a ubuntu 20.04 vm on both xorg and wayland, and as expected xorg works perfectly as always, but wayland does not. To be clear, this is expected and is due to an ubuntu 20.04 bug where pipewire was not properly included in the distro. This pr being included does not cause a regression, since wayland screen sharing never worked in 20.04 in the first place.

There is nothing we can do that I am aware of that would improve the experience for older ubuntu users that are on wayland. Those who want wayland on ubuntu should upgrade to 21.04. There the experience should work since ubuntu ships pipewire correctly just like in fedora and other distros.

Since this means there are no regressions on the major test scenarios (at least the ones I thought of), I believe this PR is ready to be merged.

Vindex17 commented 3 years ago

Hello. When will this be merged? I need this feature to work remotely

vchernin commented 3 years ago

@FakeShemp could we get this merged? I tested for regressions and couldn’t find any. Also Jitsi Meet recently made a change equivalent to what we’re doing here. I think this is quite safe to merge.

All we’re doing here is building the Flatpak with PipeWire and then passing the Chromium PipeWire flag by default. Slack support even officially recommends running with the flag for their .deb builds.

netlore commented 2 years ago

Not completely sure why, given that a year has passed, but I thought this would be useful feedback.

I see the xdg-run/pipewire-0 in the permissions, but I don't see the "--enable-features=WebRTCPipeWireCapturer" in the .desktop file, I added it manually but it doesn't seem to be required any longer?. I have tested on both Ubuntu 22.04, and on the current devel version of 22.10, and while it seems to work correctly on 22.04, I get a distorted image on 22.10, I'm not quite sure why, as it happens for both individual windows and the entire screen!

image

netlore commented 2 years ago

It may be that the distortion is caused by this, unrelated bug:- https://gitlab.gnome.org/GNOME/mutter/-/issues/1913

fernando-jascovich commented 2 years ago

I have the same distortions and I'm not using gnome (or mutter). I'm using sway on ArchLinux.