Unity-Technologies / NativeAudioPlugins

MIT License
135 stars 28 forks source link

Initial draft. #11

Open nis-unity opened 2 years ago

nis-unity commented 2 years ago

NOTE: This is just an initial draft!

A Revised SDK for Unity Native Audio Plug-ins

This pull-request contains a proposal for an updated Unity Native Audio Plugin SDK.

Why

Working with the original SDK, I found that it is quite convoluted and exposes an unnecessary amount of the low-level C API to users. Furthermore, it uses the C macro preprocessor extensively which is fragile and cumbersome to debug.

Moreover, the project is somewhat of a mouthful with almost 30 audio effects and instruments of varying complexity. A lot of those plug-ins are not production-ready as they do memory allocation and mutex locking on the audio thread. This includes the convolution reverb. In my opinion, a leaner project with a few, simple plug-ins that demonstrate the SDK, is a better approach, provided our goal is to teach users how the SDK works.

Also, right now the 'AudioPluginUtil' component is a frankenstein mash-up of essential functions related to the SDK and convenience functions for doing complex number algebra (why don't we use std::complex?!), Fourier transforms, mutex locking (users should probably use std::mutex instead), and other stuff.

Finally, the original SDK needs some love anyway, as the Xcode project no longer compiles out-of-the-box (haven't been able to test the VS project).

Approach

I've created a clean C++17 layer that completely shields the low-level API from the user. It declares a handful of interfaces that users implement and then the SDK handles all low-level stuff behind the scenes. All of the SDK that users need to understand is contained in a single header, 'UnityAudioPluginInterface.hpp', which is about 100 lines of code. Having a clean and self-contained C++ interface simplifies the mental burden that we put on our users. Also, getting rid of all the macro-preprocessor non-sense and weird callbacks such as 'AudioPluginUtil::RegisterParameter' from the original SDK, makes plug-ins easier to maintain and debug.

On top of the C++ interface, I have implemented two simple plug-ins, an attenuator and a noise generator. Both are very simple plugins. We could probably do with an extra and more complicated plug-in with multiple parameters and more advanced DSP.

I have also created a Unity project, an Xcode project (for macOS) and a Visual Studio project (for Windows) and tested that the code compiles on both platforms. On macOS, I have verified that the plug-ins can load and that they work as expected. I haven't been able to do this on Windows as I don't have a Windows machine yet.

NOTE: I haven't made any changes to the low-level C API. (So far!)

Unclarified Points

  1. We need to discuss what version of Unity / Xcode / Visual Studio we want to use for the SDK.

  2. The way parameters is exposed in the new SDK via a callback function that returns a vector of parameter descriptors (in the 'EntryPoint' struct) is not as userfriendly as it could be.

  3. Is it ok to use the C++ Standard Library (STL)?

Next Steps

  1. We need to implement a sane way to transfer samples from the managed code to a plugin. The way it is done now in the convolution reverb example is a bit of a hack and uses locks and memory allocation on the audio thread.

  2. Better functionality for sending events to plugins from the managed code would be useful for any kind of procedural audio that we want to trigger from within a game.

  3. This one is a little bit complicated! If we made it possible to pass data to a 'UnityAudioEffect_CreateCallback', e.g. via the 'UnityAudioEffectState' struct, we can most likely merge the 'UnityAudioPluginInterface-Raw.hpp' and 'UnityAudioPluginInterface-Raw.inl' headers into the 'UnityAudioPluginInterface.cpp' file. As of now, we need to include both of the 'Raw'-files in the 'EntryPoints.cpp'-file, which is annoying. Getting rid of those two files, 'UnityAudioPluginInterface.hpp' would be the only file that users ever need to think about.

  4. A more complex demo effect, such as an equalizer or something similar, with multiple parameters and a GUI.

nis-unity commented 2 years ago

Naming. We use CamelCasing everywhere else consistently, so I would highly suggest to review the coding conventions and apply that.

I'll fix the formatting.

Aggregrate non-POD (?) initialization requires C++20, I'm not sure whether we want to impose that.

Yep, good point. We should probably stick with C++17 for now.

Either here or in a follow-up, I would abstract the sample buffers to channel + sample argument indexers instead, and hide whether it's internally contiguous and/or interleaved (which we're going away from!)

Very good point. I like this idea!

Consider whether we can split this PR up multiple pieces, eg....

Good idea. We can do that in the end.

monroeberg commented 2 years ago

Hi Nis, overall I think this is a pretty nice update. I've been working with this branch a little bit, as it was the basis for our platform audio programmer test, and it's pretty nice to work with. I love how simple and accessible it feels. I see you're getting some good, detailed feedback already from Janus and I don't have too much to add on that level, but I do have two things.

First, I think we need to make sure it is possible to implement spatializer plugins and ambisonic decoder plugins, even within this new simplified interface. There isn't too much that's unique about them, but you need to have the option to add the following flags to the effect's definition: UnityAudioEffectDefinitionFlags_IsSpatializer, UnityAudioEffectDefinitionFlags_IsAmbisonicDecoder, and UnityAudioEffectDefinitionFlags_AppliesDistanceAttenuation. The other thing is the effect needs to be able to access UnityAudioEffectState's spatializerdata or ambisonicdata, depending on which kind of effect it is. That's really about it. Optionally, it also might be helpful to include a simple spatializer example, just because it kind of helps explain some of the matrix data that is being passed in that isn't maybe super intuitive. It definitely doesn't need to be the full example spatializer that was in the original plugin repo, but instead it could just implement a traditional 3d panning algorithm or something. I have an example you could take a look at if you're interested.

Second, I monkeyed with the programmer test a little bit before I sent it out, so this could have been my mistake, but there might be an issue with the Windows debug build. I was getting crashes when trying to load the debug plugins into the Windows Editor. When you try this out on Windows, definitely check that out, and see if it is all working OK.