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 ASIO support to CPAL #52

Closed
mitchmindtree opened this issue Feb 15, 2018 · 23 comments · Fixed by #189
Closed

Add ASIO support to CPAL #52

mitchmindtree opened this issue Feb 15, 2018 · 23 comments · Fixed by #189
Labels

Comments

@mitchmindtree
Copy link
Member

For some reason Dante produces a device for every two channels on Windows' WDM API. This is an issue as the audio server expects to be able to target a single device - targeting multiple devices would require a significant hack and be a special condition only required on windows.

The easiest way forward is probably to add ASIO support to CPAL.

Tomaka has mentioned that he's happy to see ASIO support added and that it should follow a platform-specific extensions API similar to the way winit and glutin do. See his comment here.

cc @freesig maybe we can work out some day after Monday next week (when I get back) to have a sesh and work on this together 👍

@mitchmindtree
Copy link
Member Author

mitchmindtree commented Feb 19, 2018

cc @freesig here's a very rough outline of how I'd approach this. Feel free to post comments or call if you get started on this before we hang and anything is unclear of if you get stuck 👍

Setup

  1. Fork and clone CPAL to windows machine and make an asio_backend branch
  2. Install ASIO libs.

Generate Rust Bindings

Use rust-bindgen to generate a module of rust bindings to all items/function is ASIO from the ASIO header file(s). The user guide is super handy. There are two options for approaches here - 1. dynamically generate bindings from header at build time using a build.rs script or 2. generate bindings ahead of time using the bindgen CLI. The rust-bindgen folks recommend approach 1. so I reckon we try that. This will mean adding:
- a build.rs script to CPAL's cargo.toml directory
- an asio/sys.rs module to which the generated bindings will be written (bindgen docs will explain this).
- an asio/mod.rs module in which we will write a friendly API for what we need around the raw bindings in the sys submodule.

Keep in mind that users should still be able to use cpal on windows without installing ASIO, so we'll have to make sure to fail "gracefully"/silently if ASIO is not available and provide a runtime check for checking whether or not it is available.

CPAL API Implementation

We can now begin implementing the API in CPAL. Tomaka's preference for this seems to be to have an operating-system specific extensions module for each module which will contain all items uniquely availble on that OS. As there are currently none of these modules yet in cpal, we will add them. First, we'll add a pub mod os; to the cpal crate root and a src/os/ directory and a src/os/mod.rs for that module. This is where the platform-specific modules will be placed. Next add a pub mod windows; to the src/os/mod.rs and add the associated src/os/windows.rs file. This is where we will provide the user-friendly windows-specific API so that they can access it via cpal::os::windows::some_function(), etc.

Currently, CPAL is setup so that windows defaults directly to using the wasapi.rs backend (this is basically the WDM backend that Dante won't work properly with). I think what we want to do is create a new backend in the windows module which delegates to either the wasapi.rs backend or the asio.rs backend, depending on whether or not some initialise_asio function or something was called. E.g. a CPAL user who wants to choose asio on windows if it's available might do something like this:

extern crate cpal;

fn main() {
    // Only generate code for this block if we're targeting windows.
    #[cfg(target_os = "windows")]
    {
        if cpal::os::windows::is_asio_supported() {
            cpal::os::windows::initialise_asio().unwrap();
        }
    }
    // All following cpal function calls and items will now use the `asio` module internally now rather than `wasapi`.
    let event_loop = cpal::EventLoop::new();
}

So internally, I imagine the windows backend event loop looking something like this:

pub enum EventLoop {
    Asio(asio::EventLoop),
    Wasapi(wasapi::EventLoop),
}

The EventLoop::new() function would construct the correct variant based on whether or not initialise_asio was called or not.

CPAL provides a src/null.rs module as a template for an unimplemented backend, so probably best to copy this into the windows.rs and asio/mod.rs modules to make it easier to get started. From here, it should just be a matter of implementing each of these functions (The ASIO one using the asio/sys.rs module we generated and the windows one by wrapping the asio/mod.rs and wasapi.rs implementations).

@freesig
Copy link
Contributor

freesig commented Feb 28, 2018

Dante Virtual Soundcard is complaining about the license being activated too many times. I can get a trial for developing. Might need @JoshuaBatty to login to the Audinate website and ask them for a to reset it because we need to move it to the NUC. Or just send me the Audinate login / pass and I can sort it.

Also a little update on this. I've finally (with lots of help from #rust irc) got cpal to compile and link with the ASIO SDK. I'm about to test the if I can see any asio drivers.
One issue I've come across with bindgen / c++ is a linker error when the Base class virtual destructor gets called. I just stopped calling the destructor but this is not a good solution.

@freesig
Copy link
Contributor

freesig commented Mar 1, 2018

Ok I can see the Dante asio driver from the asio-sys test :)
It crashes though because I don't think I'm handling the FFI correctly.
I'm also having problems with linking the libraries correctly.
But I'll do some reading. At least we know it's possible.

