Skip to content

Add Windows JACK support - #5716

Merged
tresf merged 9 commits into
LMMS:masterfrom
tresf:jack-win
Oct 30, 2020
Merged

Add Windows JACK support#5716
tresf merged 9 commits into
LMMS:masterfrom
tresf:jack-win

Conversation

@tresf

@tresf tresf commented Oct 16, 2020

Copy link
Copy Markdown
Member

JACK is surprisingly plug-and-play with regards to LMMS on Windows, thanks to the hard work of @falkTX and the rest of the JACK2 team combined with our already existing weakjack (thanks @x42) module (preventing the need for windows dev libs).

  • Adds JACK build support for Windows systems
  • Removes libjack-dev dependency unless specified
  • Adds jack2 as a submodule, related Submodule candidates #3963
  • Bumps weakjack submodule to add MSVC support.

To test:

  • Install JACK for Windows from the website
  • Launch JACK with ASIO support per JACK's website instructions
  • Switch LMMS backend to JACK
  • Configure using qjackctl, found in JACK install directory

image

Closes #2136

@LmmsBot

LmmsBot commented Oct 16, 2020

Copy link
Copy Markdown
🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Windows

Linux

macOS

🤖
{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://10066-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-752%2Bg52f780b5b-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10066?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://10067-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-752%2Bg52f780b5b-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10067?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/5l409d92f1rnlf1l/artifacts/build/lmms-1.2.2-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/36057155"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/33armqsq0i5hnai1/artifacts/build/lmms-1.2.2-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/36057155"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://10069-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-752%2Bg52f780b-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10069?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://10065-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-752%2Bg52f780b5b-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10065?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "00c98a2a35b79549f117ce4b5bd91af842dcfdc2"}

@PhysSong

Copy link
Copy Markdown
Member

The weakjack submodule should be updated to support MSVC, see x42/weakjack#9.

Comment thread src/3rdparty/weakjack/CMakeLists.txt Outdated
@tresf tresf mentioned this pull request Oct 20, 2020
Comment thread .gitmodules Outdated
@tresf

tresf commented Oct 20, 2020

Copy link
Copy Markdown
Member Author

The weakjack submodule should be updated to support MSVC, see x42/weakjack#9.

Done via b1ab5ae.

@tresf

tresf commented Oct 20, 2020

Copy link
Copy Markdown
Member Author

All notes have been addressed, but there's a lingering issue from #5636 preventing AppVeyor to succeed. I'm fine waiting for that to be fixed to make sure AppVeyor is happy or merging as-is. Thoughts welcome. Pinging @Spekular since he authored #5636.

@Spekular

Copy link
Copy Markdown
Member

Huh. Is this related to the issue fixed by #5678 somehow? Other builds since then have been fine, so I'm frankly not sure why an issue would pop up now.

@tresf

tresf commented Oct 20, 2020

Copy link
Copy Markdown
Member Author

Huh. Is this related to the issue fixed by #5678 somehow? Other builds since then have been fine, so I'm frankly not sure why an issue would pop up now.

Ok, sorry for the ping, I did search beforehand but must have missed it. I'll fast-forward and it should fix it, thanks.

Edit: Done via 7927bdd, just awaiting CI. 🤞

@tresf

tresf commented Oct 20, 2020

Copy link
Copy Markdown
Member Author

Ok, I fast-forwarded my branch didn't fix this @Spekular, can you comment on the AppVeyor error? It's the same as before.

@PhysSong

Copy link
Copy Markdown
Member

I think (std::numeric_limits<int>::max)() may work. I'll test it out and fix lmms_basic.h as well if that works.

@tresf

tresf commented Oct 27, 2020

Copy link
Copy Markdown
Member Author

I'm frankly not sure why an issue would pop up now.

@Spekular as discussed in #devtalk on Discord, it appears this is as a result of adding jack.h into AppVeyor, which is new with this PR. So, although the symptom is the same, the cause is likley a 3rdparty header we don't manage. I've made my first attempt at patching this in d56db57. If that doesn't work, I might fire up a MSVC environment instead to avoid the guesswork.

I think (std::numeric_limits<int>::max)() may work.

@DomClark shared his sentiments on Discord, and I tend to agree with them that we shouldn't have to worry about adding parens to each call to max.

I'll test it out and fix lmms_basic.h as well if that works.

I started writing this on a separate branch, but was largely unsuccessful. I'll revisit if needed.

Comment thread plugins/vst_base/RemoteVstPlugin.cpp
@tresf

tresf commented Oct 29, 2020

Copy link
Copy Markdown
Member Author

I'll test it out and fix lmms_basic.h as well if that works.

I decided to opt for a top-level...

ADD_DEFINITIONS(-DNOMINMAX)

... which I believe solves the problem proactively and holistically.

On the contrary, lmms_basics.h can't extend to headers which we don't manage, like weak_jack.h, so I believe this ADD_DEFINITIONS approach to be ideal.

Components which build using ExternalProject_Add may not import all flags, so RemoteVstPlugin is left as-is with its existing NOMINMAX flag.

@tresf

tresf commented Oct 30, 2020

Copy link
Copy Markdown
Member Author

Something's odd with Travis, it's not even queuing my builds (it's been hours and I've tried restarting it once). I'm not holding up the PR for that specifically. There's also a failure after rebase, but it's unrelated and explained here: 435dbc5#r43742718. Merging.

@tresf
tresf merged commit dc78b15 into LMMS:master Oct 30, 2020
@tresf
tresf deleted the jack-win branch October 30, 2020 20:57
@tresf tresf added the windows label Oct 30, 2020
IanCaio pushed a commit to IanCaio/lmms that referenced this pull request Mar 28, 2021
Add Windows JACK support
Also adds JACK as a submodule
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
Add Windows JACK support
Also adds JACK as a submodule
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Jack audio support on windows

6 participants