wwmm / easyeffects

Limiter, compressor, convolver, equalizer and auto volume and many other plugins for PipeWire applications
GNU General Public License v3.0
6.65k stars 270 forks source link

PipeWire version and backwards compatibility #1089

Open vchernin opened 3 years ago

vchernin commented 3 years ago

I am wondering how the PipeWire version should/will be updated for EasyEffects. This is (mostly) a Flatpak issue, but it does somewhat affect distro packages.

Context:

EasyEffects currently depends on PipeWire 0.3.31 at build-time. This is fine for Arch, Fedora and other rapidly updating distros. However, when distros like Ubuntu or Debian likely switch to PipeWire by default it is less likely newer versions of PipeWire will be present. In fact, Ubuntu 21.04 ships with 0.3.24, and Debian 11 with 0.3.19. Both far too old to build or likely run EasyEffects.

With Flatpak we can arbitrarily bundle any PipeWire version so long as it builds. However, I have noticed that bundling and building with 0.3.33 (from Flatpak) and running 0.3.32 (From Fedora host) caused EasyEffects to just not start. A user running dietpi (based off Debian 10 with PipeWire 0.2.x) also ran into this. So Flatpak doesn't really solve the outdated distro PipeWire problem, even if it does offer an alternative to distro EasyEffects packaging.

Question:

P.S. I think the Flatpak is otherwise mostly ready, I'd appreciate if you could look over the current issue list and mention if any should block EasyEffects' release onto Flathub stable.

vchernin commented 3 years ago

Ok, so I've discovered something that might be useful.

With the Flatpak, you can enter the sandbox where EasyEffects is in with: flatpak run --command=sh --devel com.github.wwmm.easyeffects

From there you can run commands like pipewire --version

On my Fedora 34 installation with PipeWire 0.3.33: I noticed that the output of pipewire --version is 0.3.31. This is because 0.3.31 is explictly bundled with the Flatpak at build time to compile EasyEffects.

However, pw-mon outputs version 0.3.33 (as well as other debug info). This means pw-mon run from within the sandbox could possibly be used to detect the host's PipeWire version. EDIT: pw-mon --version gives 0.3.31 (the same as pipewire --version so that can't be used). Instead of pw-mon, pw-dump might be easier since it's JSON.

This means that there could be a script bundled with the Flatpak that would use pw-mon (or a similar command) to output an error based on the host's PipeWire version. @wwmm any thoughts on this kind of check for the host PipeWire version? Should a check like this be run at first launch or every launch (I think every time since the version in Flatpak and the host could change past the initial check).

wwmm commented 3 years ago

I'd appreciate if you could look over the current issue list and mention if any should block EasyEffects' release onto Flathub stable.

While some issues are not nice compared to have no app at all they are not so bad. I think you can release it so it becomes easier for people to test it. Then we try to fix some of the issues like the one related to the icons.

@wwmm any thoughts on this kind of check for the host PipeWire version? Should a check like this be run at first launch or every launch (I think every time since the version in Flatpak and the host could change past the initial check).

It is probably the easiest thing to do. The startup time increase due to this check will be negligible.

vchernin commented 3 years ago

Ok, so I've thrown together a rudimentary but as far as I can tell reliable checker:

#!/bin/sh

# this checks the PipeWire version on the host, and then compares it to what we need.
# current baseline to compile EasyEffects is 0.3.31, thus the host needs minimum that as well.

# get host's PipeWire version:
# jq available in GNOME 40 Flatpak runtime
found_pipewire_version="$(pw-dump | jq .[0].info.version)"

required_pipewire_version="0.3.31" # from EasyEffects' build requirements

# compare versions
# from https://unix.stackexchange.com/a/285928

 if [ "$(printf '%s\n' "$required_pipewire_version" "$found_pipewire_version" | sort -V | head -n1)" = "$required_pipewire_version" ]; then 
        echo "Greater than or equal to ${required_pipewire_version}"
        # pop up dialog should not appear as there is no problem
 else
        echo "Less than ${required_pipewire_version}"
        # pop up dialog should appear as user must install newer PipeWire to use EasyEffects
        # could link to wiki page in EasyEffects repo or something.
 fi

