-
-
Notifications
You must be signed in to change notification settings - Fork 279
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
Fedora 35 issues #1144
Comments
So Fedora has wireplumber. If someone wants to test, follow instructions here. |
@vchernin can you try to temporarily disable wireplumber and enable the original pipewire-media-session? |
The app switch is the only thing we do through the session manager. So I agree it is suspicious to have problems after moving to wireplumber. |
I can confirm switching to pipewire-media-session with |
Well... I imagine that if they are switching to wireplumber is because it is supposed to be a drop-in replacement to the built-in session manager. If that is not happening I think the issue is on the wireplumber side. |
Unless that when wireplumber is running the metadata has a name different than easyeffects/src/pipe_manager.cpp Line 968 in e293cf5
Does the line with the name "default" changes to something else? Our interaction with the session manager is essentially some calls to |
Anyway wireplumber is available on Arch also. If it's not working with it, we have to specify pipewire-media-session as explicit dependency. |
Also I can confirm the issue on Arch, after installing wireplumber and and enabling it per this. |
Curiosity got the best of me and I decided to investigate. It turns out the problem is not where I thought it would be. Removing this line makes things work with WirePlumber easyeffects/src/effects_base_ui.cpp Line 672 in e293cf5
application_info_update is the callback of the signal emitted by holder->info_updated.emit(); and inside the callback we call pointer_connection_enable->block(); some kind of thread locking may be happening. But I wonder why only when WirePlumber is used... Strange...
|
Try also adding and removing from the blocklist. Anyway that call was useful to update the app info UI on switch and blocklist changes. Sometimes it was not updating. I don't think it's an issue related to pointer_connection_enable because we change the switch only between the connection block/unblock. easyeffects/src/effects_base_ui.cpp Line 739 in e293cf5
|
I'm making some tests commenting the Besides, if the app is not enabled from the start (because of Process all Output not enabled), when you add and remove it from the blocklist sometimes the app_info_ui is empty. |
Oh! I see. But maybe there is a better way to achieve the same results. I am not sure we should be calling the connection block function inside the slot that is being handled by the connection we want to block. I did some tests with Pavucontrol. If we enable and disable the application there the switch changes in our window with no crash. It is only when we change its state by clicking on our switch that the crash happens. |
I did more tests. The real source of the problem is calling easyeffects/src/effects_base_ui.cpp Line 739 in e293cf5
pointer_connection_enable->block(); is not avoiding it because the call happened inside the state changed slot.
|
The switch changes because pipewire triggers some change and I'm also testing and find out a bad bug. If show blocklisted apps is untoggled, the app info ui might not work in some situations because the item is not updated. I forgot to update the items of Now I fixed this, but can you suggest me how to update the enable switch on the checkbutton toggle? For the connection block, all these block/unblock calls are inside |
I do not think we can. We have to do these operations inside the |
I've found a way to "fool" gtk XD. Replace the emit call that is inside the switch by
This way the emit is not done inside the switch callback. It will be scheduled to run the next time gtk's main loop is idle. |
@Digitalone1 as you are already preparing some changes it may be more productive to add this in your pull request |
I forgot to say that this fixed the problem with wireplumber. At least on my computer. |
If the problem is the We need it when an application is added/removed from the blocklist to update the sensitive property of the enable switch. Another issue is that |
Almost that. This signal can be emitted there. The problem was that its slot called |
I agree that we do not need all these calls. But I am not sure about the race conditions. The signals are scheduled by |
Another point is that in service mode almost nothing is done when |
I added a
Look at the log
More than 40 calls in less than 100 ms. With this flag we only did 4 updates. I think this is a good approach. What do you think? Any downside I might miss? |
So far I did not see one. In PulseEffects the stream info was used to decide if our pipeline should be active or not. But in PipeWire we have the link info for that. So in theory reducing the actions taken when we receive node info updates should not cause problems. |
I just installed the latest Easyeffects Flatpak 6.1.1 on Fedora 35 Silverblue (19th Sept build). I can confirm that Firefox output is not processed and enabling the button results in a segfault.
|
The changes in the last merge request should fix this issue. |
@lakotamm the segmentation fault you see has another source that has yet to be found. The crash related to wireplumber that is being discussed here does not happen in functions related to |
Thanks for your reply. I will give it another try in the evening. However, the fix made by @Digitalone1 seems to resolve also this issue. Should I do the test again with the official version 6.1.1 and report the issue? |
This debug output is given when
|
That is probably the moment when effects are finally turned on. Humm... Wait... What is the full log before running WirePlumber or any other workaround to fix the problem? What is EasyEffects seeing as default output device? |
Full log before entering wireplumber command
After wireplumber command (should be same as above comment)
|
It is a little unusual to have two lines with |
Manually enabling the switch for Firefox has never helped. |
I wonder if we are receiving the wrong metadata object. This would explain effects not working even after enabling the switch. At this moment we rely on the metadata name being equal to easyeffects/src/pipe_manager.cpp Line 1008 in 5a99b71
But if this is really the problem starting wireplumber again should not help because we should still ignore the extra notification about the metadata. Strange... |
Actually that is only going to happen if the calls to easyeffects/src/pipe_manager.cpp Line 1009 in 5a99b71
|
Is there already a debug option to print contents of the metadata object EasyEffects receives or would that need a quick patch? |
With luck I will have time to add more debug messages today. |
@vchernin master branch updated with a warning that should be printed if the metadata object returned by |
Initial startup
run `wireplumber`
|
So the metadata object is not null... Humm... Why the hell the switch is not moving apps to our sink... All it does is asking the session manager to do the move. Is Pavucontrol able to move apps between sinks with no problems? |
Could you provide steps to do this? I am trying different options in Pavucontrol but I am not sure what moves apps between sinks. Is the video below correct? Kooha-09-29-2021-16-47-08.mp4I tried (what I thought would) move the app between the sinks.. but clicking the option didn't do anything... |
You did the right procedure. Is pavucontrol also running in a flatpak container? In any case something seems wrong even after starting wireplumber manually. When the app is moved away from our virtual sink the switch in EasyEffects should go to off and as Firefox is the only app shown in our players tab there should be no spectrum activity after that. Things are more broken than expected... By the way why is even possible to run wireplumber manually? Isn't its service already running? I do not think it is supposed to do something when another instance is already loaded. It is weird it acts like nothing it there. |
That when the move to another sink is actually done. What obviously did not happen. If Pavucontrol is not being able to do that the odds that we will be able to fix this problem on EasyEffects are very small. We may need to contact WirePlumber developers. |
Yes Pavucontrol is in Flatpak as well. I'll try the RPM. If nothing is different then yeah it sounds like something relating to WirePlumber itself. The WirePlumber service is definitely already running, since I can hear audio just fine. It's "just" EasyEffects and Pavucontrol which seem to be behaving strangely so far. |
Running the same test as above except with Pavucontrol RPM behaves as hoped. I am able to change Firefox's audio output to the EasyEffects sink (if that is the correct term). Then everything works, similar to running So it seems like WirePlumber has an issue with moving Flatpak apps to another sink? |
Yup, back with pipewire-media-session + Flatpak EE and Pavucontrol Kooha-09-29-2021-20-59-27.mp4WirePlumber + Flatpak EE and Pavucontrol Kooha-09-29-2021-21-09-47.mp4Since this behaviour wasn't found on the RPM versions, there is clearly a wireplumber/flatpak issue with changing sinks (if that is the right term). Who's fault it is I don't know, but I really doubt it's easyeffects' or pavucontrol since it works correctly with non flatpak builds. |
Oh, this would explain the issues I have trying to use EasyEffects on Fedora 35 and also Spotify (which now needs a re-launch when audio output changes). I couldn't find an issue for this in WirePlumber, so I opened one @ https://gitlab.freedesktop.org/pipewire/wireplumber/-/issues/59 |
Now I remember that when I ported PulseEffects to PipeWire we also had problems moving apps in the flatpak install https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/667. At the time Taymans did some changes to how flatpak permissions were handled. On our side we had to set our media category tag to
I wonder if WirePlumber is making use of this. Probably not. I will add this information on the issue opened by @garrett |
Just testing here on my F35 system with Easyeffects installed as an RPM and using the latest Wireplumber test build (https://bodhi.fedoraproject.org/updates/FEDORA-2021-b70755fdc3). Easyeffects seems to work perfectly fine for me. |
@cschalle thanks for testing, indeed there were issues but by now they were solved for the RPM version. The last issue being discussed here was specific to Flatpak, and that's been fixed with the latest WirePlumber release. So EasyEffects should be mostly ready for Fedora 35/WirePlumber. To update the recap of what was discussed here:
I did see @Digitalone1 mentioned:
Is that still an issue? |
It should be fixed too https://gitlab.freedesktop.org/pipewire/wireplumber/-/issues/57 |
I did not try to install wireplumber's master branch to test and see if ti is really the case. But it seems it was an easy fix to do on their side. |
Ah right, thanks. I guess this issue can be closed then? WirePlumber 0.4.3 should make it to Arch's repos soon hopefully. Oh, it already made it! |
I installed it now and our virtual source is working with wireplumber. The previous issues seem to be gone. |
With the new version of wireplumber, everything is working on Silverblue 35 + flatpak easyeffects! Thank you very much guys! |
I had trouble with Fedora 35, I belive this may be related to this: https://fedoraproject.org/wiki/Changes/WirePlumber
It's the only significant change I know has happened to PipeWire in Fedora 35.
All the below is for 35 but not 34:
On this build the enabling switch in the output section is just grayed out :
This picture is on stable (6.1.0 tag):
This is on this PR's build:
Enabling the switch on this build reliably causes easyeffects to crash within a second.
The only log from doing so is the joyful:
Segmentation fault (core dumped)
I think that issue is likely a regression from some other change in master.
Also I found on Fedora 35 I don't think EasyEffects Flatpak even stable is working at all... I think this is not an EasyEffects problem though.
I am curious what happens with the COPR build on 35.
Originally discussed: #1141 (comment)
The text was updated successfully, but these errors were encountered: