Conversation
mofr
left a comment
mofr
left a comment
There was a problem hiding this comment.
Thanks for your pull request! I've left some comments which I would like to be fixed before merging.
|
|
||
| private static void ReadSample(BinaryReader reader, float[] sampleBuffer, int startIndex, int sampleCount) | ||
| { | ||
| byte[] buffer = new byte[sampleCount * 2]; |
There was a problem hiding this comment.
Could we reuse the buffer array somehow to avoid frequent allocations and excessive garbage collector work?
There was a problem hiding this comment.
How about create a reuseable fixed length byte[] buffer when new Wav(), and transfer this buffer to ReadSample method. Works like a Stream's internal buffer.
| { | ||
| byte byte1 = buffer[i]; | ||
| byte byte2 = buffer[i + 1]; | ||
| short s = (short)((byte2 << 8) | byte1); |
There was a problem hiding this comment.
Let's reuse BytesToFloat here to avoid the transformation logic duplication, please.
There was a problem hiding this comment.
Maybe we shall test the performances of the following two solutions:
- reuse
BytesToFloatmethod; - directly insert body of
BytesToFloat;
After performance test, choose the fastest one.
There was a problem hiding this comment.
Do you expect significant function call overhead? In this case, I like your idea to test both of them. If the inlined version works faster I think it's a reasonable cost to duplicate the code.
2 participants
When smaplesToRead reaches 35000 or above, reading a float a time will consumes about 3 seconds, which stalls the whole game.
Should use bunch read to speed up.