Optional standalone format#12
Conversation
208f79a to
872d5d3
Compare
872d5d3 to
56dfdbb
Compare
|
I would prefer not to have tool-specific files like justfile in the repo |
|
In general, I think having a standalone version for testing is a good idea. Lack of duplex audio in cpal is somewhat of a blocker but apparently it should be landing soon: RustAudio/cpal#1096 |
|
The fact that it's currently opening all MIDI input devices doesn't feel optimal; on Windows up until a recent Windows 11 update, MIDI device access is exclusive so running this would block all MIDI input for other applications unless they already had the device open. As long as this is only for testing, it's not a huge issue of course, but if we think about extending this feature to support standalone releases like some plugin instruments are doing, this isn't acceptable. What I would do is that instead of a macro to automatically generate a main function and application, have the developer write the main function themselves and call a function to set up things like device selection. It doesn't have to be all there at once, but I would drop the macro from this PR and use the manual main function method instead. Then add device selection and probably other features later. |
|
Oh and also, I have "run everything through rustfmt and start doing formatting on save" on my TODO list, just haven't gotten around to that yet. Don't worry about matching my formatting. |
- optional feature, disabled by default - CPAL for audio IO, opening the default device with default settings - midir for note input, opening the first available MIDI input device - winit to create a slint host window
to ease building and testing the gain-plugin example
Yep, that's indeed quite a hack, but it avoids creating config files or passing cmd line args, or adding a config GUI. The proper version of it would be adding a Slint GUI for that - embedding the plugin view into some custom GUI, but that's out of scope for this PR. Everything else, env variables, config files or cmd line args, do not really fix anything, as we need to get the names of the devices from somewhere in order to specify them. Fine with me to drop the main macro, but there's also no need to use it in case you need a custom main. What would work for you now, a compile-time or run-time option to avoid opening up all MIDI devices? |
56dfdbb to
eef5489
Compare
I think having the developer write a custom main function instead of using a macro feels like the correct choice in the long run since for anything except quick and dirty development builds the macro can't really do everything that's needed.
Current behavior is fine for now! I just think going the manual main function route and offering an API for querying available devices etc. is the only good solution down the line so let's start on that road now. |
|
What about adding a standalone 'run' function (and skipping the main macro) that takes a 'config' argument, to make that clear? The config's default impl will use be set up to use the default audio device and opens all MIDI devices, so the easy, default option oasth is then one we have now but allow configuring it somehow. I could give this a try later this week... |
|
Yeah I was thinking of a similar solution, sounds good. We can always support more complex scenarios later. |
|
Added assertions, logs for the audio allocs as discussed, as well as a basic README to the gain example to clarify how it can be built (bundle vs. standalone) now that the justfile is gone. I don't think this is obvious, and it will help people who are new to all this. Please change the README as you see it fit or remove it, if you don't want it there. Going to take a look at the standalone config stuff later this week or next week. |
|
Added audio and MIDI configs now, defaulting to default audio device and all MIDI inputs, as discussed. Kept the |
| // Process audio, ensuring we don't call process with more than P::MAX_BLOCK_SIZE frames | ||
| debug_assert!( | ||
| self.buffer.capacity() == P::MAX_BLOCK_SIZE, | ||
| "Buffer must be preallocated to avoid allocation on the saudio thread" |
| /// Returns all audio drivers available on this platform. | ||
| /// | ||
| /// Always includes [`AudioDeviceDriver::Default`], followed by any named drivers that are | ||
| /// currently available (e.g. ASIO, WASAPI on Windows; ALSA, JACK on Linux). |
There was a problem hiding this comment.
Comment about JACK doesn't match code below
| Ok(devices) | ||
| } | ||
|
|
||
| pub fn open_host(&self) -> Result<cpal::Host, Box<dyn std::error::Error>> { |
There was a problem hiding this comment.
Just a note: down the line I would prefer a type state based solution for opening devices etc. but for now I think this is good enough
There was a problem hiding this comment.
I think we can get fancy here when someone actually starts using it :) Didn't wanted to bloat this unnecessarily now.
I'll remove the macro. Okay with you to add two run functions then run_standalone and run_standalone_with_config where run_standalone uses default configs?
| @@ -1,8 +1,37 @@ | |||
| /// Generates a `main` entry point that runs the given plugin as a standalone application. | |||
There was a problem hiding this comment.
The documentation here is nice 👌
However I still think that having a macro for something that's a one-liner with the default config is not a good choice. The documentation could be in standalone.rs and possibly on the front page when we have proper docs at somepoint.
| } else { | ||
| for port in &ports { | ||
| let name = midi_in.port_name(port).unwrap_or_else(|_| port.id()); | ||
| log::info!("Available MIDI input port: '{name}'"); |
There was a problem hiding this comment.
"debug" feels like a more approriate log level here as this is potentially spammy
…standalone_with_config` with examples in comments
|
Oh, btw. I didn't enable |
|
A dedicated error type for Audio/MIDI related errors from the config also wouldn't hurt. Please add them as you seem this fit later... |
Adds a new optional
standalonecrate feature, which adds a new 'Standalone' plugin format and plugin export.The standalone impl uses:
It will be difficult to add a custom GUI for MIDI and audio settings, so right now CPAL uses the audio host with the default audio device, which usually works just fine. As there is no such thing as a default MIDI device the standalone tries to open all MIDI input devices and forwards all MIDI events from each device to the plugin.
The editor is attached to a bare-bones window created by winit, and scaling is set as most host would do.
Duplex audio isn't possible with CPAL right now, but we could allow a wav file to be loaded via a program argument, which would then be looped forever as the audio input for FX. But I don't really need that, so I skipped it.
Note that this is a very basic implementation, and isn't intended for release to users but just eases development and debugging. I find that especially useful in combination with Slint's live preview feature.
To document how to use the live preview feature and bundling, for example, I've added a
justfileas well. I personally use them everywhere, because they help me to remember commands when switching between projects. Let me know if you would prefer me to remove it.Also totally fine if you think a standalone target is not worth maintaining. I'll then simply keep it in my branch for testing...
btw: I find it very hard to follow your code formatting style. I usually run cargo fmt on save, but this messes up your formatting. Maybe you could add some custom cargo fmt rules to ease that?