(The echos are only there for debugging's sake, they could be replaced by a meaningful output). Edit: I realize I should put a error catcher in case pw-dump is not present at all. i.e. PipeWire is not out of date, rather it is not present on the host at all.)

Ideally I think a GTK4 dialog explaining the error would be best here, since it would match EasyEffects itself. Is there a good place to put this within EasyEffects' start up logic? I looked but I am not familar enough to say where something like this should be called.

Also, I am wondering if this makes sense to include in EasyEffects' tree. As far as I know this check is only useful for the Flatpak. Furthermore, I don't want to unecessarily add jq as a dependency for other EasyEffects packages (even if I could swap jq for something else). Or maybe it should be a build-time option?

vchernin commented 3 years ago

Through some strange luck I was browsing the PipeWire repo and found this: https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/1515

@wwmm I would definitely read that if you can. I think we should check PipeWire versions not just for Flatpak, but for all builds.

vchernin commented 3 years ago

Or maybe instead of doing version comparisons, EasyEffects should give a user-friendly error if it could not start up properly. Essentially this achieves the same goal, but also ensures we aren't blindly assuming new enough PipeWire would actually work (eg. someone's package manager could have made a mess and installed PipeWire in a broken state).

I think there is already a command line error given "could not communicate with PipeWire" or something. Not sure if that is from EasyEffects or PipeWire itself. I should probably test that again.

I think a better warning would be a GTK4 one, that would tell the user what PipeWire version was detected, and then they could be sent to a wiki page with detailed instructions. If they do have the right PipeWire daemon installed they can be directed to open a bug.

wwmm commented 3 years ago

I think we should check PipeWire versions not just for Flatpak, but for all builds.

I would say that more because it will be the easiest way to handle the check for the flatpak package. The problem that user is facing is not happening because of ABI changes. pw_filter_get_node_id does the same since I started to use PipeWire.

And in his case I do not see how the check would have helped. At this moment our minimum required version is 0.3.31. If I understood it right the issue he described happened when trying to use a package compiled for 0.3.33on a system with PipeWire 0.3.32. Both versions satisfy our requirements. So the check would do nothing.

wwmm commented 3 years ago

EasyEffects should give a user-friendly error if it could not start up properly.

The problem is detecting that. There are many stages that could initialize with no problems while others could fail. I think that there are too many variables to take into account to achieve that. The issue that user is facing is one example. It isn't pw_filter_get_node_id that is failing. For some reason PipeWire did not initialized one of our filters. As a result pw_filter_get_node_id never returns a valid id. In that case we would have to define a timeout and if it takes too long we would have to assume something failed and take some action. But that is the kind of solution you only think about once you know this kind of problem can happen. So it is hard to take into account every part of the code that could fail without seeing they failing first.

vchernin commented 3 years ago

And in his case I do not see how the check would have helped. At this moment our minimum required version is 0.3.31. If I understood it right the issue he described happened when trying to use a package compiled for 0.3.33 on a system with PipeWire 0.3.32. Both versions satisfy our requirements. So the check would do nothing.

My assumption so far is using a package compiled with a too-new PipeWire is the issue. The idea with the script above is to see if the package was built with a newer PipeWire than what is actually on the system. So if the user has 0.3.32 but the package was compiled with 0.3.33 that will give an error. But if the user has 0.3.33 and the package was compiled with 0.3.32 that is fine. This matches with my anecdotal testing.

The problem is detecting that. There are many stages that could initialize with no problems while others could fail. I think that there are too many variables to take into account to achieve that. The issue that user is facing is one example. It isn't pw_filter_get_node_id that is failing. For some reason PipeWire did not initialized one of our filters. As a result pw_filter_get_node_id never returns a valid id. In that case we would have to define a timeout and if it takes too long we would have to assume something failed and take some action. But that is the kind of solution you only think about once you know this kind of problem can happen. So it is hard to take into account every part of the code that could fail without seeing they failing first.