@freesig
Copy link
Contributor

freesig commented Mar 2, 2018

Ok I've got it showing 64 channels.
This is in the asio-sys example test.
So I might tidy it up and try and implement it for the enumerate example.

@freesig
Copy link
Contributor

freesig commented Mar 22, 2018

Get audio both directions through ASIO.

Use CPAL examples beep and record to test this works.

Please feel free to add / edit these if my understanding is off:

  • Add a way for the preprocessor to choose asio as impl_device instead of wasapi
  • First match behavior of enumerate. This might work without need to expose anymore ffi
  • Add required functions to the asio ffi to for audio input / output
  • Create the api for asio mod that allows beep and enumerate to run. Test using asio4all
  • Copy the rest of the wasapi api for the asio mod.

This last one I understand the least. But I'm guessing all "devices" have to match the interface that cpal is expecting.

Plan to get this done in a little 2 - 4 day sprint starting tomorrow.

@mitchmindtree
Copy link
Member Author

Nice one @freesig, sounds great, I'll be working on audio_server non-stop until its done starting tomorrow - feel free to hit me up if you'd like help/advice at any point! Maybe we can do a voice at some point and make a game plan for now until completion 👍

@freesig
Copy link
Contributor

freesig commented Mar 24, 2018

Hey @mitchmindtree today I tried to fill out a Format struct from ASIO.
I attempted to get the sample rate from ASIO but I'm stuck on a link error
This is the banch
Todays work is mostly in asio-sys/examples/enumerate.rs and asio-sys/lib.rs:get_sample_rate()
(A lot of the style is overly explicit and immutable. I will optimize it after. Just wanted to get the design clear first)
I think that this might be confusing the bindings:

#if IEEE754_64FLOAT
	typedef double ASIOSampleRate;
#else
	typedef struct ASIOSampleRate {
		char ieee[8];
	} ASIOSampleRate;
#endif

