AudioUnit allocates on the heap for every RenderCallback

[trampster]

Steps to Reproduce

1. Go to https://github.com/xamarin/xamarin-macios/blob/master/src/AudioUnit/AudioUnit.cs
2. Go to line 730 in method RenderCallbackImpl
3. Notice that for every AudioRender callback an AudioBuffers instance is allocated on the heap

Expected Behavior

No heap allocations during audio playback

Actual Behavior

A new AudioBuffer is allocated for every call for me this is 50 times a second. This will be slow, and increase the frequency of Garbage collection calls, resulting in GC pauses during playback which could impact audio quality.

Environment

You can see this by looking at the github code

Possible solutions:

• Provide a way to specify a AudioBuffers instance for it to reuse. (if the user knows this is safe, ie doesn't hold onto it)
• Change AudioBuffers to a struct, so it is allocated on the stack instead.

[trampster]

I've done some more looking at this. Each call from the native code uses the same underlying buffer (confirmed by printing the memory address of the IntPtr on each call.)

Whoever wrote the c# wrapper code doesn't seem to be worried about the caller keeping a reference to the AudioBuffers instance because it is disposed immediately. (which does nothing because in this case AudioBuffers doesn't own the pointer so the dispose returns without doing anything)

Given these two things I propose the following:
A single AudioBuffers instance is created and is reused for each call. As a safeguard if the IntPtr changes address, (it doesn't seem to) then we can create a new one for it then.

I am happy to provide a pull request that does this.

[trampster]

It would be nice to get an indication if such a change is likely to be accepted before I go ahead and implement it and submit a merge request.

[chamons]

Being able to improve this API to be less allocation friendly would be a solid improvement, if we can do it in a backward compatible way.

Unfortuntely AudioBuffers is a public class, so converting it to a struct would be a hard API break.

On:

A single AudioBuffers instance is created and is reused for each call. As a safeguard if the IntPtr changes address, (it doesn't seem to) then we can create a new one for it then.

I'll let @dalexsoto comment on how viable that is before you work up a PR.

[spouliot]

@trampster the probability of acceptance depends a lot on not having breaking changes in the public API. Since AudioBuffers is public and exposed thru the RenderDelegate then this limit the number of possibilities.

	public delegate AudioUnitStatus RenderDelegate (AudioUnitRenderActionFlags actionFlags, AudioTimeStamp timeStamp, uint busNumber, uint numberFrames, AudioBuffers data);

However that does not makes changes impossible, just a bit harder (to keep existing code working).

A single AudioBuffers instance is created and is reused for each call. As a safeguard if the IntPtr changes address, (it doesn't seem to) then we can create a new one for it then.

I'm no expert on AudioUnit (or our bindings for it), other people will chime in when back at work. Still that sounds like a reasonable approach.

I suggest you to put out a more complete proposal first (it can be a pull request or here in the bug). It might take a few iterations to solve the issue, without creating compatibility problems, but it's clearly worth it :)

[rolfbjarne]

Also note that it looks like RenderCallback (and InputCallback) are bound incorrectly, some of their arguments should be ref arguments: #15808 (comment)