So maybe a simple version check (error if compiled with newer PipeWire than on host) is still called for? It is useful for distros that have no or very old PipeWire (eg. 0.2.x).

wwmm commented 3 years ago

So maybe a simple version check (error if compiled with newer PipeWire than on host) is still called for?

I am not sure if we should "give an error" in the usual sense. It still may work so I would say it would be a warning. In any case getting the information necessary for the check is easy. We already show the strings I just need to compare them and show something to the user Screenshot from 2021-08-15 21-48-18 Header version means compiled with this version and library version means the version of the library found at runtime.

And the warning has to be implemented in a way that it is showed once. Showing a dialog about this every time the window is opened will annoy the users.

vchernin commented 3 years ago

At least the above method won't work for Flatpak: I have 0.3.33 on my Fedora 34 system. 0.3.31 is used at build time.

image

vchernin commented 3 years ago

I am not sure if we should "give an error" in the usual sense. It still may work so I would say it would be a warning.

Sure, I think a warning is fine.

And the warning has to be implemented in a way that it is showed once. Showing a dialog about this every time the window is opened will annoy the users.

I guess there could be a "I acknowledge the potential problem" or something.

wwmm commented 3 years ago

At least the above method won't work for Flatpak: I have 0.3.33 on my Fedora 34 system. 0.3.31 is used at build time.

Well... Then doing this in a way that is integrated with our graphical interface code won't be trivial... Maybe it isn't even possible without resorting to things like std::system... But there is something I do not understand. If PipeWire is reporting to us a library version that matches the one EasyEffects was compiled with at which moment is it using the system server? It seems to me we are not interacting with the server that is outside of the sandbox.

wwmm commented 3 years ago

Just to clarify why it seems weird the library version string is given to us directly by the PipeWire server. I am not querying it somewhere else. If we were talking to the server outside of the sandbox the string would have the other value.

wwmm commented 3 years ago

This is how I get those strings https://github.com/wwmm/easyeffects/blob/356b6babc51d565a4c2abb263a6b38cbe9c7ff7c/src/pipe_manager.cpp#L1107

vchernin commented 3 years ago

As mentioned above I am not entirely sure how Flatpak works here, it is beyond my knowledge about it.

I do know if you enter the Flatpak sandbox you can access the host's version info with the right command:

flatpak run --command=sh --devel com.github.wwmm.easyeffects
pw-dump

But commands like pipewire --version don't help us (they give the bundled ones)

The script above does work correctly with Flatpak (at a basic level at least).

vchernin commented 3 years ago

If PipeWire is reporting to us a library version that matches the one EasyEffects was compiled with at which moment is it using the system server? It seems to me we are not interacting with the server that is outside of the sandbox.

I do know we explictly give the Flatpak access to the PipeWire of the host in some form, other Flatpak apps use this as well. If I remove this permission EasyEffects doesn't start. xdg-run/pipewire-0:ro

wwmm commented 3 years ago

I misunderstood what pw_get_library_version does https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/master/src/pipewire/version.h.in#L39

It shows the version the app was linked to. So it won't help us to show a warning in our window.

I do not think PipeWire has other functions in its API returning this kind of information. It does not seem we can do this in a clean way without external scripts...

wwmm commented 3 years ago

I do not think PipeWire has other functions in its API returning this kind of information.

But Pulseaudio had a way to get the version of the server being used... Maybe PipeWire is doing this in a less obvious place.... I will investigate more this week.

vchernin commented 3 years ago

It appears my assumption/observation was correct, a too new compiled version (client) indeed might or will be an issue:

The server can adapt to older clients but not newer clients, so compatibility is only guaranteed for older clients on newer servers. 0.3.32 server and 0.3.33 client is not going to work (client has newer version of the client-node interface, that the server rejects).

I am not sure if the client-node interface is updated every time, but it definitely appears we can expect compatibility issues unless doing a version check.

For reference and extra clarity the script above applies the same logic as here. If the server version is greater or equal to the compiled version there is no problem. Else there should be an error/warning.

vchernin commented 3 years ago

This is now mostly addressed thanks to https://github.com/flathub/com.github.wwmm.easyeffects/pull/2.

