Support weak functions.

[wmedrano]

Fixes crashes on Jack on Pipewire.

[Be-ing]

Why are you removing dlib? What problem is this solving? I have not encountered any crashes with Pipewire.

[wmedrano]

Why are you removing dlib? What problem is this solving? I have not encountered any crashes with Pipewire.

When attempting to build with default features.

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: LoadLibraryError("The resquested symbol was missing: jack_get_internal_client_name\u{0}")', examples/livi-jack.rs:36:81
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[Be-ing]

Correct me if I'm wrong, but this seems to remove the choice whether to dlopen or link libjack?

[Be-ing]

Wouldn't a macro be a better tool for the job of autogenerating code?

[wmedrano]

Yes, I didn't use a macro for practical reasons.

1. I'm not good at them.
2. The errors that would've been produced would've likely cost me a lot of time to debug through a macro. I've found macro errors to be very mysterious.

[Be-ing]

I haven't written Rust macros myself (yet), but I have maintained code like this that autogenerated code by printing strings and it was not fun.

[wmedrano]

Ack. I think in terms of maintainability from myself, I prefer Normal > Codegen > Macro.

FWIW: I do this a lot at work (although with arguably better tooling) and its not ideal but not among the worst things we do.

[Be-ing]

I can't reproduce that panic running the livi-jack example with the main branch of livi. I am using Pipewire.

[wmedrano]

Correct me if I'm wrong, but this seems to remove the choice whether to dlopen or link libjack?

Right the PR in a WIP. Was planning to maybe add it back in. However, what is the benefit? From the user perspective, isn't it enough that they added the dep and the example works out the box?

[wmedrano]

I can't reproduce that panic running the livi-jack example with the main branch of livi. I am using Pipewire.

The panic occurs when using the library with default_features = false. IMO: The fact that you have to remember to add default_features = false on linux so that it works with Pipewire is very annoying. If pipewire gets more traction, I'd say this is close to unusable.

[wmedrano]

Correct me if I'm wrong, but this seems to remove the choice whether to dlopen or link libjack?

Also, dlib does not meet the needs. There are lots of versions of Jack out there and failing because a single function wasn't implemented is pretty annoying.

This new method also makes it easier to add stuff that is not supported by all jack libraries to try it out. Like jack_ctl_ stuff.

[Be-ing]

The panic occurs when using the library with default_features = false

I can't reproduce this using the livi-jack example. Can you push the branch you are encountering this panic with and I can try to reproduce it?

Was planning to maybe add it back in. However, what is the benefit? From the user perspective, isn't it enough that they added the dep and the example works out the box?

Packagers prefer linking libraries that are required by the application. dlopen can be appropriate on Linux if the application can function without libjack. Some applications using these bindings may require JACK and some might function without JACK.

There are lots of versions of Jack out there

Are there really? As I explained before, I don't think these bindings need any special handling for ancient versions of libjack from before 2010.

The specific function mentioned in the panic message you posted is implemented in Pipewire.

[wmedrano]

I can't reproduce that panic running the livi-jack example with the main branch of livi. I am using Pipewire.

Here is the branch I was working on: wmedrano/livi-rs#22
FYI: Running on Ubuntu 21.10. I actually hadn't made any effort to properly configure pipewire, but it seems to be my daily driver.

This is what I meant for different versions of JACK. In my experience with helping others, something about the setups are different and bugs are encountered. I would prefer consistency of this library, rather than trying to debug individual setups.

[wmedrano]

Maybe you're right in theory, but it doesn't work on my fairly standard system. By versions, I don't just mean jack2 and pw-jack, also the versions within them. Seems like things on working on your pw system, but not mine.

[Be-ing]

I actually hadn't made any effort to properly configure pipewire, but it seems to be my daily driver.

Are you sure it's trying to load Pipewire's libjack? How have you confirmed this? Something isn't adding up here. Why would it fail with the dlopen feature but work without it?

This seems like a complicated solution without a clear picture of what the root problem is.

[Be-ing]

