Placeholder-Software / Dissonance

Unity Voice Chat Asset
69 stars 5 forks source link

Please explain magic numbers and formulas in code #148

Closed kevinfoley closed 5 years ago

kevinfoley commented 5 years ago

DarkRift2Client line 14 (magic numbers 3, 4096):

private readonly ConcurrentPool<byte[]> _byteBuffers = new ConcurrentPool<byte[]>(3, () => new byte[4096]);

DissonanceVoiceServerPlugin line 16:

private readonly ConcurrentPool<byte[]> _buffers = new ConcurrentPool<byte[]>(10, () => new byte[4096]);

BasicMicrophoneCapture line 77 (magic formula):

_maxReadBufferPower = *(byte)Math.Ceiling(Math.Log(0.1f _clip.frequency, 2));**

BasicMicrophoneCapture line 84:

_rawMicSamples = new BufferedSampleProvider(_format, frameSize * 4);

DecoderPipeline line 60:

bytePool = new ConcurrentPool<byte[]>(12, () => new byte[frameSize decoder.Format.Channels 4]);

martindevans commented 5 years ago

DarkRift2Client line 14 (magic numbers 3, 4096) DissonanceVoiceServerPlugin line 16

These two ConcurrentPool lines are constructing pools of byte[] which are used to temporarily store buffers of network data. The constructor called is this: public ConcurrentPool(int maxSize, Func<T> factory). You can Get a buffer from a pool and it will return a buffer or allocate a new one using the factory function. Once you have finished using a buffer you can Put it in the pool ready for someone else to re-use.

The first number is maximum size of the pool, when a buffer is returned to the pool it will be discarded if the pool is full so this should be a rough estimate of the maximum number of buffers we'll ever need at once. In the case of the client this is small (a single client doesn't send or receive many packets). On the server it is larger because there could be several clients communicating via the server at once. To be honest both of these numbers are fairly arbitrary, the buffers are fairly small so even increasing the max limit by a lot would not waste a significant amount of memory (and the buffers are lazily allocated).

The factory function creates buffers when a new one is needed but nothing is in the pool. So you can see all buffers in both of these pools will be 4KB. This is simply the maximum size of data that can be stored in a buffer, it's a very large overestimate of how big a Dissonance packet will be so it should never be a limiting factor. Most network integrations actually use a limit of 1024 instead of 4096 here and it's not a problem.

BasicMicrophoneCapture line 77 (magic formula)

The mic capture system uses a set of buffers all sized to a power of two. This formula works out what power of two will hold 0.1 seconds of audio at the current sample rate (and then rounds up), this is the maximum buffer size. All the mic audio is drained every single frame, so if more than 0.1s of audio has built up something has gone horribly wrong and we don't bother capturing that audio.

BasicMicrophoneCapture line 84

The flow of data in the mic capture system is:

  1. Mic captures audio
  2. Dissonance drains audio (using a pow2 sized buffer)
  3. The data is moved out of that buffer into the _rawMicSamples buffer, as much as possible at a time
  4. While a frame of data is in the _rawMicSamples buffer it is read out (through _rawMicFrames) and passed on downstream

As long as _rawMicSamples is large enough to contain a single frame it's technically large enough. However it's oversized slightly so that if things lag behind and a lot of audio is delivered at once it can be copied in bigger blocks (4 frames at a time in this case).

DecoderPipeline line 60

This concurrent pool is used to store encoded audio data from the network waiting to be played. Because of the jitter buffer encoded audio may sit in this buffer for a while so the limit is larger than the network packet limit (packets are decoded and recycled ASAP). 12 (frame) * 20 (smallest frame size) = 240ms which is quite a long time for the jitter buffer to delay things, so is a reasonable limit.


In general all of the numbers you have highlighted are fairly arbitrary and are not terribly important so long as they are not too small (which experience has shown they are not). If anything I'm tempted to go back and double the max size of all the ConcurrentPools, the extra capacity will only be used if it is needed, and the extra memory consumption would be insignificant (just a few extra KB), so there's really no reason not to do that.

If that answers all your questions feel free to close this :)

kevinfoley commented 5 years ago

Thanks for the info. I should have been a little more specific in my original post, but you've more than answered everything fully.