-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Proposal: Endianness Enum and Extensions for BinaryReader an BinaryWriter #26904
Comments
Sounds like it would overlap quite a bit with |
@khellang Yes, I have implemented this by myself and i have done it with help of BinaryPrimitives.ReverseEndianness(); |
I've not worked with many data streams that need to read different endianness data in the same stream.
|
Hi @vcsjones I have never worked on something that needs to change the Endianness in the same stream, but it could be a possible (I cant imagine a case like this). I'm not completely sure, but I think that BinaryReader takes by default the Endianness of the architecture by default (Most arch are little endian). 1.- As I said, I think its the default (What is actualy implemented). But I agree that would be great to have a static property in some class that represents the system information that handles the Endianess. Just like BitConverter.IsLittleEndian but returning the actual Endianness enum value. 2.- I would prefere the API I have described, but I don't think its a bad idea what you said. Something like set the default endianness of the BinaryRW on the constructor. I think that we can have the two things at the same time without cost. 3.- As I said, I think that it gets the default of the architecture, so your case can be handled in the example of 2, putting the default endiannes on the constructor or something like that. |
As far as I'm aware, the .NET CLR itself is little-endian, and the BinaryReader/Writer implementations are built with this in mind, swapping bytes as necessary when running on a big-endian machine. This simplifies running code on the CLR, as endianness is completely taken out of the equation. |
@Halofreak1990 If somebody from CoreCLR could provide us a bit more of help |
I'm not opposed to having additional members, but I would want to see more feedback to justify the value here. It is relatively trivial to derive from |
@JeremyKuhne For my experience, most of the times I use the |
Just a data point on this, parts of the TDS protocol used in SqlClient require specific and opposite endian-ness |
For example, if you have a game data file that needs to be written using different endianness depending on the platform. If reading/writing the file for Windows, then you may use LittleEndian, and instantiate the BinaryReader/Writer as so. Whereas for the PlayStation 3, the reader/writer would be instantiated with BigEndian. |
@Ryder25 that is assuming you'd output your assets in a different endianness for each specific platform, which is something I'd personally never do. Why complicate development, if the framework automagically fixes things for you? .NET, as it currently works, allows one to use little-endian assets, and in code never care about that, unless you need to fiddle with specific bits. For example, a little endian long value in a file will produce the expected value in code, regardless of the endianness of the platform. I'd be inclined to say that, if you really need control of the endiannes on a case by case basis, you really need to re-evaluate what you're doing, because something's out of whack. |
I'm not saying I would do it, but I've seen it done before with several games. I assume it is with performance reasons in mind. I can see a case where the game itself is written in C++, and there could be tooling done in C#. At that point, you can't only focus on what's happening in the .Net bcl. |
Triage: There are enough scenarios that we see the value in doing this. |
@carlossanlop What is required for making this API real? |
@deinok in the main description with the API proposal, instead of |
@carlossanlop Done |
Passing an extra I agree with @JeremyKuhne that it is trivial to derive from |
There are multiple ways to do this. Here are some different ideas explained by usage examples:
public class BigEndianBinaryReader : BinaryReader { ... }
int Example(Stream in)
{
// Assume first byte in stream denotes whether the data is little or big endian
var endianness = in.ReadByte(); // 0 - means little, 1+ means big
var reader = endianness == 0 ? new BinaryReader(in) : new BigEndianBinaryReader(in);
using (reader)
{
return reader.ReadInt32();
}
}
int Example(Stream in)
{
// Assume first byte in stream denotes whether the data is little or big endian
var endianness = in.ReadByte(); // 0 - means little, 1+ means big
bool isLittleEndian = endianness == 0;
using (BinaryReader reader = new BinaryReader(in, isLittleEndian))
{
return reader.ReadInt32();
}
}
These implementations should add little overhead in terms of creating work to maintain the API. This also prevents someone from having to create their own utility library for this kind of functionality, or from having to copy that code from project to project. Overall this will make it really convenient to read and write binary data for various architectures. Of course, we'll also have to think about performance when it comes to this API which may change things. I just want to get the ball rolling with some ideas. |
I'd have thought that if we add a flag for this it would hit performance as an extra if condition would be needed for every read/write. Would it be better to simply provide a second set of methods for big endian with a BE suffix. Something like this?
|
Missed that fact. I supose that for big frames that if condition would cause a big overhead |
A couple observations I would add as someone who's implemented big endian by derivation:
Given these considerations, my design endpoint here is |
In the UWP platform there is the
So, my proposal is to migrate these instead of making changes to |
This issue has been hanging around for 5 years... |
@GF-Huang That's only because they shut the previous feature suggestion lists down and started new ones. It's been kicking around since .Net 1.0 was released. 21 years ago. |
Amazing.. and unbelievable.. |
I've implemented the Big Endian methods as extensions. Many network protocols are big endian on the wire, so being able to easily deserialize big endian binary payloads with BinaryReader comes in handy. The lack of big endian methods seems like a huge oversight, I'm not surprised it has been a known issue for 21 years. These extensions are not as performant as an internal implementation, but enough features are exposed via the public api surface that it's possible to get close to a native implementation. In particular, there's an optimisation in BinaryReader when reading from MemoryStream to avoid buffer copies, however it only works if MemoryStream was constructed with public static class BinaryReaderBigEndianExtensions
{
public static short ReadInt16BigEndian(this BinaryReader binaryReader) => BinaryPrimitives.ReadInt16BigEndian(
binaryReader.BaseStream is MemoryStream ms && ms.TryReadSpanUnsafe(2, out ReadOnlySpan<byte> readBytes) ? readBytes : binaryReader.ReadSpan(stackalloc byte[2]));
public static ushort ReadUInt16BigEndian(this BinaryReader binaryReader) => BinaryPrimitives.ReadUInt16BigEndian(
binaryReader.BaseStream is MemoryStream ms && ms.TryReadSpanUnsafe(2, out ReadOnlySpan<byte> readBytes) ? readBytes : binaryReader.ReadSpan(stackalloc byte[2]));
public static int ReadInt32BigEndian(this BinaryReader binaryReader) => BinaryPrimitives.ReadInt32BigEndian(
binaryReader.BaseStream is MemoryStream ms && ms.TryReadSpanUnsafe(4, out ReadOnlySpan<byte> readBytes) ? readBytes : binaryReader.ReadSpan(stackalloc byte[4]));
public static uint ReadUInt32BigEndian(this BinaryReader binaryReader) => BinaryPrimitives.ReadUInt32BigEndian(
binaryReader.BaseStream is MemoryStream ms && ms.TryReadSpanUnsafe(4, out ReadOnlySpan<byte> readBytes) ? readBytes : binaryReader.ReadSpan(stackalloc byte[4]));
public static long ReadInt64BigEndian(this BinaryReader binaryReader) => BinaryPrimitives.ReadInt64BigEndian(
binaryReader.BaseStream is MemoryStream ms && ms.TryReadSpanUnsafe(8, out ReadOnlySpan<byte> readBytes) ? readBytes : binaryReader.ReadSpan(stackalloc byte[8]));
public static ulong ReadUInt64BigEndian(this BinaryReader binaryReader) => BinaryPrimitives.ReadUInt64BigEndian(
binaryReader.BaseStream is MemoryStream ms && ms.TryReadSpanUnsafe(8, out ReadOnlySpan<byte> readBytes) ? readBytes : binaryReader.ReadSpan(stackalloc byte[8]));
public static Half ReadHalfBigEndian(this BinaryReader binaryReader) => BinaryPrimitives.ReadHalfBigEndian(
binaryReader.BaseStream is MemoryStream ms && ms.TryReadSpanUnsafe(2, out ReadOnlySpan<byte> readBytes) ? readBytes : binaryReader.ReadSpan(stackalloc byte[2]));
public static float ReadSingleBigEndian(this BinaryReader binaryReader) => BinaryPrimitives.ReadSingleBigEndian(
binaryReader.BaseStream is MemoryStream ms && ms.TryReadSpanUnsafe(4, out ReadOnlySpan<byte> readBytes) ? readBytes : binaryReader.ReadSpan(stackalloc byte[4]));
public static double ReadDoubleBigEndian(this BinaryReader binaryReader) => BinaryPrimitives.ReadDoubleBigEndian(
binaryReader.BaseStream is MemoryStream ms && ms.TryReadSpanUnsafe(8, out ReadOnlySpan<byte> readBytes) ? readBytes : binaryReader.ReadSpan(stackalloc byte[8]));
private static ReadOnlySpan<byte> ReadSpan(this BinaryReader binaryReader, Span<byte> buffer)
{
binaryReader.BaseStream.ReadExactly(buffer);
return buffer;
}
private static bool TryReadSpanUnsafe(this MemoryStream memoryStream, int numBytes, out ReadOnlySpan<byte> readBytes)
{
if (memoryStream.TryGetBuffer(out var msBuffer))
{
readBytes = msBuffer.AsSpan((int)memoryStream.Position, numBytes);
memoryStream.Seek(numBytes, SeekOrigin.Current);
return true;
}
else
{
readBytes = [];
return false;
}
}
} |
Proposal
Add an enum that handles the endiannes
Also add extensions to BinaryReader like:
And for BinaryWriter:
Rationale
BinaryReader and BinaryWriter works really well for accessing Streams but they seems to lack the support for byte-order. And the usage of an enum for representing the Endianness seems more clear to me than using the BitConverter.IsLittleEndian bool.
The text was updated successfully, but these errors were encountered: