Process Audio Loopback Capture

[millardjn]

I've tried a first pass at addressing Issue https://github.com/HEnquist/wasapi-rs/issues/26 as I was interested in the same functionality. The wrapper approach is a bit clunky but I couldn't see an easy way to merge it with the normal capture without opening a bunch of potential misconfiguration to users.

[HEnquist]

Thanks for the PR! It gets a bit limiting with having all the methods implemented separately on two structs (the wrapper and the normal AudioClient). I think this could maybe be improved by introducing two traits. One for all the methods that work for both cases, implemented for both structs, and one that adds the methods that only work for the normal AudioClient. That way it should be possible to build apps that can use both, without needing two completely separate code paths.

[millardjn]

I agree that the wrapper class isn't great, other than having more code separation for the first pass. I think a traits method (Option 1) could work. It has the downside of making the normal AudioClient a bit harder to use and docs heavy, and moves further from a 1:1 mapping to the underlying API. A couple of alternatives are:

(Option 2) Have the special process capture constructor on AudioClient, and carry an extra flag into initialization to branch on. Then handle the misconfiguration risk with a bunch of extra doc notes and a good example on the special constructor. This was going to be my original approach but it felt like my new code was weaved all existing sections that I didn't have a good understanding of.

(Option 3) Have a temporary constructor class with just the constructor method and the modified initialization method which would then return a normal non-wrapped AudioClient. Again just handle restriction on methods with doc notes. Whether this is viable comes down to the part of the lifecycle between construction and initialization and if it is worth abstracting over.

I'm happy to take a swing at any of the above, though it will probably be weekends only.

[HEnquist]

It's not obvious what is the best way. Option 2 has the advantage of staying quite close to the underlying windows api. Do you know if the limitations in process capture are described in some Microsoft docs? I think I prefer this solution. I think it should be enough to mention the limitations in the docs for the special constructor. The comments you wrote for those commented out methods on the wrapper would be a very good starting point. Option 2 also has another possible advantage. In case some of the methods that don't work on the process capture audio client start working in a future windows version, no changes are needed to support this.

I'm happy to take a swing at any of the above, though it will probably be weekends only.

Sounds great! All work on this project is done on weekends and evenings so this fits nicely :)

[JohnMitchell04]

Hi, just about to submit my pull request for this, was a bit late as I have been quite busy recently, my implementation is broadly the same but I have implemented solely on the AudioClient struct.

Having briefly looked over this request I can see one potential issue with memory here: pBlobData: &audio_client_activation_params as *const _ as *mut _, in the prop variant activation, it should be held in a box otherwise there is a memory panic at least when I was testing it.

[HEnquist]

Ok so now we have to suggestions for how to implement this :) I haven't had time to look closely at either yet, but at a glance both contain good stuff. It would be great if we could somehow combine the best of both.

As I mentioned before, I like option 2 above, which is the approach taken in #29. This PR (#28) however contains more information on what methods are useful or not. Could the wisdom in those comments be included in the docstrings of #29? Another nice thing in #28 is that the activation handler is implemented in the library, so that there is no need to have it in the code using this library (like in https://github.com/JohnMitchell04/wasapi-rs/blob/a0c330847eb28268e37ff7acf544c28fa5589d1c/examples/application_loopback_capture.rs#L19).

[millardjn]

OK! I've integrated the information in each in the style of Option 2. Switching the direction of the AudioClient from Capture to Render as per JohnMitchell04's PR solved problems integrating with the existing AudioClient initialize client method.

To address the panic https://github.com/HEnquist/wasapi-rs/pull/28#issuecomment-2091715736 I've used pinning for the values passed as pointers during interface activation in https://github.com/HEnquist/wasapi-rs/pull/28/commits/f6d04c3f86117503e4509d5eff5df3dc73624f1d. Could you give this a test @JohnMitchell04? I haven't been able to reproduce the panic and can't use MIRI to check it because of all the FFI going on. I assume the compiler was free to move unpinned values around on the stack even if it wasn't happening to me. If we use a box for the first struct I still think we have to pin the second one, but we could get rid of the ManuallyDrop wrapper if we trust that the dealloc performed by windows will always have the right alignment etc.

[JohnMitchell04]

All appears to be working on my end and everything looks like a good implementation as far as I can see.

[HEnquist]

Thanks for updating! This is looking good. I'll merge it and then make a new release once I have looked at https://github.com/HEnquist/wasapi-rs/issues/27.