search

hydrogen-music / hydrogen

The advanced drum machine for Linux, macOS, and Windows
http://www.hydrogen-music.org
GNU General Public License v2.0
1.03k stars 172 forks source link

JACK client libraries should not be bundled with Hydrogen binaries #1287

Open cme opened 3 years ago

cme commented 3 years ago

Hydrogen version * : All current Operating system + version : macOS, Windows Audio driver + version : JACK

The binary distributions of Hydrogen for macOS and Windows bundle their own version of the JACK dylib/DLLs. This, it turns out, is Wrong™.

JACK uses data structures in shared memory between the client and the server processes that are not defined or versioned in any way. The formats of these structures are baked into libjack and libjackserver, so the behaviour is entirely unpredictable if the libjack bundled with Hydrogen is from a different version of JACK from the version of libjack. It might work. It might not. The interface between server and client processes is not well-defined, only the binary interface between client application and libjack.

The only supported way to use JACK is to use dynamic linking and allow the system-wide installed JACK libraries to be picked up at run-time.

I haven't actually seen this mentioned anywhere in the JACK documentation itself, but it seems to be 'common knowledge', which I found while working on jackaudio/jack2#749 . It is mentioned in the documentation for JACK python bindings, though.

This creates a problem for building binaries that use JACK because if we link against stubs for libjack and the system doesn't have JACK installed, dynamic linking will fail and Hydrogen won't start. So, we must either:

  1. provide a stub libjack and somehow manipulate linking order so that it is picked up after the system-wide libjack (I don't know if this is possible in macOS bundles or Windows EXEs, but it's certainly possible in theory), or
  2. use a stub library such as weakjack that dynamically finds and opens the system libjack

I'm honestly quite surprised that this isn't part of JACK itself.

On the plus side, this removed the need to rebuild JACK in the macOS build process. Silver linings.

mauser commented 3 years ago

Yay. Nice to see that this common knowledge trickles down to us after building jack-enabled binaries for the last 11 years :) But sarcasm put aside, thanks a lot Colin for solving this problem!

Option (2) sounds good for me, at least i don't see any downside (apart from relying on the correctness of the external code and having no audio when the audio driver is set erronously to jack). Weakjack seems to be easy to integrate, so it should be worth a try.

theGreatWhiteShark commented 3 years ago

Puh. Quite odd that there are no references to this behavior in the JACK docs. This plus the fact that the jack-devel mailing list went offline almost two years ago does not look very promising with respect to logn term support.

I like option 2 too. But I would also suggest we change the our code a bit to adopt this change. In Hydrogen::createDriver() the "Auto" option allows for trying different drivers. But according to the README of weakjack

The first call to jack_client_open() will try to find and load libjack, if it cannot be found, it will fail (return NULL and set jack_status_t if provided to JackFailure.)

the absence of JACK will only get noticed afterwards when calling the init() method of the audio driver. With a restructing of createDriver we could catch this one too and make the function try a different audio driver instead. Also we should add a proper error message + popup for macOS and Windows (not "could not load audio driver" but "No JACK installation found. Please install the JACK audio driver on your system first.")

mauser commented 3 years ago

I like option 2 too. But I would also suggest we change the our code a bit to adopt this change. In Hydrogen::createDriver() the "Auto" option allows for trying different drivers.

Agreed!

I can try to build a prototype with weakjack and see if it works on my Windows system. If it works, we can think about bringing this in for 1.1.

cme commented 3 years ago

Strongly agree (with both).

I've submitted a PR for weakjack to add macOS paths for MacPorts and for the new HomeBrew prefix. It might be worthwhile to fork and submodule weakjack under hydrogen-music as we might want to also add the ability to set where the JACK library is located via the GUI (in case of multiple installations).

There's another annoying consequence of this. Currently, an Intel Hydrogen+libjack will be able to work just fine with a native AArch64 JACK server running on Apple Silicon. Using weakjack will break this, at least on systems using HoweBrew's native JACK because opening an AArch64 dylib from an Intel process won't work.

cme commented 3 years ago

Did you have a chance to try this @mauser? I was thinking of giving it a go this week if not.

mauser commented 3 years ago

@cme : Please do so, Colin! I've started experimenting with it, but never found the time to finish it.

cme commented 3 years ago

An annoying little snag. Having got builds set up to use WeakJack now, finding the local jack install by weakjack no problem etc...

...I find that libjack is still being pulled in as a dependency, because PortAudio is linked against it.

It's not clear what the right thing to do with respect to PortAudio's use of JACK is. Ideally, PortAudio ought to also use WeakJack.

I think for the moment I'm leaning towards:

theGreatWhiteShark commented 3 years ago

I have not yet dealt with PortAudio but this wiki page suggests it's just a wrapper handing all calls to either the JACK or Core Audio API. If that's true, wouldn't this mean it's as flawed as the JACK driver if WeakJack is required?

cme commented 3 years ago

If that's true, wouldn't this mean it's as flawed as the JACK driver if WeakJack is required?

Sort of. Anything that links with libjack has the same problem. I think of this really as more a design flaw in Jack than anything else, for which WeakJack is a workaround. I think I'll try raising this over on the Jack project.

Be-ing commented 2 years ago

I'm honestly quite surprised that this isn't part of JACK itself.

Actually, it is. JackWeakAPI.c in the jack2 repository does this. It is not used by JACK's build system; it's up to applications to build it. It also doesn't seem to work on Windows so I just made a PR upstream for that. vcpkg's jack2 package uses JackWeakAPI, which I've linked with PortAudio to use PortAudio's JACK backend on macOS and Windows with Mixxx (there are still some issues with that though). However, if your application uses the JACK API directly, I suggest disabling PortAudio's JACK backend. That should solve your linking issue.