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

Fixes empty connections bug on pipewire #1748

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ecruz1986
Copy link

@ecruz1986 ecruz1986 commented Feb 26, 2023

Fixes #1625 regarding empty connections that appears while using Carla patchbay with pipewire, among other related issues.

The ignore lists logic on this piece of code was not consistent. There were corner cases that caused inconsistencies with the mechanism of ignoring ports and connection registers. So we revert back to processing every single event in the order they are queued and this fixes the bug.

See my comments on the issue #1625 for more information about why the logic was inconsistent.

This effectively reverts what was done in commit e5f4d02.

I'm not sure why this ignore mechanism was needed in the first place, I lack project history and context. My guess is it was necessary 20 months ago to work around some pipewire bug that existed back then, but it doesn't seem necessary anymore today. I don't see any abnormalities or crashes while using it with today's pipewire 0.3.66

The ignore lists logic on this piece of code was not consistent, so we revert back to processing every single event and this fixes the bug that caused empty connections to show up in the canvas over pipewire which led to crashes.
@SnipeXandrej
Copy link

Sadly this does not seem to fix Carla crashing for me.

@ecruz1986
Copy link
Author

Sadly this does not seem to fix Carla crashing for me.

Sorry... Then unfortunately your issue has a different cause and it's unrelated to this. I'll remove mention to your issue from the description.

@falkTX
Copy link
Owner

falkTX commented Mar 5, 2023

Thanks for the research and finding the culprit!
Looking around the code I am not particularly happy with how it was being done.. then got worse with pipewire in the mix and me trying to workaround a few things..

I have simplified the whole thing in f8b35fd, which includes your idea to not bother keeping 2 list of events, one to ignore etc..

Please give it a try. If it still works fine under pipewire I will backport the fix into a new 2.5.x release.
It should work, but feedback confirming it would be very welcome! (dont have a aystem with PW right now to test myself)

@ecruz1986
Copy link
Author

I have simplified the whole thing in [f8b35fd]
(...)
Please give it a try.

I have been using the latest git master for two days now, it surely does fix the empty connections bug under pipewire, and I haven't noticed any regressions. I haven't reviewed the code, but from a users perspective, it looks good to me! You may close this pull request, and a backport would surely be of use to many users who are suffering with this.

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.

Carla lists and draw "empty" connections
3 participants