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

cmake: support building VST3 #330

Merged
merged 4 commits into from
Sep 29, 2021
Merged

cmake: support building VST3 #330

merged 4 commits into from
Sep 29, 2021

Conversation

jpcima
Copy link
Collaborator

@jpcima jpcima commented Sep 28, 2021

Builds the VST3 bundle, has support code to determine the name of the architecture folder.
Has a to-do remark on macOS, lacking the Info.plist template at the moment.

Add detections for ARM/ARM64 Windows architectures later.
For now it's mentioned by Steinberg as "Proposal" only, and I don't know MSVC architecture strings for this pair.

LIBRARY_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin/${NAME}.vst3/Contents/${vst3_arch}-win/$<0:>")
else()
set_target_properties("${NAME}-vst3" PROPERTIES
LIBRARY_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin/${NAME}.vst3/Contents/${vst3_arch}-linux/$<0:>")
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm this is not quite correct due to DPF supporting basically anything posix compatible..
I know Steinberg has no spec outside of mac, windows and linux, but that perhaps should not stop us?
Or maybe we raise the issue upstream.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unless mistaken, it would appear that at least FreeBSD already supports the "linux" naming.
The port of sfizz has the VST build option, and we don't have a complaint by yuri about it.

Copy link
Contributor

@falkTX falkTX Sep 29, 2021

Choose a reason for hiding this comment

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

tbf yuri just packages things without much concern, so even without complaints it still doesnt seem right.
does bsd even have a vst3 host at this point? (not sure how ardour or qtractor work there)
if someone wants to ship a multi-os binary that includes bsd and linux at the same time, well that is just not possible if both are defined as "linux"

on bsd it would seem logical to just use the "bsd" string instead of "linux". but for a few other systems it is not so clear.
the vst3 spec just needs to say what format or style to use for not-yet-inside-vst3-defined-systems.
lets raise an issue upstream, I will do that now.

Copy link
Contributor

Choose a reason for hiding this comment

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

opened ticket at steinbergmedia/vst3sdk#86

@falkTX
Copy link
Contributor

falkTX commented Sep 29, 2021

Do you know if the vst2 macos bundle plist can be used as-is for vst3?
I havent verified this at all yet.

The windows build error is an odd one there, perhaps due to the location of V3_API ?

@jpcima
Copy link
Collaborator Author

jpcima commented Sep 29, 2021

Do you know if the vst2 macos bundle plist can be used as-is for vst3?

According to what we have in sfizz, most likely yes.

The windows build error is an odd one there, perhaps due to the location of V3_API ?

It would seem so, V3_API should go after return type.

@jpcima jpcima force-pushed the cmake-vst3 branch 2 times, most recently from 8b655ba to f50a866 Compare September 29, 2021 01:34
@jpcima
Copy link
Collaborator Author

jpcima commented Sep 29, 2021

Note: Thread and Mutex do not work on MS compiler
is something wrong about using standard-library versions of these things?

@falkTX
Copy link
Contributor

falkTX commented Sep 29, 2021

Note: Thread and Mutex do not work on MS compiler is something wrong about using standard-library versions of these things?

It is not properly supported in mingw and it is a C++11 feature.

The thread should be commented out for macos/windows, or I guess more easily for now to just comment it out for windows.
The final version should not use threads.

@falkTX
Copy link
Contributor

falkTX commented Sep 29, 2021

Note you can't remove the V3_API from the lambda functions, it is needed by mingw32 builds.
Or at least it was before the lowlevel v3 struct functions were like before..
Did you try this to work on win32 and win64?

@jpcima
Copy link
Collaborator Author

jpcima commented Sep 29, 2021

Note you can't remove the V3_API from the lambda functions, it is needed by mingw32 builds. Or at least it was before the lowlevel v3 struct functions were like before.. Did you try this to work on win32 and win64?

I didn't try anything except to get it to build, which currently it doesn't.
That which I find out, it's that stdcall lambdas are not a possibility using msvc.

With that compiler, the non-capturing lambda pointer admits a conversion operator to a stdcall version of it.
This solution based on static_cast is very verbose.
https://stackoverflow.com/questions/17714175/how-to-use-stdcall-to-qualify-c-lambda#comment25818130_17714218

@falkTX
Copy link
Contributor

falkTX commented Sep 29, 2021

I didn't try anything except to get it to build, which currently it doesn't.
That which I find out, it's that stdcall lambdas are not a possibility using msvc.

oh wow, impressively bad haha
well, lets just disable the vst3 on msvc for now then.
the final version won't use lambdas or threads, and this works on mingw anyway for now in case we need to do windows testing.

I will try to get rid of the lambdas soon then, won't postpone it much.
Just please add back the V3_API on the lambdas as needed for mingw.
Thanks

@jpcima
Copy link
Collaborator Author

jpcima commented Sep 29, 2021

It's done.

@falkTX
Copy link
Contributor

falkTX commented Sep 29, 2021

Thanks!

@falkTX falkTX merged commit 87a195a into DISTRHO:develop Sep 29, 2021
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