I can't think of a reason that dlopen wouldn't work when linking works unless dlopen is trying to open a different library than what is being linked. Is there a mismatch between the library that pkg-config is pointing to for linking and what dlopen is opening? Using Pipewire's JACK library requires configuring the dynamic linker to do so. Debian does not do this by default.

[wmedrano]

This is my Ubuntu system.

wmedrano@donuts:~$ pactl info
Server String: /run/user/1000/pulse/native
Library Protocol Version: 35
Server Protocol Version: 35
Is Local: yes
Client Index: 43
Tile Size: 65472
User Name: <redacted>
Host Name: <redacted>
Server Name: PulseAudio (on PipeWire 0.3.32)
Server Version: 14.0.0
Default Sample Specification: float32le 2ch 48000Hz
Default Channel Map: front-left,front-right
Default Sink: alsa_output.usb-Audioengine_Audioengine_D1-00.analog-stereo
Default Source: alsa_input.usb-046d_Logitech_BRIO_1238547A-03.analog-stereo
Cookie: <redacted>

[wmedrano]

I can't think of a reason that dlopen wouldn't work when linking works unless dlopen is trying to open a different library than what is being linked.

Also, if you mean static and dynamic should work the same, this is not true. dlopen fails entirely if 1 function is missing. If you statically link and at runtime it is discovered that the dynamic library doesn't have the symbol, it actually fails at runtime with symbol not found error. The main difference here is that dlib fails at startup causing none of the dynamic library to be available. Where as in the static linked, the library should be fine as long as you don't reach for that unimplemented feature.

Currently in progress: Will try calling the original function that was failing from livi-jack compiled with statically linked jack.

[wmedrano]

wmedrano/livi-rs#23

wmedrano@donuts:~/code/livi-rs$ cargo build --release
    Finished release [optimized] target(s) in 0.01s
wmedrano@donuts:~/code/livi-rs$ pw-jack target/release/examples/livi-jack
target/release/examples/livi-jack: symbol lookup error: target/release/examples/livi-jack: undefined symbol: jack_get_internal_client_name

[Be-ing]

This is my Ubuntu system.

pactl isn't relevant to what libjack you're using.

[wmedrano]

I actually hadn't made any effort to properly configure pipewire, but it seems to be my daily driver.

Are you sure it's trying to load Pipewire's libjack? How have you confirmed this? Something isn't adding up here. Why would it fail with the dlopen feature but work without it?

This seems like a complicated solution without a clear picture of what the root problem is.

Ack, this is complicated, but I'm pretty confident this is more robust. That is to say, using some dynamic loading library that allows for having some missing function implementations. This solution is more scalable than debugging everyone's setups.

I'm gonna go ahead and check this in and release as is. If you feel strongly that this should not go in, then I would consider giving up ownership of rust-jack to you.

[Be-ing]

Also, if you mean static and dynamic should work the same, this is not true. dlopen fails entirely if 1 function is missing. If you statically link and at runtime it is discovered that the dynamic library doesn't have the symbol, it actually fails at runtime with symbol not found error. The main difference here is that dlib fails at startup causing none of the dynamic library to be available. Where as in the static linked, the library should be fine as long as you don't reach for that unimplemented feature.

libjack is always shipped as a dynamic library (otherwise every application would run into the issue of using a different libjack than the server as described in the documentation removed in this PR). But yeah when linking a missing symbol isn't an issue if it isn't used by the application.

jack_get_internal_client_name was added to Pipewire in 0.3.41 which was released 13 December 2021. Ubuntu 21.10 still ships the outdated Pipewire 0.3.32. Ubuntu 22.04, which will be released in 11 days, will ship Pipewire 0.3.48. I do not think it is a good idea to merge a complicated workaround to a problem that will fix itself in 11 days.

[Be-ing]

I'm gonna go ahead and check this in and release as is. If you feel strongly that this should not go in, then I would consider giving up ownership of rust-jack to you.

I will not upgrade my application to a version that does not allow for regular linking and would consider maintaining a fork if you did that.

