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

Add CLI to wrap CLAP plugins #316

Open
wants to merge 11 commits into
base: next
Choose a base branch
from
Open

Conversation

CrushedPixel
Copy link

@CrushedPixel CrushedPixel commented Oct 10, 2024

DRAFT PR. DO NOT MERGE. DISCUSSION REQUIRED.
Any feedback is welcome.

This still has several flaws:

  • largely untested on Windows
  • Standalone doesn't work on macOS either Standalone is unsupported atm as we can't build it into single files on all platforms.
  • uses CPM to get fmt when we already vendor it, cause otherwise the program doesn't link
  • all the metadata going into AU needs to be removed once it's read from the CLAP. This should happen before this gets merged

Needs to be discussed:

  • replace OS APIs with std::filesystem at the expense of bumping min macOS version to 10.15?

This still has several flaws:
- completely untested on Windows
- Standalone doesn't work on macOS either
- uses CPM to get fmt when we already vendor it, cause otherwise the program doesn't link
- all the metadata going into AU needs to be removed once it's read from the CLAP. This should happen before this gets merged

Needs to be discussed:
- replace OS APIs with std::filesystem at the expense of bumping min macOS version to 10.15?
@CrushedPixel
Copy link
Author

CrushedPixel commented Oct 10, 2024

The error when building standalone:

[ 87%] Building OBJCXX object CMakeFiles/gain_standalone.dir/_deps/clap-wrapper-src/src/detail/standalone/macos/AppDelegate.mm.o
/Users/marius/Documents/CLAPPlugins/clap-wrapper/cmake-build-debug/cli/wrapped/wrap-build/_deps/clap-wrapper-src/src/detail/standalone/macos/AppDelegate.mm:41:24: error: use of undeclared identifier 'HOSTED_CLAP_NAME'
   41 |   std::string clapName{HOSTED_CLAP_NAME};
      |                        ^

Is it even possible to wrap a CLAP plugin into a single portable standalone binary on Windows? If not, do we even want to support this from the CLI?

cli/src/main.cpp Outdated Show resolved Hide resolved
cli/src/main.cpp Outdated Show resolved Hide resolved
cli/src/main.cpp Outdated Show resolved Hide resolved
@baconpaul
Copy link
Collaborator

OK so a couple of things looking

  1. We can use filesystem just we need to use either ghc or built in. The wrapper does this now and has a target for it.
  2. I gotta figure out why fmt didn't work

so let me pull this branch down now and see if i can fix that and push up a diff or two to get you started.

baconpaul and others added 3 commits October 10, 2024 08:25
1. Make fmt work from the vendored fmt
2. Add the cmake flags so we can use std::filesystem back to 10.13
   in the fs::space and modify the create_directories to use a path
   at least
3. Vendor in CLI11
- Also use fs for all path related activities
@CrushedPixel
Copy link
Author

CrushedPixel commented Oct 10, 2024

At this point, I believe what's left to do is:

  • test/fix Windows
  • add license headers
  • write a README

Please let me know if there's any more work to do.

Copy link
Collaborator

@baconpaul baconpaul left a comment

Choose a reason for hiding this comment

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

Some comments on a quick morning skim

else()
add_custom_command(TARGET ${VST3_TARGET} POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy_directory
"${OUTPUT_DIR}/${PROJECT_NAME}.vst3"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can't work right? IT copies from nowhere to the output dir?

Copy link
Author

Choose a reason for hiding this comment

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

Whoops. Should be fixed now, but requires some testing on Windows, so I won't resolve convo yet

cli/src/main.cpp Outdated Show resolved Hide resolved
cli/src/main.cpp Outdated Show resolved Hide resolved
cli/src/main.cpp Outdated Show resolved Hide resolved
@CrushedPixel CrushedPixel changed the title [DRAFT] Add CLI to wrap CLAP plugins Add CLI to wrap CLAP plugins Oct 11, 2024
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