In the future perhaps there could be a cleaner solution integrated into EasyEffects, which means rare but possible cases like https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/1515 could be addressed.

But for the most part I don't think there's a huge concern here anymore. Issues like #1047 or #535 are the ones I'd like to work on now for Flatpak.

v1gnesh commented 3 years ago

Hi,

Is it an option to consider the Debian Janitor for Ubuntu/Debian-based packages. On Ubuntu 21.04, as you mentioned, pipewire version is 0.3.24 and so I was forced by the recent easyeffects update to switch to pipewire-debian PPA. It worked fine with 0.3.24...

Going a bit tangential, there's this issue that I was hoping newer pipewire would solve, but it didn't. Due to this, after waking the system up from hibernate, each time, I have to restart pipewire via systemctl and kill & restart easyeffects (finding process ID, kill, etc.)

So in short, the line command easyeffects doesn't work natively in Ubuntu because it's a flatpak app. flatpak ps lists it, but I'm unable to properly kill and restart it with just flatpak commands. A native build for Debian/Ubuntu will be handy to deal with this pipewire-fails-on-hibernate issue.

vchernin commented 3 years ago

It worked fine with 0.3.24

So this does seem to be at least somewhat the case, the thing is though I can't say we should rely on it working.

As said by PipeWire's main developer:

The server can adapt to older clients but not newer clients, so compatibility is only guaranteed for older clients on newer servers. 0.3.32 server and 0.3.33 client is not going to work (client has newer version of the client-node interface, that the server rejects).

PipeWire itself can't guarantee running a newer client (built against newer PipeWire like 0.3.31) will work on an older server (like 0.3.24). But it might still work as it has for you, and EasyEffects Flatpak is still programmed to launch even after giving that warning. But I have no idea how well it will work, especially given how rapidly PipeWire gets new features and updates that EasyEffects will inevitably end up using, one way or another.

I think it is preferable to over-warn instead of potentially leaving users with a completly unusable app. I am open to improve it though, perhaps I should make it clearer it might work, but at the same time I don't want to give confusing instructions. Any ideas? Here's the current error for reference

Going a bit tangential, there's this issue that I was hoping newer pipewire would solve, but it didn't. Due to this, after waking the system up from hibernate, each time, I have to restart pipewire via systemctl and kill & restart easyeffects (finding process ID, kill, etc.)

So do you want the app to be auto launched after hibernate? This might be improved in Flatpak by #535 but I can't say for sure. Perhaps could you open a new issue for hibernate specifically though? Maybe there's something in PipeWire that can be improved too.

A native build for Debian/Ubuntu will be handy to deal with this pipewire-fails-on-hibernate issue.

Indeed that might be the case, follow https://github.com/wwmm/easyeffects/issues/1000 for news of a Debian/Ubuntu package.

v1gnesh commented 3 years ago

Here's the current error for reference

Yeah, I saw this and interpreted it as, "I need to upgrade pipewire".

So do you want the app to be auto launched after hibernate?

An option to bounce it or perhaps with one or two new systemd services, stop it when sleep/hibernate is encountered, and start it when the system is resumed.

I can hijack the same new systemd service(s) to stop/start pipewire* services, as that's where the problem really is. EasyEffects is falling over because pipewire doesn't work after waking from hibernate.

Thanks, will keep an eye on #1000.

EDIT: Some general related news - https://www.jelmer.uk/fresh-builds.html

wwmm commented 3 years ago

I have lost count of how many issues related to suspending or hibernation have been reported to me. For reasons I can not understand since Pulseaudio they simply do not play nice with us in some computers. I thought things would be different on PipeWire but so far no luck. Even after not using GStreamer to handle the pipeline. And to make things worse no problems happen on my desktop when I tell GNOME to suspend it. Audio just works as expected. So I imagine that some kind of hardware dependency is involved in this situation. But what whole it would be playing I have no idea...

vchernin commented 3 years ago

Are there similar audio apps users could try to reproduce suspend/hibernate issues?

wwmm commented 3 years ago

Are there similar audio apps users could try to reproduce suspend/hibernate issues?