[Be-ing]

An alternative solution could be putting the internal client functions behind a feature flag like the metadata functions. I think it would be fine to have that feature off by default considering it is rarely used. Practically speaking the only Rust application I know of using it is Jackctl by @IGBC who contributed the bindings.

[Be-ing]

The panic occurs when using the library with default_features = false. IMO: The fact that you have to remember to add default_features = false on linux so that it works with Pipewire is very annoying. If pipewire gets more traction, I'd say this is close to unusable.

I'm confused. The dlopen feature is enabled by default, so default-features = false would link at build time. If you're trying to dlopen an outdated libjack from Pipewire 0.3.32, shouldn't the panic happen without default-features = false using the dlopen feature? The documentation which has been removed in this PR specifically advises against using the dlopen feature on Linux. If you want to change the defaults so dlopen is not used by default because Cargo doesn't let libraries specify platform-specific defaults, that would be another alternative solution which I think would be better than adding all this complexity.

[wmedrano]

Also, if you mean static and dynamic should work the same, this is not true. dlopen fails entirely if 1 function is missing. If you statically link and at runtime it is discovered that the dynamic library doesn't have the symbol, it actually fails at runtime with symbol not found error. The main difference here is that dlib fails at startup causing none of the dynamic library to be available. Where as in the static linked, the library should be fine as long as you don't reach for that unimplemented feature.

libjack is always shipped as a dynamic library (otherwise every application would run into the issue of using a different libjack than the server as described in the documentation removed in this PR). But yeah when linking a missing symbol isn't an issue if it isn't used by the application.

jack_get_internal_client_name was added to Pipewire in 0.3.41 which was released 13 December 2021. Ubuntu 21.10 still ships the outdated Pipewire 0.3.32. Ubuntu 22.04, which will be released in 11 days, will ship Pipewire 0.3.48. I do not think it is a good idea to merge a complicated workaround to a problem that will fix itself in 11 days.

This assumes that everyone will upgrade their computer. The problem isn't this specific case IMO. I created this library over 6 years ago and issues like this are par for the course. Overall, having to look into jack implementations to debug is a waste of time, especially for users that don't care about JACK. Ideally they just copy paste an example and it works perfectly right out the box. I'll probably add static bindings back in a month or 2. If this is not an acceptable solution, I'll look into transferring ownership.

[Be-ing]

Wouldn't linking rather than dlopen by default solve this?

Ideally they just copy paste an example and it works perfectly right out the box.

An example is currently provided in the documentation, though you could make an argument that simply adding jack = "verison" in Cargo.toml is better. But then you'd need a special case for Windows and macOS to use the dlopen feature. Practically lots of JACK applications only run on Linux, so maybe that would be better. I think that would be a better outcome than maintaining a code generator written specifically for this library.

[wmedrano]

Wouldn't linking rather than dlopen by default solve this?

Ideally they just copy paste an example and it works perfectly right out the box.

An example is currently provided in the documentation, though you could make an argument that simply adding jack = "verison" in Cargo.toml is better. But then you'd need a special case for Windows and macOS to use the dlopen feature. Practically lots of JACK applications only run on Linux, so maybe that would be better. I think that would be a better outcome than maintaining a code generator written specifically for this library.

Ack, code generator should be replaced by a macro. I'm just going to go ahead and release this. I'm open to transferring ownership though.

[Be-ing]

I'm open to transferring ownership though.

I would like to continue maintaining this together. I think simply changing the default features is an easier and more maintainable solution than a complete rewrite using a code generator written exclusively for this library, especially one that removes an important feature.

[Be-ing]

I'm confused what your intentions are here. If you're tired of maintaining this, I'd be happy to take maintainership. I am also intending to use livi in my application the future, so I would like to continue working with you.

[Be-ing]

jack_get_internal_client_name is not marked as a weak function in the C header.

[Be-ing]

If you really think the functions marked as weak in the JACK C headers should be handled specially, then another alternative solution could be adding the capability to handle these to dlib. But jack_get_internal_client_name isn't marked as a weak function so I think that's tangential to the error you encountered.

