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

fix crash on pulseaudio #4

Closed
wants to merge 1 commit into from
Closed

fix crash on pulseaudio #4

wants to merge 1 commit into from

Conversation

Vendicated
Copy link
Member

@Vendicated Vendicated commented Oct 14, 2023

the current implementation segfaults on pulseaudio because it tries to call pipewire methods, which, for obvious reason, are not actually available:

Message: Process 486613 (electron) of user 1000 dumped core.
         
         Stack trace of thread 487173:
         #0  0x00007f4aa36f9404 pw_proxy_get_bound_id (libpipewire-0.3.so.0 + 0x78404)
         #1  0x00007f4aa37a0fc8 n/a (/tmp/misc/Vesktop/static/dist/venmic.node + 0x3cfc8)
         ELF object binary architecture: AMD x86-64

this pr fixes it by first manually dlopening libpipewire to check if it's installed

i don't know if this is the best method to do so, so feel free to close this pr if there's a better method

@Vendicated Vendicated requested a review from Curve October 14, 2023 03:55
@Curve
Copy link
Member

Curve commented Oct 14, 2023

As pipewire-pulse exists, I'd prefer if we first use the pulseaudio api to check the name of the server, if it does not contain "(pipewire)" we throw (using pa_server_info)

That way we also catch users that have pipewire installed but don't use it as their audio server

@Curve
Copy link
Member

Curve commented Oct 14, 2023

Also, will we provide pre-built binaries for the addon or will we just pull it in through npm

If the latter is the case, the user will need pipewire installed otherwise it might not compile (so we might just tell the user to install pipewire - just having it installed won't force them to use it)

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.

2 participants