I don't know. The problem is that we have to do things that audio players do not have to do. Like loading virtual devices, setting a chain of filters for effects, redirecting streams to our virtual device, etc. I think that people using Carla or Ardour on PipeWire to set custom audio effects will be the closest ones to what we do. But even in this case there workflow is very different.

What I really do not get is why isn't everyone affected. I could not see this issue even for a single time on my desktop. But the same machine also does not reproduce other issues a fair amount of people faced when we used GStreamer... Sometimes audio problems depend on the hardware being used in ways that are really hard to understand...

vchernin commented 3 years ago

@v1gnesh what do you think of this tweaked warning message? image

v1gnesh commented 3 years ago

Missed saying this the first time... thank you @wwmm, @vchernin, and all the others who have worked on this project and contributed in one way or another. It's a breath of fresh air to see useful software & utilities come to life right in front of one's eyes, on GitHub, GitLab, etc

About the warning message: Thank you, looks good. Once Ubuntu 21.10 releases in a couple of months, with GNOME 40, I'm hoping it'll be easier to figure out building EasyEffects 6.x.x for it.

BTW, audio (pipewire & easyeffects) is working fine for me now. When I reported the problem (pipewire falling over & easyeffects going unresponsive due to that) even after upgrading to 0.3.31, I hadn't done a clean reboot.

So... system up --> upgrade pipewire --> restart pipewire & easyeffect services --> hibernate --> resume --> didn't work but then reboot --> normal load of pipewire & easyeffects --> hibernate --> resume --> works now, twice. Both with short-term hibernate (couple of hours) & long-term hibernate (overnight, turning off power after hibernate).

Mentioning these 2 cases because in the pipewire repo, there was an issue where it was mentioned that the problem was only seen with long-term/overnight hibernate.

vchernin commented 3 years ago

Mentioning these 2 cases because in the pipewire repo, there was an issue where it was mentioned that the problem was only seen with long-term/overnight hibernate.

https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/902 right? I'd suggest commenting there and adding your experience, it is useful to know that hibernate works for someone. Even if you're using different applications.

wwmm commented 3 years ago

system up --> upgrade pipewire --> restart pipewire & easyeffect services --> hibernate --> resume --> didn't work but then reboot --> normal load of pipewire & easyeffects --> hibernate --> resume --> works now, twice.

As I reboot my computer a few times along the day sometimes I forget that not everybody does the same. I have to remember to ask the users about this when unusual problems happen. I do not know why but not rebooting the computer after upgrading Pulseaudio or PipeWire usually causes problems.

vchernin commented 3 years ago

I do not know why but not rebooting the computer after upgrading Pulseaudio or PipeWire usually causes problems.

Indeed these issues can occur sometimes, which is why I personally like Fedora Silverblue, to actually apply updates like for PipeWire (or any system package) a reboot is necessary (but quick).

vchernin commented 2 years ago

The current situation:

Since this change for pipewire 0.3.41 this has gotten better upstream, EE will receive ENOSYS when calling newer methods. Admittely I am not sure if pipewire documents which methods are newer and therefore should be checked for ENOSYS.

The question is what to do if pipewire < 0.3.41.

Ideally we could just ignore this case, however there will be distros shipping without recent pipewire for years like Debian 11 (0.3.19), or perhaps no pipewire at all. But if easyeffects application startup is tied to pipewire on the host working, this isn't fixable without a wrapper script (which flatpak has currently). How feasible is it to allow easyeffects to launch and display a warning/empty app perhaps if pipewire < 0.3.41 or is nonexistent? (as just a simple check, no timeouts etc). See above.

wwmm commented 2 years ago

How feasible is it to allow easyeffects to launch and display a warning/empty app perhaps if pipewire < 0.3.41 or is nonexistent?

I am not sure. We could try to create a different window https://github.com/wwmm/easyeffects/blob/bdc9b84cf3c4d2015a86e50993813ff32ffe0975/src/application.cpp#L343 in case PipeWire has failed. But I think that the hardest part is making sure we won't immediately crash if PipeWire is broken. It really depends on what is not working.