[wmedrano]

Weak is probably the wrong term, maybe something more like optional. Sorry about any disagreements, I hope to address them sometime later, some key takeaways:

1. To me, failing on Linux without specifying feature flags is a huge failure of the current library and I treated it with some sort of urgency. I think in your opinion, I have some niche setup. I disagree. And either way, niche setups are common in the Linux niche.
2. Due to strongly believing in point 1, I did push this out somewhat in a hurry. The options I saw for myself are to push it out in a hurry because I believe in point 1, sit on the current issues which reflect poorly on my library and address them when there is time, spend more time working on this at the expense of my mental health and life.
3. The ability to link the library was removed and I may be undervaluing this functionality. The justification I saw was that package maintainers recommend this, but it didn't click with me as there are many cases where applications want to ship JACK as optional anyways.
4. There is some concern that I have a half baked codegen, fair enough. However, I would note that the exposed interface is practically identical to jack_sys before dlopen was rolled out. This is a pretty successful migration IMO and something done the right way can replace what I did but provide the correct approach.

Something else to note, I do this in my own free time (though thankfully I have a $5 monthly sponsorship). My day job is in no way similar to this. What this means is that I'm driven by the joy of writing software and the pride in producing what I believe to be quality (given the constraints). Not addressing point 1 when I feel its easy is really demotivating.

[Be-ing]

I think in your opinion, I have some niche setup. I disagree.

Yes, I think you do. Ubuntu does not recommend Pipewire for audio yet. Technically the package exists but it still requires manual setup to use. I personally haven't heard of anyone else using Pipewire for audio on Ubuntu or Debian using the distro packages (besides myself when I briefly ran Mobian on my Pinephone Pro before I got Fedora on it); I think people who are using Pipewire for audio on these distros are generally using this PPA which keeps up to date with upstream Pipewire.

I say this not to dismiss the issue you ran into, but I think you overestimated its impact because you ran into it on your own machine. Recall that nobody said anything about these bindings not working with Pipewire until I did in #142 and fixed it months later in #154. It's good to take pride in your work, but you don't need to let that stress you out. If you want help maintaining this, I'd be happy to help. I doubt anyone would think it would reflect poorly on you if you didn't immediately have a solution to the problem you encountered, especially considering nobody else has reported it. By contrast, as a developer also depending on this library, it would have looked better to me to fully investigate the source of the problem and take the time to consider the pros and cons of various solutions before rushing a +2,794 −2,031 major rewrite of jack-sys that removes an important feature.

I'm still confused about the conditions you encountered the error. The error looks like it happened at runtime when you used the dlopen feature, which was enabled by default, but you said it occurred with default-features = false. If it only occurs with the dlopen feature, a simple solution would be disabling that by default. On further thought, I think this may be a good idea in general regardless of the issue with jack_get_internal_client_name. JACK is most commonly used on Linux, and it's helpful for Linux packagers to link at build time, so I think it would be good to have the default work well for Linux packaging. Applications that want to use these bindings on Windows and macOS could enable the dlopen feature for those operating systems.

I would note that the exposed interface is practically identical to jack_sys before dlopen was rolled out. This is a pretty successful migration IMO

Yes, I am glad to have the safe Rust abstractions that don't leak implementation details of jack-sys. 😄

[Be-ing]

I am resuming working on my project which uses this library and ran into this issue again when my collaborator bumped the jack crate to 0.10.0 without knowing about this issue. I hope we can reach a consensus on a long term solution.

I do not think writing macros just for this crate as proposed in #174 is a good idea because that would pretty much be reimplementing dlib. Ubuntu 21.10 has been outdated by 22.04 for several months now, so I don't think it's worthwhile to go out of the way to work around an upstream bug that has already been fixed. I think the best way forward would be to revert this pull request. Unfortunately that's not trivially simple anymore since you merged #175 after this. I could do the work of reimplementing #175 after reverting to before this PR.