freesig / cpal

Audio player in pure Rust
0 stars 0 forks source link

Opt into ASIO #9

Closed freesig closed 5 years ago

freesig commented 5 years ago

Right now ASIO is on by default for windows and we need a way to only build / compile the code if a flag is set

mitchmindtree commented 5 years ago

There are two processes in particular that come to mind that should be optional.

1. Compiling and binding to the ASIO SDK.

Compilation and binding of the ASIO SDK should probably be done based on whether or not the CPAL_ASIO_DIR environment variable is set. If this variable is set, we should assume that the user might actually want to use ASIO and thus we should attempt to build it and generate bindings. If any step within this process fails, we should print to stderr so that the failure can be visible to the user, however we should not panic! or halt the whole build process as the user may just want to use the WASAPI backend (e.g. maybe they just set CPAL_ASIO_DIR for an old project but they don't need it for the current project).

2. Selection of ASIO backend vs WASAPI.

Ideally we would use something like a Host API, but until something like that lands we might need to use some sort of global, platform os-specific function. For example allow the user to do something like this:

fn main() {
    #[cfg(target = "windows")]
    {
        if cpal::os::windows::asio_is_available() {
            cpal::os::windows::use_asio_backend().expect("failed to select ASIO backend");
        }
    }
    // ...
}

@freesig thoughts?

freesig commented 5 years ago

Yep that seems like a good idea. I imagine some people will want the option to switch between asio and WDM at runtime which the global should allow for. Definitely want to avoid building ASIO for anyone that hasn't explicitly called for it. Do you think it should be the CPAL_ASIO_DIR or should we take some cli arg eg cargo build my_app --asio? I'm just thinking some people will put the asio sdk in ~/.asio and therefor won't need the CPAL_ASIO_DIR

mitchmindtree commented 5 years ago

or should we take some cli arg eg cargo build my_app --asio

We can't really require command line arguments as CPAL is just a library, but the user could totally do something like that. I think it's fine to use the existence of CPAL_ASIO_DIR as an indicator on whether or not to build the ASIO backend.

I'm just thinking some people will put the asio sdk in ~/.asio and therefor won't need the CPAL_ASIO_DIR

I think we should still require CPAL_ASIO_DIR for building the ASIO backend for consistency, even if the user has their SDK in ~/.asio - I originally only suggested ~/.asio as a recommended default for where users might want to put their ASIO SDK so that they have one less thing to think about.

freesig commented 5 years ago

This is implemented now:

cpal::os::windows::use_asio_backend().expect("failed to select ASIO backend");

But I'm not sure how to implement this:

cpal::os::windows::asio_is_available()

How can we test if the asio backend is there?

mitchmindtree commented 5 years ago

Hmm what does use_asio_backend do if ASIO isn't there? Maybe we can just rely on the user's using that rather than asio_is_available?

freesig commented 5 years ago

use_asio_backend sets a enum that at run time decides which to use for each call. I'll try deleting the bindings and not giving the flag. It will probably give a compile error. Might need some way so that if CPAL_ASIO_DIR isn't supplied then the asio mod doesn't compile. I'm not sure how to do this though because detecting Env vars is a runtime op

mitchmindtree commented 5 years ago

Ahh true! I suppose that if ASIO has been compiled than it should be assumed that is_asio_available would always return true, because at runtime ASIO itself doesn't rely on any ASIO dynamic library or anything like that, it just compiles into the static bin or lib right? In that case, we probably don't need a is_asio_available at all.

Maybe we should move the use_asio_backend function into the asio module then? That way ASIO can only be selected on windows if the SDK has been compiled. I think that makes sense?

freesig commented 5 years ago

ah so calling use_asio_backend would be a compile error if there's no asio. The inner modules are private because all iterations go through the outer cpal wrapper so we can't actually name a inner function that might not compile.

We might be able to use rustc-cfg=FEATURE from the build.rs to just set a cfg(asio = "true") and then use that to make the use_asio_backend available from cpal::os::windows

freesig commented 5 years ago

Ok this is working. I'll close this now but note this commit when you do your review @mitchmindtree because I'm not sure this is the greatest approach. I had to add a build script to cpal and do a few other hairy maneuvers