It's complaining that it can't link:
ASIOGetSampleRate(info:` *mut ASIOSampleRate) -> ASIOError;
but it links to:
ASIOInit(info: *mut ASIODriverInfo) -> ASIOError;
just fine.
They live in the same file and both are in the bindings.
My guess is that bindgen doesn't handle the preprocessor:
#if IEEE754_64FLOAT

Tomorrow

We should definitely have a voice chat tomorrow afternoon (my time, your morning I think) because I'm not that sure on how to handle this os::windows mod etc.
Right now I'm just writing examples inside asio-sys that mimic the cpal examples.

@freesig
Copy link
Contributor

freesig commented Mar 26, 2018

Ok got passed the link error.
Not sure what to set Format.SampleFormat to because it only has I16, U16 and F32 but I think for this computer its F64.
Anyway @mitchmindtree we should have a voice chat when you're free just to go over how the os::windows structure works. I think I've got all the ASIO backend to satisfy enumerate, so we could start with trying to get cpal run --example enumerate to work with asio

@freesig
Copy link
Contributor

freesig commented Mar 29, 2018

Ok so now:

  • All the wasapi is working under the new platform module.
  • All the interface is made for windows::asio.
  • Functions in asio are all unimplemented!
    Next step is to start implementing the asio module.

Question
Right now the backend is static backend: Backend = Backend::Asio
But how can we set this depending on whether or not the user wants to use asio?
Given static mut is unsafe.

@mitchmindtree
Copy link
Member Author

mitchmindtree commented Mar 29, 2018

Nice work!!

But how can we set this depending on whether or not the user wants to use asio?

The safest way to do ths would be to use the lazy_static crate - it allows you to lazily initialize statics. Btw, the reason why mutatinng a static mut is unsafe is that it is accessible and mutable from any thread. If we wrap it in a lazy_static Arc<Mutex<>> it should be fine. Alternatively we could use an Arc<AtomicBool> to check, but I think your Backend enum is probably nicer and more future proof 👍

Edit: Actually the Arc is probably not needed.

@freesig
Copy link
Contributor

freesig commented Mar 29, 2018

Yeh I was thinking some sort of lazy init would be what we needed and that lib looks like what we need

@freesig
Copy link
Contributor

freesig commented Apr 1, 2018

Ok really basic version of enumerate is working for asio now.
I skipped the search all supported formats for now because its a bit of time and want to get something working with just the default format.
Speaking of default, ASIO has no concept of this so I just return the first in the list.
Also the data_type in Format isn't right but I will get that going asap.

@freesig
Copy link
Contributor

freesig commented Apr 4, 2018

So today I had a go at beep. I'm able to create the buffers. This is how asio handles it:
image

Next

Need a way of storing a handle to the stream.
I think this will live in EventLoop but I'm not completely sure how this will work.

@freesig
Copy link
Contributor

freesig commented Apr 14, 2018

Ok so I've hit a bit of a wall with the "Running" part.

Situation

  • Asio requires a c struct of function pointers for call backs.
  • There's two audio buffers for that swap. You fill one while the other is delivered to the audio card.
  • One of the functions in the struct:
    void (*bufferSwitch) (long doubleBufferIndex, ASIOBool directProcess);
    Is called by asio to let you know that it's time to swap and fill the other buffer.

Problem

How can I communicate from this callback to anything in in cpal.

Things I've tried

  • Use channels. (There's no way to give the switchBuffer a handle to the channel without altering it's signature, which would then not be accepted by Asio)_
  • Put the handle in the struct. (Can't do this because the struct that Asio is expecting is a certain size and cannot hold other data)
    (basically can't use any function that takes self as an arg because that from the c side is a different signature)
  • Use a closure. (I thought this was a promising approach because it looks like the coreaudio is doing something similar. But it seems the closure is actually a struct behind the scences with fields that reference its context. So technically a closure takes self as an arg. Although I haven't ruled this approach out, I can't figure out if there's a way to make it work)
  • Use a global variable (This would work. Although it seems messy to use a global in a threaded context like this, but it's doable with the right guards. This seems like the only option)

I'm thinking of going the global variable option but I just wanted some advice before going down that road.

@mitchmindtree
Copy link
Member Author

One of the functions in the struct:
void (*bufferSwitch) (long doubleBufferIndex, ASIOBool directProcess);
Is called by asio to let you know that it's time to swap and fill the other buffer.

By this, do you mean that this callback is the function in which the user is expected to fill the next buffer? If not, what exactly is the user expected to do when this function is called? Just want to clarify this before adding further advice

@freesig
Copy link
Contributor

freesig commented Apr 14, 2018

Yeh exactly. So the doubleBufferIndex is either 0 or 1 and that is set to the index of the buffer you should fill now.

@mitchmindtree
Copy link
Member Author

OK sweet, yeah I think the best bet for this would be to use a closure and do something similar to what is going on in the coreaudio backend. It may be necessary to Box this closure in order to pass it to the ASIO backend without requiring adding type parameters to it. I think the reason why the closure is wrapped in a struct on the other hand is so we can easily cast the "data" void pointer provided by the C callback back to that struct type and then call the inner closure after that is done. Definitely a bit of a tricky task, but should be doable - wanna have a voice chat about this? I'll jump on messenger.

@freesig
Copy link
Contributor

freesig commented Apr 16, 2018

I think I'm getting close to getting example beep to run on asio but I'm stuck on one last problem:

  • I'm storing the callbacks so they need to be in a Box because they aren't sized.
  • Run doesn't take a lifetime.
    pub fn run<F>(&self, mut callback: F) -> !
        where F: FnMut(StreamId, StreamData) + Send

Problem: Box needs to know the lifetime of the closure to make sure the captured data lives long enough.

In the coreaudio version they got around in by doing this:

            let callback: &mut (FnMut(StreamId, StreamData) + Send) = &mut callback;
            self.active_callbacks
                .callbacks
                .lock()
                .unwrap()
                .push(unsafe { mem::transmute(callback) });

Which stores into this:
callbacks: Mutex<Vec<&'static mut (FnMut(StreamId, StreamData) + Send)>>

Seems very unsafe to transmute the callback.
Is there another way to get around this lifetime check?
_All the other implementations don't store the call back but this would not be ideal for ASIO because of the way that it needs to interact with the callbacks in a global way.

@mitchmindtree
Copy link
Member Author

Send and Sync traits

So, the Send and Sync traits are two "built-in" traits. These are special in the sense that they're automatically implemented for all types that can implement them. These along with ownership are the reason rust can have such thorough guarantees about thread safety.

Send is a trait that is auto-implemented for types that can be safely moved between threads. If the language itself can't determine whether or not it is safe (e.g. if it contains a reference bound to a life-time on the current thread's stack, or if it is a raw mutable pointer, etc), the user must manually and unsafely implement the trait themselves, but only if the type and its exposed API meet the trait's expected behaviour of being safe to send across threads.

Sync is a trait that is auto implemented for types that can be safely shared between threads. Arc for example requires that the type it is pointing to is Sync, otherwise a compile-time error might cocur. An easy way to determine whether a type T is Sync is if &T is Send.

The std docs should also have a lot of info on these.

Boxed Closure Lifetime

So the main reason the box needs to know about the lifetime of the closure is because it is being sent from the current thread to the audio thread. This means the lifetime of the given callback must be static as there is no other way to guarantee that the lifetime of the callback is at least as long as its use on the audio thread. In this case, the Box type we want should be something like Box<FnMut(StreamId, StreamData) + Send + 'static> - the key being the 'static lifetime. Unfortunately the CPAL API itself doesn't require this lifetime yet, and I don't think it's been discussed in the issues yet. So by the looks of it, the way the coreaudio backend works around this is by just "assuming" that the lifetime of the given callback is 'static and forcing the compiler to accept this using unsafe - definitely super unsafe and probably needs some review.

Will talk to ya some more in person soon 👍

@freesig
Copy link
Contributor

freesig commented Apr 19, 2018

Update

Working:

The beep example is now working smoothly. It sounds correct.

Issues:

  • This is with hardcoded values for i16. Unfortunately ASIO seems to be insisting on i32 which isn't supported by cpal.
    solution: This could be overcome by mapping the i16 range to i32. Using i32::MAX etc.
  • Deinterleaving: I wasn't expecting this and it's not trivial to do fast. Basically CPAL audio is interleaved R, L, R, L etc. and ASIO is R,R,R,R,L,L,L,L etc.
    solution: Basically I used a step iterator to grab every second value and another offset step iterator to grab the odd index values. then append one to the other. This needs more testing because I'm not sure which channel is which part of the buffer. It's also slow.
    The ASIO docs are confusing on this point:

For efficient processing and great flexibility ASIO implements channels (input and output) as circular buffers with blocks of data. Actually a double buffer mechanism is used, which can be easily adapted to a great number of different buffer implementations. One buffer contains always data for one single channel only. This approach allows the host application to implement additional processing algorithms very effectively (opposed to an interleaved buffer model). The size of the data blocks is ascertained from the audio driver, to allow the best support for the audio hardware.

My interpretation is that you get a buffer and then that is for say channel 1. Then it swaps and the next buffer is for channel 2. Then it swaps and the next buffer is for channel 3.
There is a ASIOGetSamplePos() call that gets you some sample position. I think this might be how you know which channel you are sending to but I'm not sure.

  • Little / Big endian: The SampleType can be in either so might need to swap the buffer depending on what our current system is using. Easy enough. But this might be another point of slow down.

Summary

Theres probably a reason that portaudio-asio is 3000 lines but I found this great file outlining how they did it HERE
I have the next 3 days blocked out to work on this. It's going to be a long project before its very stable and generic so I think we should really focus this on just what is needed for the audio server.

@freesig
Copy link
Contributor

freesig commented Apr 19, 2018

actually on second thought it looks like the buffer pointers are per channel. Which is odd because I'm only using one but I'm getting stereo. This might just be luck because they are next to each other in memory. Gotta love unsafe land

@BlueJayLouche
Copy link
Collaborator

BlueJayLouche commented Apr 19, 2018 via email

@mitchmindtree
Copy link
Member Author

@BlueJayLouche I think the main issue on linux is that by default PulseAudio takes exclusive access to the audio hardware and CPAL (the cross platform audio backend we're using) currently only targets ALSA directly. As a result if pulseaudio is running, you eventually run into some contention or a data race which can cause a segfault or some other weird undefined behaviour. I think there are a couple workarounds for this:

  1. Disable pulseaudio.
  2. Run pulseaudio on top of ALSA's dmix rather than letting it take exclusive access over the hardware.
  3. Add pulseaudio support to CPAL.

Just thought I'd add this for the record in case someone comes across this in the future!

@freesig Awesome work on getting the CPAL ASIO beep going and finding that portaudio reference doc, looks super useful!

De-interleaving

This is isn't too uncommon to have to interleave or deinterleave buffers at different parts of the signal chain, and will rarely show up as a bottleneck (especially if we only have to do it once ASIO -> CPAL or vice versa). My understanding is that non-interleaved channels can be more efficient for some purposes and interleaved can be more efficient for others. It would definitely be useful if CPAL actually just served the original buffers whether they are interleaved or not, maybe in some enum.

Endianness

We can possibly use the byteorder crate for this, but it's hard to know without seeing the implementation just yet.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants