-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Stream.Read(Span<byte>) overloads. #21356
base: bleed
Are you sure you want to change the base?
Conversation
The default Stream implementation of this method has to rent an array so it can call the overload that accepts an array, and then copy the output over. This is because the array overload is required and the span overload was only added more recently. We can avoid the overhead of this by implementing the span overload and working with the destination span directly. Do so for all classes we have that derive from Stream, and redirect their array overload to the span overload for code reuse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes LGTM, and tested that the 4 default mods all launch to the shellmap without problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had also tested a bunch of formats, but didn't test videos yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
untested.
@@ -112,8 +117,8 @@ public override int Read(byte[] buffer, int offset, int count) | |||
for (var i = 0; i < samples; i++) | |||
{ | |||
var conversion = (short)(floatBuffer[i] * 32767); | |||
buffer[offset++] = (byte)(conversion & 255); | |||
buffer[offset++] = (byte)(conversion >> 8); | |||
buffer[i * 2 + 0] = (byte)(conversion & 255); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trivial. But slower. Would then store it in a temp var.
The default Stream implementation of this method has to rent an array so it can call the overload that accepts an array, and then copy the output over. This is because the array overload is required and the span overload was only added more recently.
We can avoid the overhead of this by implementing the span overload and working with the destination span directly. Do so for all classes we have that derive from Stream, and redirect their array overload to the span overload for code reuse.