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

Store the registry as a struct pw_registry instead of a pw_proxy #1625

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

Conversation

antlarr-suse
Copy link

This is a better fix for #1611 and an improvement over #1624 as requested by @swick.

src/pipewire.c Outdated
@@ -224,7 +229,7 @@ pipewire_remote_destroy (PipeWireRemote *remote)
/* This check is a workaround for older PW versions */
if (remote->registry)
spa_hook_remove (&remote->registry_listener);
g_clear_pointer (&remote->registry, pw_proxy_destroy);
g_clear_pointer (&remote->registry, pipewire_registry_destroy);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
g_clear_pointer (&remote->registry, pipewire_registry_destroy);
g_clear_pointer ((struct pw_proxy **)&remote->registry, pw_proxy_destroy);

Copy link
Author

Choose a reason for hiding this comment

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

ouch, I would have said I tried that and it didn't build, before adding the helper function. Thanks for suggesting that change. I made that change and I guess you can squash all commits before merging, right? or do you want me to merge them into one and force push?

Copy link
Contributor

Choose a reason for hiding this comment

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

Cleaning the history and force pushing would be appreciated.

Copy link
Author

Choose a reason for hiding this comment

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

Of course. Done

Copy link
Contributor

Choose a reason for hiding this comment

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

For the future: we also try to prefix the commit message with the area the code touches. In this case pipewire: ....

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't aware of that. Noted.

This is a better fix for flatpak#1611 and an improvement over flatpak#1624
as requested by @swick.
@antlarr-suse antlarr-suse force-pushed the fix-build-pipewire-1.3.82-take2 branch from ec29ea9 to ad138bc Compare February 13, 2025 07:35
@swick
Copy link
Contributor

swick commented Feb 13, 2025

Thanks, this LGTM.

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.

3 participants